Skip to content

Fix #4456: Pickle quote before compiling #4457

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 13, 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
21 changes: 20 additions & 1 deletion compiler/src/dotty/tools/dotc/core/quoted/PickledQuotes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,26 @@ import scala.reflect.ClassTag
object PickledQuotes {
import tpd._

/** Pickle the quote into strings */
/** 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)
Copy link
Contributor

Choose a reason for hiding this comment

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

The Nil there makes me wonder what if this fix applies to non-inline functions.

Copy link
Contributor

Choose a reason for hiding this comment

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

// ignore, I have just went through the second unit test and you do what I had in mind.

}

/** 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 {
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 9 additions & 3 deletions compiler/src/dotty/tools/dotc/quoted/Toolbox.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/ReifyQuotes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/Splicer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand Down
18 changes: 16 additions & 2 deletions library/src/scala/quoted/Expr.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -36,8 +43,15 @@ object Exprs {
override def toString: String = s"Expr($value)"
}

/** An Expr backed by a tree. Only the current compiler trees are allowed. */
final class TreeExpr[Tree](val tree: Tree) extends quoted.Expr[Any] {
/** 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(<raw>)"
}

Expand Down
8 changes: 8 additions & 0 deletions library/src/scala/quoted/FreeVariableError.scala
Original file line number Diff line number Diff line change
@@ -0,0 +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")
10 changes: 10 additions & 0 deletions tests/neg/quote-run-in-macro-1/quoted_1.scala
Original file line number Diff line number Diff line change
@@ -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
}
}
7 changes: 7 additions & 0 deletions tests/neg/quote-run-in-macro-1/quoted_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import Macros._
object Test {
def main(args: Array[String]): Unit = {
val x = 3
println(foo(x)) // error
}
}
3 changes: 3 additions & 0 deletions tests/run/quote-run-in-macro-1.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
1
4
5
10 changes: 10 additions & 0 deletions tests/run/quote-run-in-macro-1/quoted_1.scala
Original file line number Diff line number Diff line change
@@ -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
}
}
12 changes: 12 additions & 0 deletions tests/run/quote-run-in-macro-1/quoted_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import Macros._
object Test {
def main(args: Array[String]): Unit = {
println(foo(1))
println(foo(1 + 3))
val x = 3
println(foo {
val x = 5
x
})
}
}
3 changes: 3 additions & 0 deletions tests/run/quote-run-in-macro-2.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
"> 3"
"> 12"
(x: scala.Int) => "> ".+(y).apply(3)
18 changes: 18 additions & 0 deletions tests/run/quote-run-in-macro-2/quoted_1.scala
Original file line number Diff line number Diff line change
@@ -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
}
}
}
9 changes: 9 additions & 0 deletions tests/run/quote-run-in-macro-2/quoted_2.scala
Original file line number Diff line number Diff line change
@@ -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))
}
}