Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes involving SAM types #10979

Merged
merged 3 commits into from
Jan 5, 2021
Merged

Fixes involving SAM types #10979

merged 3 commits into from
Jan 5, 2021

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jan 3, 2021

First, move ExpandSAMs to a group before HoistSuperArgs

Fixes #9391

This also uncovered two follow-on errors which were previously not detected, and which are fixed in the 2nd and 3rd commit.

There are interactions for treating partial functions that require
a split into different groups.

This move uncovered two follow-on errors which were previously not detected.
…ass.

This causes an error in RefChecks for run/fragables-extension.scala otherwise. Previously
the error was not detected since methods created in the same group as RefChecks are not checked.
When extracting a SAMtype we use avoidance of the class parameters since these
are not visible outside the class. But we should not do that when the SAMType
is extracted with an owner within the class.

Fixes a problem with sammy_scope.scala that was left dormant until now since
RefChecks did not check methods generated by ExpandSAMs.
@odersky
Copy link
Contributor Author

odersky commented Jan 4, 2021

It was instructive to see how this could be fixed:

#9391 looked at first mysterious, with a key-not-found crash in a tricky part of LambdaLift. But turning on -Ycheck:all revealed that there was a wrong owner after RefChecks. The phase group before RefChecks contained both the miniphase ByNameClosures for transforming by-name parameters and the miniphase ExpandSAMs for lifting closures into PartialFunctions. Since it was known that the example failed only if the two features were combined it was natural to try to separate the two miniphases. I split the phase group in the middle and that fixed the problem.

Since we do not want to create new phase groups if not absolutely necessary, I then moved ExpandSAMs to the previous phase group. That exposed two other errors, which in the end both had to do with the fact that methods created in the same group as RefChecks are not necessarily checked by it (the thinking is that these methods are compiler-generated anyway, so should be OK to start with). Now that ExpandSAMs ran in a group before RefChecks, some errors in the code it generated were reported.
So the larger part of the fix was to address these errors.

Morale: Kudos to our internal architecture and checking framework that allows us to move so fast to diagnose and fix bugs.

@prolativ prolativ merged commit 2be0707 into scala:master Jan 5, 2021
@prolativ prolativ deleted the fix-#9391 branch January 5, 2021 10:44
@Kordyjan Kordyjan added this to the 3.0.0 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"NoSuchElementException: key not found" with unusual partial function
3 participants