Fix macro annotations spliceOwner (#16513)

The splice owner of the macro annotation should be the owner of the
annotated definition. The issue was that if that owner was a class
quotes unpickled with this context accidentally entered definitions
in the quotes into the class. Now we unpickle these definitions using
a dummy val owner (similar to the LocalDummy).

Author: nicolasstucki

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

@@ -5,6 +5,7 @@ import dotty.tools.dotc.ast.{TreeTypeMap, tpd}
 import dotty.tools.dotc.config.Printers._
 import dotty.tools.dotc.core.Contexts._
 import dotty.tools.dotc.core.Decorators._
+import dotty.tools.dotc.core.Flags._
 import dotty.tools.dotc.core.Mode
 import dotty.tools.dotc.core.Symbols._
 import dotty.tools.dotc.core.Types._
@@ -248,23 +249,41 @@ object PickledQuotes {
           case pickled: String => TastyString.unpickle(pickled)
           case pickled: List[String] => TastyString.unpickle(pickled)
 
-        quotePickling.println(s"**** unpickling quote from TASTY\n${TastyPrinter.showContents(bytes, ctx.settings.color.value == "never")}")
+        val unpicklingContext =
+          if ctx.owner.isClass then
+            // When a quote is unpickled with a Quotes context that that has a class `spliceOwner`
+            // we need to use a dummy owner to unpickle it. Otherwise any definitions defined
+            // in the quoted block would be accidentally entered in the class.
+            // When splicing this expression, this owner is replaced with the correct owner (see `quotedExprToTree` and `quotedTypeToTree` above).
+            // On the other hand, if the expression is used as a reflect term, the user must call `changeOwner` (same as with other expressions used within a nested owner).
+            // `-Xcheck-macros` will check for inconsistent owners and provide the users hints on how to improve them.
+            //
+            // Quotes context that that has a class `spliceOwner` can come from a macro annotation
+            // or a user setting it explicitly using `Symbol.asQuotes`.
+            ctx.withOwner(newSymbol(ctx.owner, "$quoteOwnedByClass$".toTermName, Private, defn.AnyType, NoSymbol))
+            else ctx
 
-        val mode = if (isType) UnpickleMode.TypeTree else UnpickleMode.Term
-        val unpickler = new DottyUnpickler(bytes, mode)
-        unpickler.enter(Set.empty)
+        inContext(unpicklingContext) {
 
-        val tree = unpickler.tree
-        QuotesCache(pickled) = tree
+          quotePickling.println(s"**** unpickling quote from TASTY\n${TastyPrinter.showContents(bytes, ctx.settings.color.value == "never")}")
 
-        // Make sure trees and positions are fully loaded
-        new TreeTraverser {
-          def traverse(tree: Tree)(using Context): Unit = traverseChildren(tree)
-        }.traverse(tree)
+          val mode = if (isType) UnpickleMode.TypeTree else UnpickleMode.Term
+          val unpickler = new DottyUnpickler(bytes, mode)
+          unpickler.enter(Set.empty)
 
-        quotePickling.println(i"**** unpickled quote\n$tree")
+          val tree = unpickler.tree
+          QuotesCache(pickled) = tree
+
+          // Make sure trees and positions are fully loaded
+          new TreeTraverser {
+            def traverse(tree: Tree)(using Context): Unit = traverseChildren(tree)
+          }.traverse(tree)
+
+          quotePickling.println(i"**** unpickled quote\n$tree")
+
+          tree
+        }
 
-        tree
   }
 
 }

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

@@ -119,7 +119,7 @@ class MacroAnnotations(thisPhase: DenotTransformer):
     // TODO: Remove when scala.annaotaion.MacroAnnotation is no longer experimental
     assert(annotInstance.getClass.getClassLoader.loadClass("scala.annotation.MacroAnnotation").isInstance(annotInstance))
 
-    val quotes = QuotesImpl()(using SpliceScope.contextWithNewSpliceScope(tree.symbol.sourcePos)(using MacroExpansion.context(tree)).withOwner(tree.symbol))
+    val quotes = QuotesImpl()(using SpliceScope.contextWithNewSpliceScope(tree.symbol.sourcePos)(using MacroExpansion.context(tree)).withOwner(tree.symbol.owner))
     annotInstance.transform(using quotes)(tree.asInstanceOf[quotes.reflect.Definition])
 
   /** Check that this tree can be added by the macro annotation and enter it if needed */

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

@@ -2662,7 +2662,9 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler
 
         def show(using printer: Printer[Symbol]): String = printer.show(self)
 
-        def asQuotes: Nested = new QuotesImpl(using ctx.withOwner(self))
+        def asQuotes: Nested =
+          assert(self.ownersIterator.contains(ctx.owner), s"$self is not owned by ${ctx.owner}")
+          new QuotesImpl(using ctx.withOwner(self))
 
       end extension
 

library/src/scala/annotation/MacroAnnotation.scala

@@ -31,14 +31,20 @@ trait MacroAnnotation extends StaticAnnotation:
    *    def transform(using Quotes)(tree: quotes.reflect.Definition): List[quotes.reflect.Definition] =
    *      import quotes.reflect._
    *      tree match
-   *        case DefDef(name, TermParamClause(param :: Nil) :: Nil, tpt, Some(rhsTree)) =>
-   *          (Ref(param.symbol).asExpr, rhsTree.asExpr) match
-   *            case ('{ $paramRefExpr: t }, '{ $rhsExpr: u }) =>
-   *              val cacheSymbol = Symbol.newUniqueVal(tree.symbol.owner, name + "Cache", TypeRepr.of[mutable.Map[t, u]], Flags.Private, Symbol.noSymbol)
-   *              val cacheRhs = '{ mutable.Map.empty[t, u] }.asTerm
+   *        case DefDef(name, TermParamClause(param  :: Nil) :: Nil, tpt, Some(rhsTree)) =>
+   *          (param.tpt.tpe.asType, tpt.tpe.asType) match
+   *            case ('[t], '[u]) =>
+   *              val cacheSymbol = Symbol.newUniqueVal(Symbol.spliceOwner, name + "Cache", TypeRepr.of[mutable.Map[t, u]], Flags.Private, Symbol.noSymbol)
+   *              val cacheRhs =
+   *                given Quotes = cacheSymbol.asQuotes
+   *                '{ mutable.Map.empty[t, u] }.asTerm
    *              val cacheVal = ValDef(cacheSymbol, Some(cacheRhs))
-   *              val cacheRefExpr = Ref(cacheSymbol).asExprOf[mutable.Map[t, u]]
-   *              val newRhs = '{ $cacheRefExpr.getOrElseUpdate($paramRefExpr, $rhsExpr) }.asTerm
+   *              val newRhs =
+   *                given Quotes = tree.symbol.asQuotes
+   *                val cacheRefExpr = Ref(cacheSymbol).asExprOf[mutable.Map[t, u]]
+   *                val paramRefExpr = Ref(param.symbol).asExprOf[t]
+   *                val rhsExpr = rhsTree.asExprOf[u]
+   *                '{ $cacheRefExpr.getOrElseUpdate($paramRefExpr, $rhsExpr) }.asTerm
    *              val newTree = DefDef.copy(tree)(name, TermParamClause(param :: Nil) :: Nil, tpt, Some(newRhs))
    *              List(cacheVal, newTree)
    *        case _ =>

tests/neg-macros/annot-accessIndirect/Macro_1.scala

@@ -5,7 +5,7 @@ import scala.quoted._
 class hello extends MacroAnnotation {
   def transform(using Quotes)(tree: quotes.reflect.Definition): List[quotes.reflect.Definition] =
     import quotes.reflect._
-    val helloSymbol = Symbol.newUniqueVal(tree.symbol.owner, "hello", TypeRepr.of[String], Flags.EmptyFlags, Symbol.noSymbol)
+    val helloSymbol = Symbol.newUniqueVal(Symbol.spliceOwner, "hello", TypeRepr.of[String], Flags.EmptyFlags, Symbol.noSymbol)
     val helloVal = ValDef(helloSymbol, Some(Literal(StringConstant("Hello, World!"))))
     List(helloVal, tree)
 }

tests/neg-macros/annot-accessIndirect/Macro_2.scala

@@ -7,7 +7,7 @@ class foo extends MacroAnnotation {
     import quotes.reflect._
     val s = '{@hello def foo1(x: Int): Int = x + 1;()}.asTerm
     val fooDef = s.asInstanceOf[Inlined].body.asInstanceOf[Block].statements.head.asInstanceOf[DefDef]
-    val hello = Ref(tree.symbol.owner.declaredFields("hello").head).asExprOf[String] // error
+    val hello = Ref(Symbol.spliceOwner.declaredFields("hello").head).asExprOf[String] // error
     tree match
       case DefDef(name, params, tpt, Some(t)) =>
         val rhs = '{

tests/pos-macros/annot-then-inline/Macro_1.scala

@@ -7,7 +7,9 @@ class useInlinedIdentity extends MacroAnnotation {
     import quotes.reflect.*
     tree match
       case DefDef(name, params, tpt, Some(rhs)) =>
-        val newRhs = '{ inlinedIdentity(${rhs.asExpr}) }.asTerm
+        val newRhs =
+          given Quotes = tree.symbol.asQuotes
+          '{ inlinedIdentity(${rhs.asExpr}) }.asTerm
         List(DefDef.copy(tree)(name, params, tpt, Some(newRhs)))
 }
 

tests/run-macros/annot-annot-order/Macro_1.scala

@@ -7,6 +7,7 @@ class print(msg: String) extends MacroAnnotation:
     import quotes.reflect._
     tree match
       case DefDef(name, params, tpt, Some(rhsTree)) =>
+        given Quotes = tree.symbol.asQuotes
         rhsTree.asExpr match
           case '{ $rhsExpr: t } =>
             val newRhs = '{ println(${Expr(msg)}); $rhsExpr }.asTerm

tests/run-macros/annot-bind/Macro_1.scala

@@ -7,7 +7,7 @@ class bind(str: String) extends MacroAnnotation:
     import quotes.reflect._
     tree match
       case ValDef(name, tpt, Some(rhsTree)) =>
-        val valSym = Symbol.newUniqueVal(tree.symbol.owner, str, tpt.tpe, Flags.Private, Symbol.noSymbol)
+        val valSym = Symbol.newUniqueVal(Symbol.spliceOwner, str, tpt.tpe, Flags.Private, Symbol.noSymbol)
         val valDef = ValDef(valSym, Some(rhsTree))
         val newRhs = Ref(valSym)
         val newTree = ValDef.copy(tree)(name, tpt, Some(newRhs))

tests/run-macros/annot-gen2/Macro_1.scala

@@ -7,6 +7,7 @@ class hello extends MacroAnnotation {
     import quotes.reflect._
     tree match
       case DefDef(name, params, tpt, Some(t)) =>
+        given Quotes = tree.symbol.asQuotes
         val rhs = '{
           ${t.asExprOf[String]} + "hello"
         }.asTerm