Add blog post on the Scala 3.3.2 post-mortem. #1603
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lbialy
suggested changes
Mar 4, 2024
| As we mentioned in the previous section, the main point of failure was a lack of tests for forward compatibility scenarios. | ||
| Our most important fix to the process will therefore be to add adequate forward compatibility tests in the compiler's test suite. | ||
|
|
||
| In addition, this particular bug highlights the importance of accurately following the specificiation of the TASTy format. |
|
|
||
| ## Conclusion | ||
|
|
||
| In this post, we examined in detail the bug and events that led to the abandon of the Scala 3.3.2 release. |
There was a problem hiding this comment.
Abandonment? To 3.3.2 being abandoned?
mkurz
reviewed
Mar 4, 2024
| Between 3.3.1 and 3.3.2, we merged [PR #18280](https://github.com/lampepfl/dotty/pull/18280). | ||
| That PR starts with what appeared to be a *refactoring* commit that renamed some `NameKind`s and their associated strings. | ||
| Unfortunately, that means the 3.3.2 compiler started emitting `UniqueName`s with the string `"contextual$"`. | ||
| When the 3.3.1 compiler reads that structured name, it tries to find the cached instance of `NameKind` that it should use for `"constextual$"`, and does not find any, because it did not use to generate that string. |
There was a problem hiding this comment.
Typo
Suggested change
| When the 3.3.1 compiler reads that structured name, it tries to find the cached instance of `NameKind` that it should use for `"constextual$"`, and does not find any, because it did not use to generate that string. | |
| When the 3.3.1 compiler reads that structured name, it tries to find the cached instance of `NameKind` that it should use for `"contextual$"`, and does not find any, because it did not use to generate that string. |
mkurz
reviewed
Mar 4, 2024
| The RC cycles failed to surface the issue as well, for a similar reason. | ||
| When trying out an RC, users rarely publish the product, even locally, then use it with an older compiler. | ||
|
|
||
| Fortunately, as a last line of defense, the issue was caught by the Play! framework, in-between the release of the 3.3.2 artifacts on Maven Central and the official announcement of the release. |
There was a problem hiding this comment.
Suggested change
| Fortunately, as a last line of defense, the issue was caught by the Play! framework, in-between the release of the 3.3.2 artifacts on Maven Central and the official announcement of the release. | |
| Fortunately, as a last line of defense, the issue was caught by the Play Framework, in-between the release of the 3.3.2 artifacts on Maven Central and the official announcement of the release. |
There was a problem hiding this comment.
Maybe reference the GitHub issue?
Suggested change
| Fortunately, as a last line of defense, the issue was caught by the Play! framework, in-between the release of the 3.3.2 artifacts on Maven Central and the official announcement of the release. | |
| Fortunately, as a last line of defense, [the issue was caught](https://github.com/playframework/playframework/issues/12418) by the Play Framework in-between the release of the 3.3.2 artifacts on Maven Central and the official announcement of the release. |
SethTisue
reviewed
Mar 5, 2024
|
All suggested changes applied. :) |
SethTisue
approved these changes
Mar 5, 2024
bishabosha
reviewed
Mar 5, 2024
bishabosha
reviewed
Mar 5, 2024
bishabosha
reviewed
Mar 5, 2024
bishabosha
approved these changes
Mar 5, 2024
Kordyjan
approved these changes
Mar 6, 2024
| For example, the name of the class behind an `object Foo` definition has a structured name `ModuleClass("Foo")`, instead of the string `"Foo$"`. | ||
|
|
||
| Likewise, generated names for anonymous entites, such as `implicit` parameters, have the structured name `UniqueName("evidence$", 1)`. | ||
| The string `"evidence$"` used in the equality definition of names, so that `UniqueName("evidence$", 1)` is not the same name as `UniqueName("foo$", 1)`, but it does not otherwise have any semantic meaning in TASTy. |
There was a problem hiding this comment.
Suggested change
| The string `"evidence$"` used in the equality definition of names, so that `UniqueName("evidence$", 1)` is not the same name as `UniqueName("foo$", 1)`, but it does not otherwise have any semantic meaning in TASTy. | |
| The string `"evidence$"` is used in the equality definition of names, so that `UniqueName("evidence$", 1)` is not the same name as `UniqueName("foo$", 1)`, but it does not otherwise have any semantic meaning in TASTy. |
SethTisue
reviewed
Mar 6, 2024
| Our most important fix to the process will therefore be to add adequate forward compatibility tests in the compiler's test suite. | ||
|
|
||
| In addition, this particular bug highlights the importance of accurately following the specification of the TASTy format. | ||
| Making additional assumptions on the *origin* of TASTy file---namely that only our own compiler can produce them---leads to problems like this issue. |
There was a problem hiding this comment.
Suggested change
| Making additional assumptions on the *origin* of TASTy file---namely that only our own compiler can produce them---leads to problems like this issue. | |
| Making additional assumptions on the *origin* of TASTy files---namely that only our own compiler can produce them---leads to problems like this issue. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
/cc @lbialy.