From f8e337dc1ac749684f6d4105fcabd7c44f22e32a Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Thu, 24 May 2018 11:52:46 +0200 Subject: [PATCH 1/2] Do not allow running Expr that came as macro arguments It is too hard to ensure that the contents of the tree can be pickled. It would be posible but the current ad-hoc solution is too brittle and has no good usecases. --- .../tools/dotc/core/quoted/PickledQuotes.scala | 15 +-------------- .../tools/dotc/core/tasty/TreeUnpickler.scala | 4 ++-- .../src/dotty/tools/dotc/quoted/Toolbox.scala | 6 +++--- .../dotty/tools/dotc/transform/Splicer.scala | 2 +- library/src/scala/quoted/Expr.scala | 7 +++---- .../src/scala/quoted/FreeVariableError.scala | 8 -------- .../quote-run-in-macro-2}/quoted_1.scala | 0 .../quote-run-in-macro-2}/quoted_2.scala | 6 +++--- tests/run/quote-run-in-macro-1.check | 3 --- 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 --------- 12 files changed, 13 insertions(+), 68 deletions(-) delete mode 100644 library/src/scala/quoted/FreeVariableError.scala rename tests/{run/quote-run-in-macro-1 => neg/quote-run-in-macro-2}/quoted_1.scala (100%) rename tests/{run/quote-run-in-macro-1 => neg/quote-run-in-macro-2}/quoted_2.scala (57%) delete mode 100644 tests/run/quote-run-in-macro-1.check delete mode 100644 tests/run/quote-run-in-macro-2.check delete mode 100644 tests/run/quote-run-in-macro-2/quoted_1.scala delete 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 629edec591d6..faf50b5848a6 100644 --- a/compiler/src/dotty/tools/dotc/core/quoted/PickledQuotes.scala +++ b/compiler/src/dotty/tools/dotc/core/quoted/PickledQuotes.scala @@ -23,19 +23,6 @@ object PickledQuotes { /** 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 { - 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) } @@ -64,7 +51,7 @@ object PickledQuotes { case value: Class[_] => ref(defn.Predef_classOf).appliedToType(classToType(value)) case value => Literal(Constant(value)) } - case expr: TreeExpr[Tree] @unchecked => expr.tree + case expr: TastyTreeExpr[Tree] @unchecked => expr.tree case expr: FunctionAppliedTo[_, _] => functionAppliedTo(quotedExprToTree(expr.f), quotedExprToTree(expr.x)) } diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index ce0edf52a31c..1a8fd49a5d99 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -22,7 +22,7 @@ import config.Config import core.quoted.PickledQuotes import scala.quoted import scala.quoted.Types.TreeType -import scala.quoted.Exprs.TreeExpr +import scala.quoted.Exprs.TastyTreeExpr /** Unpickler for typed trees * @param reader the reader from which to unpickle @@ -1150,7 +1150,7 @@ class TreeUnpickler(reader: TastyReader, val args = until(end)(readTerm()) val splice = splices(idx) def wrap(arg: Tree) = - if (arg.isTerm) new TreeExpr(arg, PickledQuotes.pickleExpr(arg)) + if (arg.isTerm) new TastyTreeExpr(arg) else new TreeType(arg) val reifiedArgs = args.map(wrap) if (isType) { diff --git a/compiler/src/dotty/tools/dotc/quoted/Toolbox.scala b/compiler/src/dotty/tools/dotc/quoted/Toolbox.scala index 59ad7a5070be..dea8d12fc097 100644 --- a/compiler/src/dotty/tools/dotc/quoted/Toolbox.scala +++ b/compiler/src/dotty/tools/dotc/quoted/Toolbox.scala @@ -6,7 +6,7 @@ import dotty.tools.dotc.printing.RefinedPrinter import scala.quoted.Expr import scala.runtime.BoxedUnit -import scala.quoted.Exprs.{LiftedExpr, TreeExpr} +import scala.quoted.Exprs.{LiftedExpr, TastyTreeExpr} import scala.runtime.quoted._ /** Default runners for quoted expressions */ @@ -24,8 +24,8 @@ object Toolbox { def run(expr: Expr[T]): T = expr match { case expr: LiftedExpr[T] => expr.value - case expr: TreeExpr[Tree] @unchecked => - new QuoteDriver().run(expr.pickled, runSettings) + case expr: TastyTreeExpr[Tree] @unchecked => + throw new scala.quoted.QuoteError("Can't run and scala.quoted.Expr coming from a macro argument") case _ => new QuoteDriver().run(expr, runSettings) } diff --git a/compiler/src/dotty/tools/dotc/transform/Splicer.scala b/compiler/src/dotty/tools/dotc/transform/Splicer.scala index 9435843dd7de..90df1b9e2428 100644 --- a/compiler/src/dotty/tools/dotc/transform/Splicer.scala +++ b/compiler/src/dotty/tools/dotc/transform/Splicer.scala @@ -67,7 +67,7 @@ object Splicer { assert(!tp.hasAnnotation(defn.InlineParamAnnot)) // Replace argument by its binding val arg1 = bindMap.getOrElse(arg, arg) - new scala.quoted.Exprs.TreeExpr(arg1, PickledQuotes.pickleExpr(arg1)) + new scala.quoted.Exprs.TastyTreeExpr(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 7e9fd0e9d9f1..41c2cdb3a4d6 100644 --- a/library/src/scala/quoted/Expr.scala +++ b/library/src/scala/quoted/Expr.scala @@ -33,7 +33,7 @@ object Expr { object Exprs { /** An Expr backed by a pickled TASTY tree */ final class TastyExpr[T](val tasty: Pickled, val args: Seq[Any]) extends Expr[T] { - override def toString: String = s"Expr()" + override def toString: String = s"Expr()" } /** An Expr backed by a lifted value. @@ -50,9 +50,8 @@ object Exprs { * * 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()" + final class TastyTreeExpr[Tree](val tree: Tree) extends quoted.Expr[Any] { + override def toString: String = s"Expr()" } /** An Expr representing `'{(~f).apply(~x)}` but it is beta-reduced when the closure is known */ diff --git a/library/src/scala/quoted/FreeVariableError.scala b/library/src/scala/quoted/FreeVariableError.scala deleted file mode 100644 index 4dac11ddbc6f..000000000000 --- a/library/src/scala/quoted/FreeVariableError.scala +++ /dev/null @@ -1,8 +0,0 @@ -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") diff --git a/tests/run/quote-run-in-macro-1/quoted_1.scala b/tests/neg/quote-run-in-macro-2/quoted_1.scala similarity index 100% rename from tests/run/quote-run-in-macro-1/quoted_1.scala rename to tests/neg/quote-run-in-macro-2/quoted_1.scala diff --git a/tests/run/quote-run-in-macro-1/quoted_2.scala b/tests/neg/quote-run-in-macro-2/quoted_2.scala similarity index 57% rename from tests/run/quote-run-in-macro-1/quoted_2.scala rename to tests/neg/quote-run-in-macro-2/quoted_2.scala index 8d1f280a2bab..02a0925642f4 100644 --- a/tests/run/quote-run-in-macro-1/quoted_2.scala +++ b/tests/neg/quote-run-in-macro-2/quoted_2.scala @@ -1,10 +1,10 @@ import Macros._ object Test { def main(args: Array[String]): Unit = { - println(foo(1)) - println(foo(1 + 3)) + println(foo(1)) // error + println(foo(1 + 3)) // error val x = 3 - println(foo { + println(foo { // error val x = 5 x }) diff --git a/tests/run/quote-run-in-macro-1.check b/tests/run/quote-run-in-macro-1.check deleted file mode 100644 index 5da50a4e6e5b..000000000000 --- a/tests/run/quote-run-in-macro-1.check +++ /dev/null @@ -1,3 +0,0 @@ -1 -4 -5 diff --git a/tests/run/quote-run-in-macro-2.check b/tests/run/quote-run-in-macro-2.check deleted file mode 100644 index 3c3aff685b58..000000000000 --- a/tests/run/quote-run-in-macro-2.check +++ /dev/null @@ -1,3 +0,0 @@ -"> 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 deleted file mode 100644 index 96cc93c8f1b2..000000000000 --- a/tests/run/quote-run-in-macro-2/quoted_1.scala +++ /dev/null @@ -1,18 +0,0 @@ -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 deleted file mode 100644 index f9e4d32c83e6..000000000000 --- a/tests/run/quote-run-in-macro-2/quoted_2.scala +++ /dev/null @@ -1,9 +0,0 @@ -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 4d4d592a434fdcdba03f0e68e09edf7cd1ba329d Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Thu, 24 May 2018 15:04:23 +0200 Subject: [PATCH 2/2] Update docs and change exception --- compiler/src/dotty/tools/dotc/quoted/Toolbox.scala | 2 +- library/src/scala/quoted/Expr.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/quoted/Toolbox.scala b/compiler/src/dotty/tools/dotc/quoted/Toolbox.scala index dea8d12fc097..9296aa31a680 100644 --- a/compiler/src/dotty/tools/dotc/quoted/Toolbox.scala +++ b/compiler/src/dotty/tools/dotc/quoted/Toolbox.scala @@ -25,7 +25,7 @@ object Toolbox { case expr: LiftedExpr[T] => expr.value case expr: TastyTreeExpr[Tree] @unchecked => - throw new scala.quoted.QuoteError("Can't run and scala.quoted.Expr coming from a macro argument") + throw new Exception("Cannot call `Expr.run` on an `Expr` that comes from an inline macro argument.") case _ => new QuoteDriver().run(expr, runSettings) } diff --git a/library/src/scala/quoted/Expr.scala b/library/src/scala/quoted/Expr.scala index 41c2cdb3a4d6..f6fe2d478389 100644 --- a/library/src/scala/quoted/Expr.scala +++ b/library/src/scala/quoted/Expr.scala @@ -48,7 +48,7 @@ object Exprs { * 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. + * May contain references to code defined outside this TastyTreeExpr instance. */ final class TastyTreeExpr[Tree](val tree: Tree) extends quoted.Expr[Any] { override def toString: String = s"Expr()"