From 257d7b4e0ebbd793a80a0fa9a5dcf30465efb0c7 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Fri, 4 May 2018 08:30:30 +0200 Subject: [PATCH 1/3] Fix #4456: Pickle quote before compiling --- .../dotty/tools/dotc/core/quoted/PickledQuotes.scala | 8 +++++++- .../dotty/tools/dotc/core/tasty/TreeUnpickler.scala | 4 ++-- compiler/src/dotty/tools/dotc/quoted/Toolbox.scala | 12 +++++++++--- .../src/dotty/tools/dotc/transform/Splicer.scala | 3 ++- library/src/scala/quoted/Expr.scala | 3 ++- tests/run/quote-run-in-macro-1.check | 2 ++ tests/run/quote-run-in-macro-1/quoted_1.scala | 10 ++++++++++ tests/run/quote-run-in-macro-1/quoted_2.scala | 7 +++++++ 8 files changed, 41 insertions(+), 8 deletions(-) create mode 100644 tests/run/quote-run-in-macro-1.check create mode 100644 tests/run/quote-run-in-macro-1/quoted_1.scala create mode 100644 tests/run/quote-run-in-macro-1/quoted_2.scala diff --git a/compiler/src/dotty/tools/dotc/core/quoted/PickledQuotes.scala b/compiler/src/dotty/tools/dotc/core/quoted/PickledQuotes.scala index 3bf08863b91c..f7d38c39e8bb 100644 --- a/compiler/src/dotty/tools/dotc/core/quoted/PickledQuotes.scala +++ b/compiler/src/dotty/tools/dotc/core/quoted/PickledQuotes.scala @@ -22,7 +22,13 @@ import scala.reflect.ClassTag object PickledQuotes { import tpd._ - /** Pickle the quote into strings */ + /** Pickle the tree of the a quoted.Expr */ + def pickleExpr(tree: Tree)(implicit ctx: Context): scala.quoted.Expr[Any] = { + val pickled = pickleQuote(tree) + scala.runtime.quoted.Unpickler.unpickleExpr(pickled, Nil) + } + + /** Pickle the tree of the quote into strings */ def pickleQuote(tree: Tree)(implicit ctx: Context): scala.runtime.quoted.Unpickler.Pickled = { if (ctx.reporter.hasErrors) Nil else { diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index 44635eb5a249..19d95a711c22 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -19,7 +19,7 @@ import scala.collection.{ mutable, immutable } import config.Printers.pickling import typer.Checking import config.Config -import dotty.tools.dotc.core.quoted.PickledQuotes +import core.quoted.PickledQuotes import scala.quoted import scala.quoted.Types.TreeType import scala.quoted.Exprs.TreeExpr @@ -1142,7 +1142,7 @@ class TreeUnpickler(reader: TastyReader, val idx = readNat() val args = until(end)(readTerm()) val splice = splices(idx) - val reifiedArgs = args.map(arg => if (arg.isTerm) new TreeExpr(arg) else new TreeType(arg)) + val reifiedArgs = args.map(arg => if (arg.isTerm) new TreeExpr(arg, PickledQuotes.pickleExpr(arg)) else new TreeType(arg)) if (isType) { val quotedType = splice.asInstanceOf[Seq[Any] => quoted.Type[_]](reifiedArgs) PickledQuotes.quotedTypeToTree(quotedType) diff --git a/compiler/src/dotty/tools/dotc/quoted/Toolbox.scala b/compiler/src/dotty/tools/dotc/quoted/Toolbox.scala index 81ac31d146d2..b223f63bcb65 100644 --- a/compiler/src/dotty/tools/dotc/quoted/Toolbox.scala +++ b/compiler/src/dotty/tools/dotc/quoted/Toolbox.scala @@ -3,11 +3,13 @@ package dotty.tools.dotc.quoted import dotty.tools.dotc.ast.Trees._ import dotty.tools.dotc.ast.tpd import dotty.tools.dotc.core.Constants._ +import dotty.tools.dotc.core.Contexts.Context +import dotty.tools.dotc.core.quoted.PickledQuotes import dotty.tools.dotc.printing.RefinedPrinter import scala.quoted.Expr import scala.runtime.BoxedUnit -import scala.quoted.Exprs.LiftedExpr +import scala.quoted.Exprs.{LiftedExpr, TreeExpr} import scala.runtime.quoted._ /** Default runners for quoted expressions */ @@ -23,8 +25,12 @@ object Toolbox { ): Toolbox[T] = new Toolbox[T] { def run(expr: Expr[T]): T = expr match { - case expr: LiftedExpr[T] => expr.value - case _ => new QuoteDriver().run(expr, runSettings) + case expr: LiftedExpr[T] => + expr.value + case expr: TreeExpr[Tree] @unchecked => + new QuoteDriver().run(expr.pickled, runSettings) + case _ => + new QuoteDriver().run(expr, runSettings) } def show(expr: Expr[T]): String = expr match { diff --git a/compiler/src/dotty/tools/dotc/transform/Splicer.scala b/compiler/src/dotty/tools/dotc/transform/Splicer.scala index 3877c36cdb59..a19681812889 100644 --- a/compiler/src/dotty/tools/dotc/transform/Splicer.scala +++ b/compiler/src/dotty/tools/dotc/transform/Splicer.scala @@ -60,7 +60,8 @@ object Splicer { case (arg, tp) => assert(!tp.hasAnnotation(defn.InlineParamAnnot)) // Replace argument by its binding - new scala.quoted.Exprs.TreeExpr(bindMap.getOrElse(arg, arg)) + val arg1 = bindMap.getOrElse(arg, arg) + new scala.quoted.Exprs.TreeExpr(arg1, PickledQuotes.pickleExpr(arg1)) } args1 ::: liftArgs(tp.resType, args.tail) case tp: PolyType => diff --git a/library/src/scala/quoted/Expr.scala b/library/src/scala/quoted/Expr.scala index b59a62ce6a97..d2c9e4fb14cb 100644 --- a/library/src/scala/quoted/Expr.scala +++ b/library/src/scala/quoted/Expr.scala @@ -37,7 +37,8 @@ object Exprs { } /** An Expr backed by a tree. Only the current compiler trees are allowed. */ - final class TreeExpr[Tree](val tree: Tree) extends quoted.Expr[Any] { + final class TreeExpr[Tree](val tree: Tree, pickle: => Expr[_]) extends quoted.Expr[Any] { + def pickled[T]: Expr[T] = pickle.asInstanceOf[Expr[T]] override def toString: String = s"Expr()" } diff --git a/tests/run/quote-run-in-macro-1.check b/tests/run/quote-run-in-macro-1.check new file mode 100644 index 000000000000..2f1b638ff548 --- /dev/null +++ b/tests/run/quote-run-in-macro-1.check @@ -0,0 +1,2 @@ +1 +4 diff --git a/tests/run/quote-run-in-macro-1/quoted_1.scala b/tests/run/quote-run-in-macro-1/quoted_1.scala new file mode 100644 index 000000000000..34d795a53b70 --- /dev/null +++ b/tests/run/quote-run-in-macro-1/quoted_1.scala @@ -0,0 +1,10 @@ +import scala.quoted._ + +import dotty.tools.dotc.quoted.Toolbox._ + +object Macros { + inline def foo(i: Int): Int = ~{ + val y: Int = ('(i)).run + y.toExpr + } +} diff --git a/tests/run/quote-run-in-macro-1/quoted_2.scala b/tests/run/quote-run-in-macro-1/quoted_2.scala new file mode 100644 index 000000000000..e3197b8842a5 --- /dev/null +++ b/tests/run/quote-run-in-macro-1/quoted_2.scala @@ -0,0 +1,7 @@ +import Macros._ +object Test { + def main(args: Array[String]): Unit = { + println(foo(1)) + println(foo(1 + 3)) + } +} From 3b0128afa7cd80d36b3d1d501d5e8b7d4252159b Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Fri, 4 May 2018 16:06:34 +0200 Subject: [PATCH 2/3] Handle failures to pickle macro parameters --- .../tools/dotc/core/quoted/PickledQuotes.scala | 13 +++++++++++++ .../tools/dotc/transform/ReifyQuotes.scala | 3 ++- .../src/scala/quoted/FreeVariableError.scala | 3 +++ tests/neg/quote-run-in-macro-1/quoted_1.scala | 10 ++++++++++ tests/neg/quote-run-in-macro-1/quoted_2.scala | 7 +++++++ tests/run/quote-run-in-macro-1.check | 1 + tests/run/quote-run-in-macro-1/quoted_1.scala | 2 +- tests/run/quote-run-in-macro-1/quoted_2.scala | 5 +++++ tests/run/quote-run-in-macro-2.check | 3 +++ tests/run/quote-run-in-macro-2/quoted_1.scala | 18 ++++++++++++++++++ tests/run/quote-run-in-macro-2/quoted_2.scala | 9 +++++++++ 11 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 library/src/scala/quoted/FreeVariableError.scala create mode 100644 tests/neg/quote-run-in-macro-1/quoted_1.scala create mode 100644 tests/neg/quote-run-in-macro-1/quoted_2.scala create mode 100644 tests/run/quote-run-in-macro-2.check create mode 100644 tests/run/quote-run-in-macro-2/quoted_1.scala create mode 100644 tests/run/quote-run-in-macro-2/quoted_2.scala diff --git a/compiler/src/dotty/tools/dotc/core/quoted/PickledQuotes.scala b/compiler/src/dotty/tools/dotc/core/quoted/PickledQuotes.scala index f7d38c39e8bb..3ebbc05ce678 100644 --- a/compiler/src/dotty/tools/dotc/core/quoted/PickledQuotes.scala +++ b/compiler/src/dotty/tools/dotc/core/quoted/PickledQuotes.scala @@ -24,6 +24,19 @@ object PickledQuotes { /** Pickle the tree of the a quoted.Expr */ def pickleExpr(tree: Tree)(implicit ctx: Context): scala.quoted.Expr[Any] = { + // Check that there are no free variables + new TreeTraverser { + private val definedHere = scala.collection.mutable.Set.empty[Symbol] + def traverse(tree: tpd.Tree)(implicit ctx: Context): Unit = tree match { + case tree: Ident if tree.symbol.exists && !definedHere(tree.symbol) => + throw new scala.quoted.FreeVariableError(tree.name.toString) + case tree: DefTree => + definedHere += tree.symbol + traverseChildren(tree) + case _ => + traverseChildren(tree) + } + }.traverse(tree) val pickled = pickleQuote(tree) scala.runtime.quoted.Unpickler.unpickleExpr(pickled, Nil) } diff --git a/compiler/src/dotty/tools/dotc/transform/ReifyQuotes.scala b/compiler/src/dotty/tools/dotc/transform/ReifyQuotes.scala index ae9d63c3c222..cd177a907ec0 100644 --- a/compiler/src/dotty/tools/dotc/transform/ReifyQuotes.scala +++ b/compiler/src/dotty/tools/dotc/transform/ReifyQuotes.scala @@ -560,7 +560,8 @@ class ReifyQuotes extends MacroTransformWithImplicits with InfoTransformer { // see PostTyper `case Inlined(...) =>` for description of the simplification val call2 = Ident(call.symbol.topLevelClass.typeRef).withPos(call.pos) val spliced = Splicer.splice(body, call, bindings, tree.pos, macroClassLoader).withPos(tree.pos) - transform(cpy.Inlined(tree)(call2, bindings, spliced)) + if (ctx.reporter.hasErrors) EmptyTree + else transform(cpy.Inlined(tree)(call2, bindings, spliced)) } else super.transform(tree) diff --git a/library/src/scala/quoted/FreeVariableError.scala b/library/src/scala/quoted/FreeVariableError.scala new file mode 100644 index 000000000000..a13485214291 --- /dev/null +++ b/library/src/scala/quoted/FreeVariableError.scala @@ -0,0 +1,3 @@ +package scala.quoted + +class FreeVariableError(val name: String) extends QuoteError("Free variable " + name + " could not be handled") diff --git a/tests/neg/quote-run-in-macro-1/quoted_1.scala b/tests/neg/quote-run-in-macro-1/quoted_1.scala new file mode 100644 index 000000000000..f54a4fe00888 --- /dev/null +++ b/tests/neg/quote-run-in-macro-1/quoted_1.scala @@ -0,0 +1,10 @@ +import scala.quoted._ + +import dotty.tools.dotc.quoted.Toolbox._ + +object Macros { + inline def foo(i: => Int): Int = ~{ + val y: Int = ('(i)).run + y.toExpr + } +} diff --git a/tests/neg/quote-run-in-macro-1/quoted_2.scala b/tests/neg/quote-run-in-macro-1/quoted_2.scala new file mode 100644 index 000000000000..9127d3d6be5d --- /dev/null +++ b/tests/neg/quote-run-in-macro-1/quoted_2.scala @@ -0,0 +1,7 @@ +import Macros._ +object Test { + def main(args: Array[String]): Unit = { + val x = 3 + println(foo(x)) // error + } +} diff --git a/tests/run/quote-run-in-macro-1.check b/tests/run/quote-run-in-macro-1.check index 2f1b638ff548..5da50a4e6e5b 100644 --- a/tests/run/quote-run-in-macro-1.check +++ b/tests/run/quote-run-in-macro-1.check @@ -1,2 +1,3 @@ 1 4 +5 diff --git a/tests/run/quote-run-in-macro-1/quoted_1.scala b/tests/run/quote-run-in-macro-1/quoted_1.scala index 34d795a53b70..f54a4fe00888 100644 --- a/tests/run/quote-run-in-macro-1/quoted_1.scala +++ b/tests/run/quote-run-in-macro-1/quoted_1.scala @@ -3,7 +3,7 @@ import scala.quoted._ import dotty.tools.dotc.quoted.Toolbox._ object Macros { - inline def foo(i: Int): Int = ~{ + inline def foo(i: => Int): Int = ~{ val y: Int = ('(i)).run y.toExpr } diff --git a/tests/run/quote-run-in-macro-1/quoted_2.scala b/tests/run/quote-run-in-macro-1/quoted_2.scala index e3197b8842a5..8d1f280a2bab 100644 --- a/tests/run/quote-run-in-macro-1/quoted_2.scala +++ b/tests/run/quote-run-in-macro-1/quoted_2.scala @@ -3,5 +3,10 @@ object Test { def main(args: Array[String]): Unit = { println(foo(1)) println(foo(1 + 3)) + val x = 3 + println(foo { + val x = 5 + x + }) } } diff --git a/tests/run/quote-run-in-macro-2.check b/tests/run/quote-run-in-macro-2.check new file mode 100644 index 000000000000..3c3aff685b58 --- /dev/null +++ b/tests/run/quote-run-in-macro-2.check @@ -0,0 +1,3 @@ +"> 3" +"> 12" +(x: scala.Int) => "> ".+(y).apply(3) diff --git a/tests/run/quote-run-in-macro-2/quoted_1.scala b/tests/run/quote-run-in-macro-2/quoted_1.scala new file mode 100644 index 000000000000..96cc93c8f1b2 --- /dev/null +++ b/tests/run/quote-run-in-macro-2/quoted_1.scala @@ -0,0 +1,18 @@ +import scala.quoted._ + +import dotty.tools.dotc.quoted.Toolbox._ + +object Macros { + inline def foo(f: => Int => String): String = ~bar('(f)) + def bar(f: Expr[Int => String]): Expr[String] = { + try { + val y: Int => String = f.run + val res = y(3).toExpr // evaluate at compile time + res.show.toExpr + } catch { + case ex: scala.quoted.FreeVariableError => + val res = ('((~f)(3))) // evaluate at run time + res.show.toExpr + } + } +} diff --git a/tests/run/quote-run-in-macro-2/quoted_2.scala b/tests/run/quote-run-in-macro-2/quoted_2.scala new file mode 100644 index 000000000000..f9e4d32c83e6 --- /dev/null +++ b/tests/run/quote-run-in-macro-2/quoted_2.scala @@ -0,0 +1,9 @@ +import Macros._ +object Test { + def main(args: Array[String]): Unit = { + println(foo(x => "> " + x)) + println(foo(x => "> " + 4 * x)) + val y = 9 + println(foo(x => "> " + y)) + } +} From 68e32ab018ab7710d6f39014d22eca4688e159f3 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Tue, 8 May 2018 13:42:59 +0200 Subject: [PATCH 3/3] Add documentation --- .../tools/dotc/core/quoted/PickledQuotes.scala | 2 +- library/src/scala/quoted/Expr.scala | 15 ++++++++++++++- library/src/scala/quoted/FreeVariableError.scala | 5 +++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/quoted/PickledQuotes.scala b/compiler/src/dotty/tools/dotc/core/quoted/PickledQuotes.scala index 3ebbc05ce678..625c2d1b34b9 100644 --- a/compiler/src/dotty/tools/dotc/core/quoted/PickledQuotes.scala +++ b/compiler/src/dotty/tools/dotc/core/quoted/PickledQuotes.scala @@ -22,7 +22,7 @@ import scala.reflect.ClassTag object PickledQuotes { import tpd._ - /** Pickle the tree of the a quoted.Expr */ + /** Pickle the tree of the quoted.Expr */ def pickleExpr(tree: Tree)(implicit ctx: Context): scala.quoted.Expr[Any] = { // Check that there are no free variables new TreeTraverser { diff --git a/library/src/scala/quoted/Expr.scala b/library/src/scala/quoted/Expr.scala index d2c9e4fb14cb..7e9fd0e9d9f1 100644 --- a/library/src/scala/quoted/Expr.scala +++ b/library/src/scala/quoted/Expr.scala @@ -5,7 +5,14 @@ import scala.runtime.quoted.Unpickler.Pickled sealed abstract class Expr[T] { final def unary_~ : T = throw new Error("~ should have been compiled away") + + /** Evaluate the contents of this expression and return the result. + * + * May throw a FreeVariableError on expressions that came from an inline macro. + */ final def run(implicit toolbox: Toolbox[T]): T = toolbox.run(this) + + /** Show a source code like representation of this expression */ final def show(implicit toolbox: Toolbox[T]): String = toolbox.show(this) } @@ -36,7 +43,13 @@ object Exprs { override def toString: String = s"Expr($value)" } - /** An Expr backed by a tree. Only the current compiler trees are allowed. */ + /** An Expr backed by a tree. Only the current compiler trees are allowed. + * + * These expressions are used for arguments of inline macros. They contain and actual tree + * from the program that is being expanded by the macro. + * + * May contain references to code defined outside this Expr instance. + */ final class TreeExpr[Tree](val tree: Tree, pickle: => Expr[_]) extends quoted.Expr[Any] { def pickled[T]: Expr[T] = pickle.asInstanceOf[Expr[T]] override def toString: String = s"Expr()" diff --git a/library/src/scala/quoted/FreeVariableError.scala b/library/src/scala/quoted/FreeVariableError.scala index a13485214291..4dac11ddbc6f 100644 --- a/library/src/scala/quoted/FreeVariableError.scala +++ b/library/src/scala/quoted/FreeVariableError.scala @@ -1,3 +1,8 @@ package scala.quoted +/** Exception is thrown when trying to evaluate the contents of an expression + * that has free variables (i.e. has some local references to code that is + * outside of the expression). This can happen on Expr that passed as arguments to + * an inline macro. + */ class FreeVariableError(val name: String) extends QuoteError("Free variable " + name + " could not be handled")