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.

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

Fix owner of splices in class statements #18359

Merged
merged 1 commit into from
Aug 9, 2023
Merged

Fix owner of splices in class statements #18359

merged 1 commit into from
Aug 9, 2023

Conversation

nicolasstucki
Copy link
Contributor

Fixes #18358

@nicolasstucki nicolasstucki marked this pull request as ready for review August 8, 2023 14:09
@nicolasstucki nicolasstucki requested a review from smarter August 8, 2023 14:10
smarter
smarter reviewed Aug 8, 2023
View reviewed changes
compiler/src/dotty/tools/dotc/quoted/PickledQuotes.scala Outdated
@@ -125,6 +125,18 @@ object PickledQuotes {
val quotedType = evalHole.nn.apply(idx, reifyTypeHoleArgs(args))
PickledQuotes.quotedTypeToTree(quotedType)
}
case tree @ Template(constr, parents, self, _) if tree.derived.isEmpty =>
Copy link
Member

Choose a reason for hiding this comment

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

Can tree.derived not be empty here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied it from the TreeMap case. Not sure why it is there. It was introduced in fb09e82.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Following up in #18368

smarter
smarter reviewed Aug 8, 2023
View reviewed changes
compiler/src/dotty/tools/dotc/quoted/PickledQuotes.scala Outdated
case stat: DefTree => transform(stat)
case stat =>
val localDummy = tree.tpe.termSymbol
transform(stat)(using ctx.withOwner(localDummy))
Copy link
Member

Choose a reason for hiding this comment

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

Could TreeMapWithPreciseStatContexts be reused to avoid a special case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

@nicolasstucki nicolasstucki requested a review from smarter August 9, 2023 09:23
@nicolasstucki nicolasstucki added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Aug 9, 2023
@smarter smarter changed the title Fix owner of spices in class statements Fix owner of splices in class statements Aug 9, 2023
@smarter smarter merged commit 16e3bb4 into scala:main Aug 9, 2023
@smarter smarter deleted the fix-18358 branch August 9, 2023 12:08
@Kordyjan Kordyjan removed the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Oct 10, 2023
@Kordyjan Kordyjan mentioned this pull request Dec 1, 2023
Merged
Kordyjan added a commit that referenced this pull request Dec 8, 2023
@Kordyjan
Backport "Fix owner of splices in class statements" to LTS (#19153)
56415fb 
Backports #18359 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
@Kordyjan Kordyjan added this to the 3.3.2 milestone Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smarter smarter smarter approved these changes

Assignees

@smarter smarter

Labels
None yet
Projects
None yet
Milestone
3.3.2
Development

Successfully merging this pull request may close these issues.

Splices in statements of class definition have the wrong owner.
3 participants
@nicolasstucki @smarter @Kordyjan