Skip to content

Fix #7142: Detect scope extrusions in macros and run #8000

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 2 commits into from
Jan 23, 2020
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
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/core/StdNames.scala
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,6 @@ object StdNames {
val lang: N = "lang"
val length: N = "length"
val lengthCompare: N = "lengthCompare"
val `macro` : N = "macro"
val macroThis : N = "_this"
val macroContext : N = "c"
val main: N = "main"
Expand Down
44 changes: 42 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/Splicer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,17 @@ object Splicer {
*
* See: `Staging`
*/
def splice(tree: Tree, pos: SourcePosition, classLoader: ClassLoader)(implicit ctx: Context): Tree = tree match {
def splice(tree: Tree, pos: SourcePosition, classLoader: ClassLoader)(given ctx: Context): Tree = tree match {
case Quoted(quotedTree) => quotedTree
case _ =>
val interpreter = new Interpreter(pos, classLoader)
val macroOwner = ctx.newSymbol(ctx.owner, NameKinds.UniqueName.fresh(nme.MACROkw), Synthetic, defn.AnyType, coord = tree.span)
try {
given Context = ctx.withOwner(macroOwner)
// Some parts of the macro are evaluated during the unpickling performed in quotedExprToTree
val interpretedExpr = interpreter.interpret[scala.quoted.QuoteContext => scala.quoted.Expr[Any]](tree)
interpretedExpr.fold(tree)(macroClosure => PickledQuotes.quotedExprToTree(macroClosure(QuoteContext())))
val interpretedTree = interpretedExpr.fold(tree)(macroClosure => PickledQuotes.quotedExprToTree(macroClosure(QuoteContext())))
checkEscapedVariables(interpretedTree, macroOwner).changeOwner(macroOwner, ctx.owner)
}
catch {
case ex: CompilationUnit.SuspendException =>
Expand All @@ -63,6 +66,43 @@ object Splicer {
}
}

/** Checks that no symbol that whas generated within the macro expansion has an out of scope reference */
def checkEscapedVariables(tree: Tree, expansionOwner: Symbol)(given ctx: Context): tree.type =
new TreeTraverser {
private[this] var locals = Set.empty[Symbol]
private def markSymbol(sym: Symbol)(implicit ctx: Context): Unit =
locals = locals + sym
private def markDef(tree: Tree)(implicit ctx: Context): Unit = tree match {
case tree: DefTree => markSymbol(tree.symbol)
case _ =>
}
def traverse(tree: Tree)(given ctx: Context): Unit =
def traverseOver(lastEntered: Set[Symbol]) =
try traverseChildren(tree)
finally locals = lastEntered
tree match
case tree: Ident if isEscapedVariable(tree.symbol) =>
val sym = tree.symbol
ctx.error(em"While expanding a macro, a reference to $sym was used outside the scope where it was defined", tree.sourcePos)
case Block(stats, _) =>
val last = locals
stats.foreach(markDef)
traverseOver(last)
case CaseDef(pat, guard, body) =>
val last = locals
tpd.patVars(pat).foreach(markSymbol)
traverseOver(last)
case _ =>
markDef(tree)
traverseChildren(tree)
private def isEscapedVariable(sym: Symbol)(given ctx: Context): Boolean =
sym.exists && !sym.is(Package)
&& sym.owner.ownersIterator.contains(expansionOwner) // symbol was generated within the macro expansion
&& !locals.contains(sym) // symbol is not in current scope
}.traverse(tree)
tree


/** Check that the Tree can be spliced. `${'{xyz}}` becomes `xyz`
* and for `$xyz` the tree of `xyz` is interpreted for which the
* resulting expression is returned as a `Tree`
Expand Down
25 changes: 9 additions & 16 deletions compiler/src/dotty/tools/dotc/transform/TreeMapWithStages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,16 @@ abstract class TreeMapWithStages(@constructorOnly ictx: Context) extends TreeMap
/** Localy defined symbols seen so far by `StagingTransformer.transform` */
protected def localSymbols: List[Symbol] = enteredSyms

/** Enter staging level of symbol defined by `tree`, if applicable. */
private def markSymbol(sym: Symbol)(implicit ctx: Context): Unit =
if ((sym.isClass || sym.maybeOwner.isTerm) && !levelOfMap.contains(sym)) {
levelOfMap(sym) = level
enteredSyms = sym :: enteredSyms
}

/** Enter staging level of symbol defined by `tree`, if applicable. */
private def markDef(tree: Tree)(implicit ctx: Context): Unit = tree match {
case tree: DefTree =>
val sym = tree.symbol
if ((sym.isClass || sym.maybeOwner.isTerm) && !levelOfMap.contains(sym)) {
levelOfMap(sym) = level
enteredSyms = sym :: enteredSyms
}
case tree: DefTree => markSymbol(tree.symbol)
case _ =>
}

Expand Down Expand Up @@ -106,16 +108,7 @@ abstract class TreeMapWithStages(@constructorOnly ictx: Context) extends TreeMap

case CaseDef(pat, guard, body) =>
val last = enteredSyms
// mark all bindings
new TreeTraverser {
def traverse(tree: Tree)(implicit ctx: Context): Unit = tree match {
case Quoted(t) => traverse(t)(quoteContext)
case Splice(t) => traverse(t)(spliceContext)
case _ =>
markDef(tree)
traverseChildren(tree)
}
}.traverse(pat)
tpd.patVars(pat).foreach(markSymbol)
mapOverTree(last)

case _: Import =>
Expand Down
7 changes: 4 additions & 3 deletions compiler/src/dotty/tools/dotc/typer/Inliner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1273,7 +1273,6 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
private def expandMacro(body: Tree, span: Span)(implicit ctx: Context) = {
assert(level == 0)
val inlinedFrom = enclosingInlineds.last
val ctx1 = tastyreflect.MacroExpansion.context(inlinedFrom)
val dependencies = macroDependencies(body)

if dependencies.nonEmpty && !ctx.reporter.errorsReported then
Expand All @@ -1284,8 +1283,10 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
ctx.echo(i"suspension triggered by macro call to ${sym.showLocated} in ${sym.associatedFile}", call.sourcePos)
ctx.compilationUnit.suspend() // this throws a SuspendException

val evaluatedSplice = Splicer.splice(body, inlinedFrom.sourcePos, MacroClassLoader.fromContext)(ctx1)

val evaluatedSplice = {
given Context = tastyreflect.MacroExpansion.context(inlinedFrom)(given ctx)
Splicer.splice(body, inlinedFrom.sourcePos, MacroClassLoader.fromContext)
}
val inlinedNormailizer = new TreeMap {
override def transform(tree: tpd.Tree)(implicit ctx: Context): tpd.Tree = tree match {
case Inlined(EmptyTree, Nil, expr) if enclosingInlineds.isEmpty => transform(expr)
Expand Down
9 changes: 7 additions & 2 deletions staging/src/scala/quoted/staging/QuoteCompiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import dotty.tools.dotc.core.Symbols.defn
import dotty.tools.dotc.core.Types.ExprType
import dotty.tools.dotc.core.quoted.PickledQuotes
import dotty.tools.dotc.tastyreflect.ReflectionImpl
import dotty.tools.dotc.transform.Splicer.checkEscapedVariables
import dotty.tools.dotc.transform.ReifyQuotes
import dotty.tools.dotc.util.Spans.Span
import dotty.tools.dotc.util.SourceFile
Expand Down Expand Up @@ -67,8 +68,12 @@ private class QuoteCompiler extends Compiler {
cls.enter(unitCtx.newDefaultConstructor(cls), EmptyScope)
val meth = unitCtx.newSymbol(cls, nme.apply, Method, ExprType(defn.AnyType), coord = pos).entered

val qctx = dotty.tools.dotc.quoted.QuoteContext()
val quoted = PickledQuotes.quotedExprToTree(exprUnit.exprBuilder.apply(qctx))(unitCtx.withOwner(meth))
val quoted = {
given Context = unitCtx.withOwner(meth)
val qctx = dotty.tools.dotc.quoted.QuoteContext()
val quoted = PickledQuotes.quotedExprToTree(exprUnit.exprBuilder.apply(qctx))
checkEscapedVariables(quoted, meth)
}

getLiteral(quoted) match {
case Some(value) =>
Expand Down
9 changes: 9 additions & 0 deletions tests/neg-macros/i7142/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package macros
import scala.quoted._

def oops(given QuoteContext) = {
var v = '{0};
val q = '{ (x: Int) => ${ v = '{x}; v } }
'{$q($v)}
}
inline def test = ${oops}
4 changes: 4 additions & 0 deletions tests/neg-macros/i7142/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

object Test {
macros.test // error
}
8 changes: 8 additions & 0 deletions tests/neg-macros/i7142b/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package macros
import scala.quoted._
import scala.util.control.NonLocalReturns._

def oops(given QuoteContext): Expr[Int] =
returning('{ { (x: Int) => ${ throwReturn('x) }} apply 0 })

inline def test = ${oops}
4 changes: 4 additions & 0 deletions tests/neg-macros/i7142b/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

object Test {
macros.test // error
}
9 changes: 9 additions & 0 deletions tests/neg-macros/i7142c/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package macros
import scala.quoted._

def oops(given QuoteContext) = {
var v = '{0};
val q = '{ val x: Int = 8; ${ v = '{x}; v } }
v
}
inline def test = ${oops}
4 changes: 4 additions & 0 deletions tests/neg-macros/i7142c/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

object Test {
macros.test // error
}
9 changes: 9 additions & 0 deletions tests/neg-macros/i7142d/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package macros
import scala.quoted._

def oops(given QuoteContext) = {
var v = '{0};
val q = '{ ??? match { case x => ${ v = '{x}; v } } }
v
}
inline def test = ${oops}
4 changes: 4 additions & 0 deletions tests/neg-macros/i7142d/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

object Test {
macros.test // error
}
9 changes: 9 additions & 0 deletions tests/neg-macros/i7142e/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package macros
import scala.quoted._

def oops(given QuoteContext) = {
var v = '{0};
val q = '{ def x: Int = 8; ${ v = '{x}; v } }
v
}
inline def test = ${oops}
4 changes: 4 additions & 0 deletions tests/neg-macros/i7142e/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

object Test {
macros.test // error
}
9 changes: 9 additions & 0 deletions tests/neg-macros/i7142f/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package macros
import scala.quoted._

def oops(given QuoteContext) = {
var v = '{0};
val q = '{ def f(x: Int): Int = ${ v = '{x}; v } }
v
}
inline def test = ${oops}
4 changes: 4 additions & 0 deletions tests/neg-macros/i7142f/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

object Test {
macros.test // error
}
9 changes: 9 additions & 0 deletions tests/neg-macros/i7142g/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package macros
import scala.quoted._

def oops(given QuoteContext) = {
var v = '{};
val q = '{ var x: Int = 8; ${ v = '{x = 9}; v } }
v
}
inline def test = ${oops}
4 changes: 4 additions & 0 deletions tests/neg-macros/i7142g/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

object Test {
macros.test // error
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
foo
DefDef("main", Nil, List(List(ValDef("args", Inferred(), None))), Inferred(), None)
ValDef("macro$1", Inferred(), None)

bar
DefDef("foo", Nil, Nil, Inferred(), None)
Expand All @@ -8,7 +8,7 @@ bar2
DefDef("foo", Nil, Nil, Inferred(), None)

foo2
DefDef("main", Nil, List(List(ValDef("args", Inferred(), None))), Inferred(), None)
ValDef("macro$1", Inferred(), None)

baz
ValDef("foo2", Inferred(), None)
Expand Down
12 changes: 6 additions & 6 deletions tests/run-macros/tasty-location.check
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
foo Location(List(Test$, loc1))
foo Location(List(Test$, main))
foo Location(List(Test$, main))
foo Location(List(Test$, main, bar))
foo Location(List(Test$, main, bar, baz))
foo Location(List(Test$, main, f, $anonfun))
foo Location(List(Test$, loc1, macro$1))
foo Location(List(Test$, main, macro$2))
foo Location(List(Test$, main, macro$3))
foo Location(List(Test$, main, bar, macro$4))
foo Location(List(Test$, main, bar, baz, macro$5))
foo Location(List(Test$, main, f, $anonfun, macro$6))
13 changes: 13 additions & 0 deletions tests/run-staging/i7142.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import scala.quoted._
import scala.quoted.staging._
import scala.util.control.NonLocalReturns._

object Test {
given Toolbox = Toolbox.make(getClass.getClassLoader)
def main(args: Array[String]): Unit =
try run {returning('{ { (x: Int) => ${ throwReturn('x) }} apply 0 })}
catch {
case ex: dotty.tools.dotc.reporting.diagnostic.messages.Error =>
assert(ex.getMessage == "While expanding a macro, a reference to value x was used outside the scope where it was defined", ex.getMessage)
}
}