-
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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]) => { | ||
* val T1$1 = args(0).asInstanceOf[Type[T1]] | ||
* ... | ||
* val x1$1 = args(0).asInstanceOf[X] | ||
* ... | ||
* val y1$1 = args(1).asInstanceOf[Expr[Y]] | ||
* ... | ||
* { ... x1$1 .... '{ ... T1$1.unary_~ ... x1$1.toExpr.unary_~ ... y1$1.unary_~ ... } ... } | ||
* { ... x1$1 .... '{ ... T1$1.unary_~ ... x1$1.toExpr.unary_~ ... y1$1.unary_~ ... f1$1.unary_~ ... } ... } | ||
* } | ||
* ``` | ||
* Where `inline` parameters with type Boolean, Byte, Short, Int, Long, Float, Double, Char and String are | ||
* passed as their actual runtime value. See `isStage0Value`. | ||
* | ||
* Note: the parameters of `foo` are kept for simple overloading resolution but they are not used in the body of `foo`. | ||
* | ||
* At inline site we will call reflectively the static method `foo` with dummy parameters, which will return a | ||
|
@@ -243,7 +246,7 @@ class ReifyQuotes extends MacroTransformWithImplicits with InfoTransformer { | |
def levelOK(sym: Symbol)(implicit ctx: Context): Boolean = levelOf.get(sym) match { | ||
case Some(l) => | ||
l == level || | ||
sym.is(Inline) && sym.owner.is(Macro) && sym.info.isValueType && l - 1 == level | ||
l == 1 && level == 0 && isStage0Value(sym) | ||
case None => | ||
!sym.is(Param) || levelOK(sym.owner) | ||
} | ||
|
@@ -374,8 +377,8 @@ class ReifyQuotes extends MacroTransformWithImplicits with InfoTransformer { | |
else ref(defn.QuotedExpr_apply).appliedToType(body1.tpe.widen).appliedTo(body1) | ||
} | ||
else body match { | ||
case body: RefTree if isCaptured(body, level + 1) => | ||
if (body.symbol.is(Inline)) { | ||
case body: RefTree if isCaptured(body.symbol, level + 1) => | ||
if (isStage0Value(body.symbol)) { | ||
// Optimization: avoid the full conversion when capturing inlined `x` | ||
// in '{ x } to '{ x$1.toExpr.unary_~ } and go directly to `x$1.toExpr` | ||
liftValue(capturers(body.symbol)(body)) | ||
|
@@ -476,7 +479,7 @@ class ReifyQuotes extends MacroTransformWithImplicits with InfoTransformer { | |
val tpw = tree.tpe.widen | ||
val argTpe = | ||
if (tree.isType) defn.QuotedTypeType.appliedTo(tpw) | ||
else if (tree.symbol.is(Inline)) tpw // inlined term | ||
else if (isStage0Value(tree.symbol)) tpw | ||
else defn.QuotedExprType.appliedTo(tpw) | ||
val selectArg = arg.select(nme.apply).appliedTo(Literal(Constant(i))).asInstance(argTpe) | ||
val capturedArg = SyntheticValDef(UniqueName.fresh(tree.symbol.name.toTermName).toTermName, selectArg) | ||
|
@@ -509,11 +512,11 @@ class ReifyQuotes extends MacroTransformWithImplicits with InfoTransformer { | |
} | ||
|
||
/** Returns true if this tree will be captured by `makeLambda` */ | ||
private def isCaptured(tree: RefTree, level: Int)(implicit ctx: Context): Boolean = { | ||
private def isCaptured(sym: Symbol, level: Int)(implicit ctx: Context): Boolean = { | ||
// Check phase consistency and presence of capturer | ||
( (level == 1 && levelOf.get(tree.symbol).contains(1)) || | ||
(level == 0 && tree.symbol.is(Inline)) | ||
) && capturers.contains(tree.symbol) | ||
( (level == 1 && levelOf.get(sym).contains(1)) || | ||
(level == 0 && isStage0Value(sym)) | ||
) && capturers.contains(sym) | ||
} | ||
|
||
/** Transform `tree` and return the resulting tree and all `embedded` quotes | ||
|
@@ -550,13 +553,13 @@ class ReifyQuotes extends MacroTransformWithImplicits with InfoTransformer { | |
splice(ref(splicedType).select(tpnme.UNARY_~)) | ||
case tree: Select if tree.symbol.isSplice => | ||
splice(tree) | ||
case tree: RefTree if isCaptured(tree, level) => | ||
case tree: RefTree if isCaptured(tree.symbol, level) => | ||
val capturer = capturers(tree.symbol) | ||
def captureAndSplice(t: Tree) = | ||
splice(t.select(if (tree.isTerm) nme.UNARY_~ else tpnme.UNARY_~)) | ||
if (tree.symbol.is(Inline) && level == 0) capturer(tree) | ||
else if (tree.symbol.is(Inline)) captureAndSplice(liftValue(capturer(tree))) | ||
else captureAndSplice(capturer(tree)) | ||
if (!isStage0Value(tree.symbol)) captureAndSplice(capturer(tree)) | ||
else if (level == 0) capturer(tree) | ||
else captureAndSplice(liftValue(capturer(tree))) | ||
case Block(stats, _) => | ||
val last = enteredSyms | ||
stats.foreach(markDef) | ||
|
@@ -625,6 +628,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 commentThe reason will be displayed to describe this comment to others. Learn more. The addition of There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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. |
||
|
||
private def liftList(list: List[Tree], tpe: Type)(implicit ctx: Context): Tree = { | ||
list.foldRight[Tree](ref(defn.NilModule)) { (x, acc) => | ||
acc.select("::".toTermName).appliedToType(tpe).appliedTo(x) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
|
||
object Foo { | ||
inline def g(inline p: Int => Boolean): Boolean = ~{ | ||
if(p(5)) '(true) // error | ||
else '(false) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
abc42 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import scala.quoted._ | ||
|
||
object Macros { | ||
inline def h(inline f: Int => String): String = ~ '(f(42)) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
import scala.quoted._ | ||
import Macros._ | ||
|
||
object Test { | ||
def main(args: Array[String]): Unit = { | ||
println(h(x => "abc" + x)) | ||
} | ||
} |
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.