Merge pull request #8000 from dotty-staging/fix-#7142

Fix #7142: Detect scope extrusions in macros and run

Author: nicolasstucki

community-build/community-projects/sourcecode

@@ -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"

compiler/src/dotty/tools/dotc/core/StdNames.scala

@@ -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 =>
@@ -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`

compiler/src/dotty/tools/dotc/transform/Splicer.scala

@@ -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 _ =>
   }
 
@@ -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 =>

compiler/src/dotty/tools/dotc/transform/TreeMapWithStages.scala

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

compiler/src/dotty/tools/dotc/typer/Inliner.scala

@@ -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
@@ -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) =>

staging/src/scala/quoted/staging/QuoteCompiler.scala

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

tests/neg-macros/i7142/Macro_1.scala

@@ -0,0 +1,4 @@
+
+object Test {
+  macros.test // error
+}

tests/neg-macros/i7142/Test_2.scala

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

tests/neg-macros/i7142b/Macro_1.scala

@@ -0,0 +1,4 @@
+
+object Test {
+  macros.test // error
+}

tests/neg-macros/i7142b/Test_2.scala

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

tests/neg-macros/i7142c/Macro_1.scala

@@ -0,0 +1,4 @@
+
+object Test {
+  macros.test // error
+}

tests/neg-macros/i7142c/Test_2.scala

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

tests/neg-macros/i7142d/Macro_1.scala

@@ -0,0 +1,4 @@
+
+object Test {
+  macros.test // error
+}

tests/neg-macros/i7142d/Test_2.scala

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

tests/neg-macros/i7142e/Macro_1.scala

@@ -0,0 +1,4 @@
+
+object Test {
+  macros.test // error
+}

tests/neg-macros/i7142e/Test_2.scala

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

tests/neg-macros/i7142f/Macro_1.scala

@@ -0,0 +1,4 @@
+
+object Test {
+  macros.test // error
+}

tests/neg-macros/i7142f/Test_2.scala

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

tests/neg-macros/i7142g/Macro_1.scala

@@ -0,0 +1,4 @@
+
+object Test {
+  macros.test // error
+}

tests/neg-macros/i7142g/Test_2.scala

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

tests/run-custom-args/Yretain-trees/tasty-extractors-owners.check

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

tests/run-macros/tasty-location.check

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