-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Move reflection inside QuoteContext #6723
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
Move reflection inside QuoteContext #6723
Conversation
def3c4e
to
88b0607
Compare
c0d7981
to
b84d9b8
Compare
9af660c
to
9ec88d7
Compare
9ec88d7
to
34cfb6a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There are many places where we change import refl._
to import qctx.tasty._
, which is a small regression in usability at first glance. This change is only justified by the hypothesis that most macros will be quote macros (which does not need the import), reflection macros are less common.
It would be good to have more evidence to support the hypothesis in real-world projects before deprecating the old syntax for reflection macros.
@@ -2,16 +2,26 @@ package scala.quoted | |||
|
|||
import scala.quoted.show.SyntaxHighlight | |||
|
|||
class QuoteContext(reflection: tasty.Reflection) { | |||
class QuoteContext(val tasty: scala.tasty.Reflection) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick : I'm not sure if tasty
is a good name: (1) as a meta-programmer, I don't care about whether tasty or not, but only if the APIs are stable, useful and friendly; and it's easy to debug compiler crashes when using reflect API; (2) Reflection is more like an interface to compiler, reflect
might be a better name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case tasty
should be read as the Typed AST API. I will add some documentation for it.
We cannot use reflect
as it conflicts with the scala.reflect
package and also the expectation of the users that know about scala.reflect
. Calling scala.tasty.Reflection
has already proven to lead the confusion and we should probably also rename it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least in scalatest
it does not affect the code much, we can even remove one import per file.
34cfb6a
to
031110b
Compare
Note that using |
Homogenize API used for macros and runtime staging. Support
QuoteContext
instead ofReflection
as the implicit that macros take as parameter. We still a keep deprecated support forReflection
in macros. Note that the macros inscalatest
,sourcecode
andxml-interpolator
did not need to change, they only get the warning suggesting to useQuoteContext
.