Skip to content

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

Merged
merged 3 commits into from
May 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
31 changes: 19 additions & 12 deletions compiler/src/dotty/tools/dotc/transform/ReifyQuotes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ import dotty.tools.dotc.core.quoted._
* { ... x1$1 .... '{ ... T1$1.unary_~ ... x1$1.toExpr.unary_~ ... y1$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`. Other `inline` arguments such as functions are handled
* like `y1: Y`.
*
* 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
Expand Down Expand Up @@ -243,7 +247,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)
}
Expand Down Expand Up @@ -374,8 +378,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))
Expand Down Expand Up @@ -476,7 +480,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)
Expand Down Expand Up @@ -509,11 +513,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
Expand Down Expand Up @@ -550,13 +554,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)
Expand Down Expand Up @@ -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)
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 addition of !defn.isFunctionType(sym.info) is the only real change in this PR, the rest is refactoring.

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

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.


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)
Expand Down
7 changes: 4 additions & 3 deletions docs/docs/reference/principled-meta-programming.md
Original file line number Diff line number Diff line change
Expand Up @@ -394,9 +394,10 @@ point. To reflect this, we loosen the phase consistency requirements
as follows:

- If `x` is an inline value (or an inline parameter of an inline
function), it can be accessed in all contexts where the number of
splices minus the number of quotes between use and definition is
either 0 or 1.
function) of type Boolean, Byte, Short, Int, Long, Float, Double,
Char or String, it can be accessed in all contexts where the number
of splices minus the number of quotes between use and definition
is either 0 or 1.

### Relationship with Staging

Expand Down
7 changes: 7 additions & 0 deletions tests/neg/i4433.scala
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)
}
}
1 change: 1 addition & 0 deletions tests/run/i4431.check
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
abc42
5 changes: 5 additions & 0 deletions tests/run/i4431/quoted_1.scala
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))
}
8 changes: 8 additions & 0 deletions tests/run/i4431/quoted_2.scala
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))
}
}