Skip to content

Optimize ReifyQuotes#mayChange #4261

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.

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 1 commit into from
Apr 6, 2018

Conversation

smarter
Copy link
Member

@smarter smarter commented Apr 5, 2018

No description provided.

@smarter smarter requested a review from nicolasstucki April 5, 2018 23:37
@@ -620,7 +620,8 @@ class ReifyQuotes extends MacroTransformWithImplicits with InfoTransformer {
transform(tp)
}

override protected def mayChange(sym: Symbol)(implicit ctx: Context): Boolean = sym.is(Macro)
override protected def mayChange(sym: Symbol)(implicit ctx: Context): Boolean =
ctx.compilationUnit.containsQuotesOrSplices && sym.isTerm && sym.is(Macro)
Copy link
Contributor

Choose a reason for hiding this comment

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

ctx.compilationUnit.containsQuotesOrSplices will always be true. We only run this phase if it is true
https://github.com/lampepfl/dotty/pull/4261/files#diff-7b199cbe62cc044a10a26459c0f0fa46R94

Copy link
Contributor

Choose a reason for hiding this comment

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

I was wrong, we do need this check.

Copy link
Contributor

Choose a reason for hiding this comment

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

We could also avoid accessing the symbol

  override def transform(ref: SingleDenotation)(implicit ctx: Context): SingleDenotation =
    if (ctx.compilationUnit.containsQuotesOrSplices) super.transform(ref) else ref

  override protected def mayChange(sym: Symbol)(implicit ctx: Context): Boolean = sym.isTerm && sym.is(Macro)

Copy link
Member Author

Choose a reason for hiding this comment

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

Calling Denotation#symbol is just a field access, so I don't think that's worth it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok

@nicolasstucki nicolasstucki merged commit a23ae50 into scala:master Apr 6, 2018
@allanrenucci allanrenucci deleted the optimize/quote-mayChange branch April 6, 2018 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants