Merge pull request #11442 from dotty-staging/add-stonger-scope-extrusion-check

Add add informative scope extrusion check

Author: nicolasstucki

compiler/src/dotty/tools/dotc/quoted/PickledQuotes.scala

@@ -41,14 +41,14 @@ object PickledQuotes {
   /** Transform the expression into its fully spliced Tree */
   def quotedExprToTree[T](expr: quoted.Expr[T])(using Context): Tree = {
     val expr1 = expr.asInstanceOf[ExprImpl]
-    expr1.checkScopeId(QuotesImpl.scopeId)
+    ScopeException.checkInCorrectScope(expr1.scope, SpliceScope.getCurrent, expr1.tree, "Expr")
     changeOwnerOfTree(expr1.tree, ctx.owner)
   }
 
   /** Transform the expression into its fully spliced TypeTree */
   def quotedTypeToTree(tpe: quoted.Type[?])(using Context): Tree = {
     val tpe1 = tpe.asInstanceOf[TypeImpl]
-    tpe1.checkScopeId(QuotesImpl.scopeId)
+    ScopeException.checkInCorrectScope(tpe1.scope, SpliceScope.getCurrent, tpe1.typeTree, "Type")
     changeOwnerOfTree(tpe1.typeTree, ctx.owner)
   }
 
@@ -73,23 +73,25 @@ object PickledQuotes {
     val evaluateHoles = new TreeMap {
       override def transform(tree: tpd.Tree)(using Context): tpd.Tree = tree match {
         case Hole(isTerm, idx, args) =>
-          val reifiedArgs = args.map { arg =>
-            if (arg.isTerm) (q: Quotes) ?=> new ExprImpl(arg, QuotesImpl.scopeId)
-            else new TypeImpl(arg, QuotesImpl.scopeId)
+          inContext(SpliceScope.contextWithNewSpliceScope(tree.sourcePos)) {
+            val reifiedArgs = args.map { arg =>
+              if (arg.isTerm) (q: Quotes) ?=> new ExprImpl(arg, SpliceScope.getCurrent)
+              else new TypeImpl(arg, SpliceScope.getCurrent)
+            }
+            if isTerm then
+              val quotedExpr = termHole(idx, reifiedArgs, QuotesImpl())
+              val filled = PickledQuotes.quotedExprToTree(quotedExpr)
+
+              // We need to make sure a hole is created with the source file of the surrounding context, even if
+              // it filled with contents a different source file.
+              if filled.source == ctx.source then filled
+              else filled.cloneIn(ctx.source).withSpan(tree.span)
+            else
+              // Replaces type holes generated by PickleQuotes (non-spliced types).
+              // These are types defined in a quote and used at the same level in a nested quote.
+              val quotedType = typeHole(idx, reifiedArgs)
+              PickledQuotes.quotedTypeToTree(quotedType)
           }
-          if isTerm then
-            val quotedExpr = termHole(idx, reifiedArgs, QuotesImpl())
-            val filled = PickledQuotes.quotedExprToTree(quotedExpr)
-
-            // We need to make sure a hole is created with the source file of the surrounding context, even if
-            // it filled with contents a different source file.
-            if filled.source == ctx.source then filled
-            else filled.cloneIn(ctx.source).withSpan(tree.span)
-          else
-            // Replaces type holes generated by PickleQuotes (non-spliced types).
-            // These are types defined in a quote and used at the same level in a nested quote.
-            val quotedType = typeHole(idx, reifiedArgs)
-            PickledQuotes.quotedTypeToTree(quotedType)
         case tree: Select =>
           // Retain selected members
           val qual = transform(tree.qualifier)

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

@@ -39,16 +39,17 @@ object Splicer {
    *
    *  See: `Staging`
    */
-  def splice(tree: Tree, pos: SrcPos, classLoader: ClassLoader)(using Context): Tree = tree match {
+  def splice(tree: Tree, splicePos: SrcPos, spliceExpansionPos: SrcPos, classLoader: ClassLoader)(using Context): Tree = tree match {
     case Quoted(quotedTree) => quotedTree
     case _ =>
       val macroOwner = newSymbol(ctx.owner, nme.MACROkw, Macro | Synthetic, defn.AnyType, coord = tree.span)
       try
-        inContext(ctx.withOwner(macroOwner)) {
+        val sliceContext = SpliceScope.contextWithNewSpliceScope(splicePos.sourcePos).withOwner(macroOwner)
+        inContext(sliceContext) {
           val oldContextClassLoader = Thread.currentThread().getContextClassLoader
           Thread.currentThread().setContextClassLoader(classLoader)
           try {
-            val interpreter = new Interpreter(pos, classLoader)
+            val interpreter = new Interpreter(spliceExpansionPos, classLoader)
 
             // Some parts of the macro are evaluated during the unpickling performed in quotedExprToTree
             val interpretedExpr = interpreter.interpret[Quotes => scala.quoted.Expr[Any]](tree)
@@ -74,7 +75,7 @@ object Splicer {
                |  Caused by ${ex.getClass}: ${if (ex.getMessage == null) "" else ex.getMessage}
                |    ${ex.getStackTrace.takeWhile(_.getClassName != "dotty.tools.dotc.transform.Splicer$").drop(1).mkString("\n    ")}
              """.stripMargin
-          report.error(msg, pos)
+          report.error(msg, spliceExpansionPos)
           ref(defn.Predef_undefined).withType(ErrorType(msg))
       }
   }
@@ -325,10 +326,10 @@ object Splicer {
     }
 
     private def interpretQuote(tree: Tree)(implicit env: Env): Object =
-      new ExprImpl(Inlined(EmptyTree, Nil, QuoteUtils.changeOwnerOfTree(tree, ctx.owner)).withSpan(tree.span), QuotesImpl.scopeId)
+      new ExprImpl(Inlined(EmptyTree, Nil, QuoteUtils.changeOwnerOfTree(tree, ctx.owner)).withSpan(tree.span), SpliceScope.getCurrent)
 
     private def interpretTypeQuote(tree: Tree)(implicit env: Env): Object =
-      new TypeImpl(QuoteUtils.changeOwnerOfTree(tree, ctx.owner), QuotesImpl.scopeId)
+      new TypeImpl(QuoteUtils.changeOwnerOfTree(tree, ctx.owner), SpliceScope.getCurrent)
 
     private def interpretLiteral(value: Any)(implicit env: Env): Object =
       value.asInstanceOf[Object]

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

@@ -1304,7 +1304,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(using Context) {
         case res: Apply if res.symbol == defn.QuotedRuntime_exprSplice
                         && level == 0
                         && !hasInliningErrors =>
-          val expanded = expandMacro(res.args.head, tree.span)
+          val expanded = expandMacro(res.args.head, tree.srcPos)
           typedExpr(expanded) // Inline calls and constant fold code generated by the macro
         case res =>
           inlineIfNeeded(res)
@@ -1489,7 +1489,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(using Context) {
     }
   }
 
-  private def expandMacro(body: Tree, span: Span)(using Context) = {
+  private def expandMacro(body: Tree, splicePos: SrcPos)(using Context) = {
     assert(level == 0)
     val inlinedFrom = enclosingInlineds.last
     val dependencies = macroDependencies(body)
@@ -1504,7 +1504,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(using Context) {
         ctx.compilationUnit.suspend() // this throws a SuspendException
 
     val evaluatedSplice = inContext(quoted.MacroExpansion.context(inlinedFrom)) {
-      Splicer.splice(body, inlinedFrom.srcPos, MacroClassLoader.fromContext)
+      Splicer.splice(body, splicePos, inlinedFrom.srcPos, MacroClassLoader.fromContext)
     }
     val inlinedNormailizer = new TreeMap {
       override def transform(tree: tpd.Tree)(using Context): tpd.Tree = tree match {
@@ -1514,7 +1514,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(using Context) {
     }
     val normalizedSplice = inlinedNormailizer.transform(evaluatedSplice)
     if (normalizedSplice.isEmpty) normalizedSplice
-    else normalizedSplice.withSpan(span)
+    else normalizedSplice.withSpan(splicePos.span)
   }
 
   /** Return the set of symbols that are referred at level -1 by the tree and defined in the current run.

compiler/src/scala/quoted/runtime/impl/ExprImpl.scala

@@ -10,19 +10,14 @@ import dotty.tools.dotc.ast.tpd
  *
  *  May contain references to code defined outside this Expr instance.
  */
-final class ExprImpl(val tree: tpd.Tree, val scopeId: Int) extends Expr[Any] {
+final class ExprImpl(val tree: tpd.Tree, val scope: Scope) extends Expr[Any] {
   override def equals(that: Any): Boolean = that match {
     case that: ExprImpl =>
       // Expr are wrappers around trees, therefore they are equals if their trees are equal.
-      // All scopeId should be equal unless two different runs of the compiler created the trees.
-      tree == that.tree && scopeId == that.scopeId
+      // All scope should be equal unless two different runs of the compiler created the trees.
+      tree == that.tree && scope == that.scope
     case _ => false
   }
 
-  def checkScopeId(expectedScopeId: Int): Unit =
-    if expectedScopeId != scopeId then
-      throw new ScopeException("Cannot call `scala.quoted.staging.run(...)` within a macro or another `run(...)`")
-
-  override def hashCode: Int = tree.hashCode
   override def toString: String = "'{ ... }"
 }

compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala

@@ -23,8 +23,6 @@ import scala.reflect.TypeTest
 
 object QuotesImpl {
 
-  type ScopeId = Int
-
   def apply()(using Context): Quotes =
     new QuotesImpl
 
@@ -34,11 +32,6 @@ object QuotesImpl {
     if ctx.settings.color.value == "always" then TreeAnsiCode.show(tree)
     else TreeCode.show(tree)
 
-  // TODO Explore more fine grained scope ids.
-  //      This id can only differentiate scope extrusion from one compiler instance to another.
-  def scopeId(using Context): ScopeId =
-    ctx.outersIterator.toList.last.hashCode()
-
 }
 
 class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler, QuoteMatching:
@@ -81,10 +74,7 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler
     end CompilationInfo
 
     extension (expr: Expr[Any])
-      def asTerm: Term =
-        val exprImpl = expr.asInstanceOf[ExprImpl]
-        exprImpl.checkScopeId(QuotesImpl.this.hashCode)
-        exprImpl.tree
+      def asTerm: Term = expr.asInstanceOf[ExprImpl].tree
     end extension
 
     type Tree = tpd.Tree
@@ -105,7 +95,7 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler
             case _ => false
         def asExpr: scala.quoted.Expr[Any] =
           if self.isExpr then
-            new ExprImpl(self, QuotesImpl.this.hashCode)
+            new ExprImpl(self, SpliceScope.getCurrent)
           else self match
             case TermTypeTest(self) => throw new Exception("Expected an expression. This is a partially applied Term. Try eta-expanding the term first.")
             case _ => throw new Exception("Expected a Term but was: " + self)
@@ -372,11 +362,11 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler
     given TermMethods: TermMethods with
       extension (self: Term)
         def seal: scala.quoted.Expr[Any] =
-          if self.isExpr then new ExprImpl(self, QuotesImpl.this.hashCode)
+          if self.isExpr then new ExprImpl(self, SpliceScope.getCurrent)
           else throw new Exception("Cannot seal a partially applied Term. Try eta-expanding the term first.")
 
         def sealOpt: Option[scala.quoted.Expr[Any]] =
-          if self.isExpr then Some(new ExprImpl(self, QuotesImpl.this.hashCode))
+          if self.isExpr then Some(new ExprImpl(self, SpliceScope.getCurrent))
           else None
 
         def tpe: TypeRepr = self.tpe
@@ -1670,7 +1660,7 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler
         def seal: scala.quoted.Type[_] = self.asType
 
         def asType: scala.quoted.Type[?] =
-          new TypeImpl(Inferred(self), QuotesImpl.this.hashCode)
+          new TypeImpl(Inferred(self), SpliceScope.getCurrent)
 
         def =:=(that: TypeRepr): Boolean = self =:= that
         def <:<(that: TypeRepr): Boolean = self <:< that
@@ -2894,11 +2884,11 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler
 
   def unpickleExpr[T](pickled: String | List[String], typeHole: (Int, Seq[Any]) => scala.quoted.Type[?], termHole: (Int, Seq[Any], scala.quoted.Quotes) => scala.quoted.Expr[?]): scala.quoted.Expr[T] =
     val tree = PickledQuotes.unpickleTerm(pickled, typeHole, termHole)
-    new ExprImpl(tree, hash).asInstanceOf[scala.quoted.Expr[T]]
+    new ExprImpl(tree, SpliceScope.getCurrent).asInstanceOf[scala.quoted.Expr[T]]
 
   def unpickleType[T <: AnyKind](pickled: String | List[String], typeHole: (Int, Seq[Any]) => scala.quoted.Type[?], termHole: (Int, Seq[Any], scala.quoted.Quotes) => scala.quoted.Expr[?]): scala.quoted.Type[T] =
     val tree = PickledQuotes.unpickleTypeTree(pickled, typeHole, termHole)
-    new TypeImpl(tree, hash).asInstanceOf[scala.quoted.Type[T]]
+    new TypeImpl(tree, SpliceScope.getCurrent).asInstanceOf[scala.quoted.Type[T]]
 
   object ExprMatch extends ExprMatchModule:
     def unapply[TypeBindings <: Tuple, Tup <: Tuple](scrutinee: scala.quoted.Expr[Any])(using pattern: scala.quoted.Expr[Any]): Option[Tup] =
@@ -2965,7 +2955,4 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler
     }
   }
 
-  private[this] val hash = QuotesImpl.scopeId(using ctx)
-  override def hashCode: Int = hash
-
 end QuotesImpl

compiler/src/scala/quoted/runtime/impl/ScopeException.scala

@@ -1,3 +1,37 @@
 package scala.quoted.runtime.impl
 
+import dotty.tools.dotc.ast.tpd.Tree
+import dotty.tools.dotc.core.Contexts._
+
 class ScopeException(msg: String) extends Exception(msg)
+
+object ScopeException:
+  def checkInCorrectScope(scope: Scope, currentScope: Scope, tree: Tree, kind: String)(using Context): Unit =
+    if scope.root != currentScope.root then
+      throw new ScopeException(s"Cannot use $kind oustide of the macro splice `$${...}` or the scala.quoted.staging.run(...)` where it was defined")
+
+    val yCheck = ctx.settings.Ycheck.value(using ctx).exists(x => x == "all" || x == "macros")
+    if yCheck && !scope.isOuterScopeOf(currentScope) then
+      throw new ScopeException(
+        if scope.atSameLocation(currentScope) then
+          s"""Type created in a splice, extruded from that splice and then used in a subsequent evaluation of that same splice.
+          |Splice: $scope
+          |$kind: ${tree.show}
+          |
+          |
+          |Splice stack:
+          |${scope.stack.mkString("\t", "\n\t", "\n")}
+          """.stripMargin
+        else
+          s"""Expression created in a splice was used outside of that splice.
+            |Created in: $scope
+            |Used in: $currentScope
+            |$kind: ${tree.show}
+            |
+            |
+            |Creation stack:
+            |${scope.stack.mkString("\t", "\n\t", "\n")}
+            |
+            |Use stack:
+            |${currentScope.stack.mkString("\t", "\n\t", "\n")}
+          """.stripMargin)

compiler/src/scala/quoted/runtime/impl/SpliceScope.scala

@@ -0,0 +1,66 @@
+package scala.quoted
+package runtime.impl
+
+import dotty.tools.dotc.core.Contexts._
+import dotty.tools.dotc.util.Property
+import dotty.tools.dotc.util.SourcePosition
+
+/** A scope uniquely identifies the context for evaluating a splice
+ *
+ *  A nested splice gets a new scope with the enclosing scope as its `outer`.
+ *  This also applies for recursive splices.
+ */
+trait Scope {
+  /** Outer scope that was used to create the quote containing this splice.
+   *  NoScope otherwise.
+   */
+  def outer: Scope = NoScope
+  /** Is this is a outer scope of the given scope */
+  def isOuterScopeOf(scope: Scope): Boolean =
+    this.eq(scope) || (scope.ne(NoScope) && isOuterScopeOf(scope.outer))
+  /** Scope of the top level splice or staging `run` */
+  def root: Scope =
+    if outer.eq(NoScope) then this else outer.root
+  /** Stack of locations where scopes where evaluated */
+  def stack: List[String] =
+    this.toString :: (if outer.eq(NoScope) then Nil else outer.stack)
+  /** If the two scopes correspond to the same splice in source. */
+  def atSameLocation(scope: Scope): Boolean = false
+}
+
+/** Only used for outer scope of top level splice and staging `run` */
+object NoScope extends Scope:
+  override def root: Scope = this
+  override def outer: Scope = throw UnsupportedOperationException("NoScope.outer")
+
+class SpliceScope(val pos: SourcePosition, override val outer: Scope) extends Scope:
+
+  override def atSameLocation(scope: Scope): Boolean = scope match
+    case scope: SpliceScope => this.pos == scope.pos
+    case _ => false
+
+  override def toString =
+    if pos.exists then
+      s"${pos.source.toString}:${pos.startLine + 1} at column ${pos.startColumn + 1}"
+    else
+      "Unknown location"
+
+end SpliceScope
+
+
+object SpliceScope:
+
+  /** A key to be used in a context property that tracks current splices we are evaluating */
+  private val ScopeKey = new Property.Key[Scope]
+
+  def setSpliceScope(scope: Scope)(using Context): Context =
+    ctx.fresh.setProperty(ScopeKey, scope)
+
+  def contextWithNewSpliceScope(pos: SourcePosition)(using Context): Context =
+    ctx.fresh.setProperty(ScopeKey, new SpliceScope(pos, getCurrent))
+
+    /** Context with an incremented quotation level. */
+  def getCurrent(using Context): Scope =
+      ctx.property(ScopeKey).getOrElse(NoScope)
+
+end SpliceScope

compiler/src/scala/quoted/runtime/impl/TypeImpl.scala

@@ -4,24 +4,14 @@ package runtime.impl
 import dotty.tools.dotc.ast.tpd
 
 /** Quoted type (or kind) `T` backed by a tree */
-final class TypeImpl(val typeTree: tpd.Tree, val scopeId: Int) extends Type[?] {
+final class TypeImpl(val typeTree: tpd.Tree, val scope: Scope) extends Type[?] {
   override def equals(that: Any): Boolean = that match {
     case that: TypeImpl => typeTree ==
-      // TastyTreeExpr are wrappers around trees, therfore they are equals if their trees are equal.
-      // All scopeId should be equal unless two different runs of the compiler created the trees.
-      that.typeTree && scopeId == that.scopeId
+      // TastyTreeExpr are wrappers around trees, therefore they are equals if their trees are equal.
+      // All scope should be equal unless two different runs of the compiler created the trees.
+      that.typeTree && scope == that.scope
     case _ => false
   }
 
-  /** View this expression `q.Type[T]` as a `TypeTree` */
-  def unseal(using q: Quotes): q.reflect.TypeTree =
-    checkScopeId(q.hashCode)
-    typeTree.asInstanceOf[q.reflect.TypeTree]
-
-  def checkScopeId(expectedScopeId: Int): Unit =
-    if expectedScopeId != scopeId then
-      throw new ScopeException("Cannot call `scala.quoted.staging.run(...)` within a macro or another `run(...)`")
-
-  override def hashCode: Int = typeTree.hashCode
   override def toString: String = "Type.of[...]"
 }