Make more anonymous functions static #19251
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
|
commit messages have wrong ticket number (should be 19224, not 19924 — I fixed it in PR description) |
|
I've added some comments in #19224 (comment) |
retronym
reviewed
Dec 13, 2023
| } | ||
|
|
||
| val t1 = new C | ||
| val t2 = new C |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
retronym
reviewed
Dec 13, 2023
| ) | ||
| || ( | ||
| && (!sym.isAnonymousFunction || sym.owner.ownersIterator.exists(_.isConstructor)) | ||
| // For anonymous functions in static objects, we prefer them to be static because |
There was a problem hiding this comment.
What does the "in static objects" part mean here?
There was a problem hiding this comment.
Static means toplevel or nested in a static object.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
It added more comments. Hope it is clearer now.
There was a problem hiding this comment.
Thanks, that helps me understand.
An anonymous function in a static object was previously mapped to a member of that object. We now map it to a static member of the toplevel class instead. This causes the backend to memoize the function, which fixes scala#19224. On the other hand, we don't do that for anonymous functions nested in the object constructor, since that can cause deadlocks (see run/deadlock.scala). Scala 2's behavior is different: it does lift lambdas in constructors to be static, too, which can cause deadlocks. Fixes scala#19224
retronym
approved these changes
Dec 13, 2023
| def setLogicOwner(local: Symbol) = | ||
| val encClass = local.owner.enclosingClass | ||
| // When to [efer enclosing class over enclosing package: |
| ) | ||
| || ( | ||
| && (!sym.isAnonymousFunction || sym.owner.ownersIterator.exists(_.isConstructor)) | ||
| // For anonymous functions in static objects, we prefer them to be static because |
There was a problem hiding this comment.
Thanks, that helps me understand.
odersky
commented
Dec 13, 2023
odersky
added
the
backport:nominated
|
I believe it would be good to backport that patch to LTS, even though it changes runtime behavior. |
unkarjedy
pushed a commit
to JetBrains/intellij-scala
that referenced
this pull request
May 16, 2024
…n stopping in lambdas #SCL-22554 fixed #SCL-22145 - Scala 3.4 and later tries to compile lambdas to the outermost static class. - The reasons for this are explained in scala/scala3#19251. - When trying to stop at breakpoints inside lambdas, it is important to load the outermost class and its nested classes. - Java also does this in the platform implementation. - Tests that cover code examples mentioned in the Scala 3 PR have been added.
unkarjedy
pushed a commit
to JetBrains/intellij-scala
that referenced
this pull request
May 16, 2024
…n stopping in lambdas #SCL-22554 fixed #SCL-22145 - Scala 3.4 and later tries to compile lambdas to the outermost static class. - The reasons for this are explained in scala/scala3#19251. - When trying to stop at breakpoints inside lambdas, it is important to load the outermost class and its nested classes. - Java also does this in the platform implementation. - Tests that cover code examples mentioned in the Scala 3 PR have been added.
WojciechMazur
added a commit
that referenced
this pull request
Jun 26, 2024
Backports #19251 to the LTS branch. PR submitted by the release tooling. [skip ci]
WojciechMazur
removed
the
backport:nominated
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.
An anonymous function in a static object was previously mapped to a member of that object. We now map it to a static member of the toplevel class instead. This causes the backend to memoize the function, which fixes #19224. On the other hand, we don't do that for anonymous functions nested in the object constructor, since that can cause deadlocks (see run/deadlock.scala).
Scala 2's behavior is different: it does lift lambdas in constructors to be static, too, which can cause deadlocks.
Fixes #19224