8000 fix: record calls to constructors in lambdaLift by KacperFKorban · Pull Request #22487 · scala/scala3 · GitHub
[go: up one dir, main page]
Skip to content

fix: record calls to constructors in lambdaLift #22487

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 18, 2025
Merged

fix: record calls to constructors in lambdaLift #22487

merged 2 commits into from
Feb 18, 2025

Conversation

KacperFKorban
Copy link
Member
@KacperFKorban KacperFKorban commented Jan 31, 2025 •
edited
Loading

possible fix for #21931 #22470

Attempt no. 2 for #21931

@KacperFKorban
Copy link
Member Author

test performance please

@KacperFKorban KacperFKorban marked this pull request as ready for review February 6, 2025 14:49
@KacperFKorban KacperFKorban requested a review from odersky February 6, 2025 14:50
@Gedochao Gedochao linked an issue Feb 17, 2025 that may be closed by this pull request
Regression in JVM backend when access closures from local class #22470
Closed
odersky
odersky reviewed Feb 17, 2025
View reviewed changes
compiler/src/dotty/tools/dotc/transform/LambdaLift.scala
if sym.is(Method)
then deps.logicalOwner.getOrElse(sym, sym.enclosure)
else sym.enclosure
deps.logicalOwner.getOrElse(sym, sym.enclosure)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was the isMethod test dropped?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was just my previous approach for solving this issue, so this is basically a revert. Turns out that picking the enclosure (if there is a logicalOwner set) here just isn't correct in some cases.

@KacperFKorban KacperFKorban removed their assignment Feb 18, 2025
@odersky odersky merged commit 417d542 into scala:main Feb 18, 2025
29 checks passed
@odersky odersky deleted the fix-i22470 branch February 18, 2025 15:44
@WojciechMazur WojciechMazur added this to the 3.7.0 milestone Mar 11, 2025
@tgodzik tgodzik mentioned this pull request Apr 7, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@odersky odersky odersky approved these changes

Assignees

@odersky odersky

Labels
None yet
Projects
None yet
Milestone
3.7.0
Development

Successfully merging this pull request may close these issues.

Regression in JVM backend when access closures from local class
3 participants
@KacperFKorban @odersky @WojciechMazur
0