Skip to content

fix #11401 (proxy parameter should be owned by proxy) #11552

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 1 commit into from
Mar 1, 2021
Merged

fix #11401 (proxy parameter should be owned by proxy) #11552

merged 1 commit into from
Mar 1, 2021

Conversation

rssh
Copy link
Contributor

@rssh rssh commented Feb 27, 2021

No description provided.

michelou
michelou reviewed Feb 27, 2021
View reviewed changes
Copy link
Contributor

@michelou michelou left a comment

Choose a reason for hiding this comment

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

@rssh A naive question : why not just write this ?

val arg1 = arg.changeOwner(ctx.owner, boundSym)
if (isByName) DefDef(boundSym, arg1) else ValDef(boundSym, arg1)

@rssh
Copy link
Contributor Author

rssh commented Feb 27, 2021

@rssh A naive question : why not just write this ?

val arg1 = arg.changeOwner(ctx.owner, boundSym)
if (isByName) DefDef(boundSym, arg1) else ValDef(boundSym, arg1)

QuoteUtils.changeOwner checks if the tree already has the right owner.
But here - bounsSym is a new symbol, so this check is always false.
You are right; this will be the same and cleaner.

@rssh
fix scala#11401 (proxy parameter should be owned by proxy)
0241150 
 (since boundSym is new, no need to use check in QuoteUtils.changeOwner)
@nicolasstucki nicolasstucki linked an issue Mar 1, 2021 that may be closed by this pull request
passing generate lambda as macro parameter produce tree with incorrect owner #11401
Closed
@nicolasstucki nicolasstucki merged commit 7bef33b into scala:master Mar 1, 2021
@nicolasstucki
Copy link
Contributor

@rssh WYI now Github requires the fix #11401 text to be in one of the comments to link the PR with the issue.

@nicolasstucki
Copy link
Contributor

Thank you for the fix

@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
Reviewers

@michelou michelou michelou left review comments

@nicolasstucki nicolasstucki nicolasstucki approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.

passing generate lambda as macro parameter produce tree with incorrect owner
4 participants
@rssh @nicolasstucki @michelou @Kordyjan