Skip to content

Avoid retraversing parts of the tree that do not contain Quote trees #17407

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

Merged
merged 4 commits into from
May 16, 2023
Merged

Avoid retraversing parts of the tree that do not contain Quote trees #17407

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c27f870
Avoid retraversing parts of the tree that do not contain Quote trees
nicolasstucki May 4, 2023
627b558
Refactor way we handle hole contents and tags while pickling quotes
nicolasstucki May 12, 2023
7195952
Remove illegal use of WildcardType
nicolasstucki May 12, 2023
b7cc753
Fix docs
nicolasstucki May 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions compiler/src/dotty/tools/dotc/transform/PickleQuotes.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,8 @@ class PickleQuotes extends MacroTransform {
case Apply(Select(quote: Quote, nme.apply), List(quotes)) =>
val (contents, quote1) = makeHoles(quote)
val quote2 = encodeTypeArgs(quote1)
val contents1 = contents ::: quote.tags
val pickled = PickleQuotes.pickle(quote2, quotes, contents1)
transform(pickled) // pickle quotes that are in the contents
val contents1 = contents.map(transform(_)) ::: quote.tags
Copy link
Member

Choose a reason for hiding this comment

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

By the way, why are the tags after the content? Aren't they referenced in the body?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tags will not contain quotes, they are all TermRefs. Therfore we know that this transformation will have no effect.

Copy link
Member

Choose a reason for hiding this comment

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

right, I mean, why isn't it quote.tags ::: contents.map(...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The order was irrelevant as we split them in PickleQuotes.pickle. This also deserves some optimization. I will add a commit for that.

Copy link
Contributor Author

@nicolasstucki nicolasstucki May 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It was added at the end because of a zipWithIndex that we needed for the contents but not the tags. There was a TODO to cleanup that mess.

PickleQuotes.pickle(quote2, quotes, contents1)
case tree: DefDef if !tree.rhs.isEmpty && tree.symbol.isInlineMethod =>
tree
case _ =>
Expand Down