-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #4431: Change semantics of inline params in macro #4460
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
Conversation
For context: the motivation for this semantics in #4431 is that this is a limited form of cross-stage persistence (maybe in reverse) or inspection of trees, which is easy for literals/primitive types but hard in general. (Tho, I was thinking of using a typeclass for extensibility). 👍 for the semantic change (and for updating docs). |
@@ -66,16 +66,19 @@ import dotty.tools.dotc.core.quoted._ | |||
* ``` | |||
* to | |||
* ``` | |||
* inline def foo[T1, ...](inline x1: X, ..., y1: Y, ....): Seq[Any] => Object = { (args: Seq[Any]) => { | |||
* inline def foo[T1, ...](inline x1: X, ..., y1: Y, ..., inline f1: X => Y): Seq[Any] => Object = { (args: Seq[Any]) => { |
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.
Maybe add f1
at the original form as well in the comment.
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.
I removed it and made it part of the comment below
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.
Seems good, nice.
@@ -625,6 +629,9 @@ class ReifyQuotes extends MacroTransformWithImplicits with InfoTransformer { | |||
} | |||
} | |||
|
|||
private def isStage0Value(sym: Symbol)(implicit ctx: Context): Boolean = | |||
sym.is(Inline) && sym.owner.is(Macro) && !defn.isFunctionType(sym.info) |
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.
The addition of !defn.isFunctionType(sym.info)
is the only real change in this PR, the rest is refactoring.
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 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.
After discussion with Aggelos, also LGTM for me.
@Blaisorblade "cross stage persistence in reverse" is a good way to characterize this.
inline
macro parameters in stage 0 if they have type Boolean, Byte, Short, Int, Long, Float, Double, Char or StringThis change in semantics allows us to support
inline
parameters of function type.