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 2 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 a quoted.Expr */
Copy link
Contributor

Choose a reason for hiding this comment

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

remove the a :P

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
3 changes: 2 additions & 1 deletion library/src/scala/quoted/Expr.scala
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ object Exprs {
}

/** An Expr backed by a tree. Only the current compiler trees are allowed. */
Copy link
Contributor

Choose a reason for hiding this comment

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

Clarify what we mean by "current compiler"?

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(<raw>)"
}

Expand Down
3 changes: 3 additions & 0 deletions library/src/scala/quoted/FreeVariableError.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package scala.quoted

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))
}
}