Skip to content

Commit 4b0fe4d

Browse files
nicolasstuckinicolasstucki
authored
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).
2 parents ddc96b9 + 9431066 commit 4b0fe4d

File tree

17 files changed

+99
-47
lines changed

17 files changed

+99
-47
lines changed

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

+31-12
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import dotty.tools.dotc.ast.{TreeTypeMap, tpd}
55
import dotty.tools.dotc.config.Printers._
66
import dotty.tools.dotc.core.Contexts._
77
import dotty.tools.dotc.core.Decorators._
8+
import dotty.tools.dotc.core.Flags._
89
import dotty.tools.dotc.core.Mode
910
import dotty.tools.dotc.core.Symbols._
1011
import dotty.tools.dotc.core.Types._
@@ -248,23 +249,41 @@ object PickledQuotes {
248249
case pickled: String => TastyString.unpickle(pickled)
249250
case pickled: List[String] => TastyString.unpickle(pickled)
250251

251-
quotePickling.println(s"**** unpickling quote from TASTY\n${TastyPrinter.showContents(bytes, ctx.settings.color.value == "never")}")
252+
val unpicklingContext =
253+
if ctx.owner.isClass then
254+
// When a quote is unpickled with a Quotes context that that has a class `spliceOwner`
255+
// we need to use a dummy owner to unpickle it. Otherwise any definitions defined
256+
// in the quoted block would be accidentally entered in the class.
257+
// When splicing this expression, this owner is replaced with the correct owner (see `quotedExprToTree` and `quotedTypeToTree` above).
258+
// 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).
259+
// `-Xcheck-macros` will check for inconsistent owners and provide the users hints on how to improve them.
260+
//
261+
// Quotes context that that has a class `spliceOwner` can come from a macro annotation
262+
// or a user setting it explicitly using `Symbol.asQuotes`.
263+
ctx.withOwner(newSymbol(ctx.owner, "$quoteOwnedByClass$".toTermName, Private, defn.AnyType, NoSymbol))
264+
else ctx
252265

253-
val mode = if (isType) UnpickleMode.TypeTree else UnpickleMode.Term
254-
val unpickler = new DottyUnpickler(bytes, mode)
255-
unpickler.enter(Set.empty)
266+
inContext(unpicklingContext) {
256267

257-
val tree = unpickler.tree
258-
QuotesCache(pickled) = tree
268+
quotePickling.println(s"**** unpickling quote from TASTY\n${TastyPrinter.showContents(bytes, ctx.settings.color.value == "never")}")
259269

260-
// Make sure trees and positions are fully loaded
261-
new TreeTraverser {
262-
def traverse(tree: Tree)(using Context): Unit = traverseChildren(tree)
263-
}.traverse(tree)
270+
val mode = if (isType) UnpickleMode.TypeTree else UnpickleMode.Term
271+
val unpickler = new DottyUnpickler(bytes, mode)
272+
unpickler.enter(Set.empty)
264273

265-
quotePickling.println(i"**** unpickled quote\n$tree")
274+
val tree = unpickler.tree
275+
QuotesCache(pickled) = tree
276+
277+
// Make sure trees and positions are fully loaded
278+
new TreeTraverser {
279+
def traverse(tree: Tree)(using Context): Unit = traverseChildren(tree)
280+
}.traverse(tree)
281+
282+
quotePickling.println(i"**** unpickled quote\n$tree")
283+
284+
tree
285+
}
266286

267-
tree
268287
}
269288

270289
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ class MacroAnnotations(thisPhase: DenotTransformer):
119119
// TODO: Remove when scala.annaotaion.MacroAnnotation is no longer experimental
120120
assert(annotInstance.getClass.getClassLoader.loadClass("scala.annotation.MacroAnnotation").isInstance(annotInstance))
121121

122-
val quotes = QuotesImpl()(using SpliceScope.contextWithNewSpliceScope(tree.symbol.sourcePos)(using MacroExpansion.context(tree)).withOwner(tree.symbol))
122+
val quotes = QuotesImpl()(using SpliceScope.contextWithNewSpliceScope(tree.symbol.sourcePos)(using MacroExpansion.context(tree)).withOwner(tree.symbol.owner))
123123
annotInstance.transform(using quotes)(tree.asInstanceOf[quotes.reflect.Definition])
124124

125125
/** Check that this tree can be added by the macro annotation and enter it if needed */

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

+3-1
Original file line numberDiff line numberDiff line change
@@ -2662,7 +2662,9 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler
26622662

26632663
def show(using printer: Printer[Symbol]): String = printer.show(self)
26642664

2665-
def asQuotes: Nested = new QuotesImpl(using ctx.withOwner(self))
2665+
def asQuotes: Nested =
2666+
assert(self.ownersIterator.contains(ctx.owner), s"$self is not owned by ${ctx.owner}")
2667+
new QuotesImpl(using ctx.withOwner(self))
26662668

26672669
end extension
26682670

‎library/src/scala/annotation/MacroAnnotation.scala

+13-7
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,20 @@ trait MacroAnnotation extends StaticAnnotation:
3131
* def transform(using Quotes)(tree: quotes.reflect.Definition): List[quotes.reflect.Definition] =
3232
* import quotes.reflect._
3333
* tree match
34-
* case DefDef(name, TermParamClause(param :: Nil) :: Nil, tpt, Some(rhsTree)) =>
35-
* (Ref(param.symbol).asExpr, rhsTree.asExpr) match
36-
* case ('{ $paramRefExpr: t }, '{ $rhsExpr: u }) =>
37-
* val cacheSymbol = Symbol.newUniqueVal(tree.symbol.owner, name + "Cache", TypeRepr.of[mutable.Map[t, u]], Flags.Private, Symbol.noSymbol)
38-
* val cacheRhs = '{ mutable.Map.empty[t, u] }.asTerm
34+
* case DefDef(name, TermParamClause(param :: Nil) :: Nil, tpt, Some(rhsTree)) =>
35+
* (param.tpt.tpe.asType, tpt.tpe.asType) match
36+
* case ('[t], '[u]) =>
37+
* val cacheSymbol = Symbol.newUniqueVal(Symbol.spliceOwner, name + "Cache", TypeRepr.of[mutable.Map[t, u]], Flags.Private, Symbol.noSymbol)
38+
* val cacheRhs =
39+
* given Quotes = cacheSymbol.asQuotes
40+
* '{ mutable.Map.empty[t, u] }.asTerm
3941
* val cacheVal = ValDef(cacheSymbol, Some(cacheRhs))
40-
* val cacheRefExpr = Ref(cacheSymbol).asExprOf[mutable.Map[t, u]]
41-
* val newRhs = '{ $cacheRefExpr.getOrElseUpdate($paramRefExpr, $rhsExpr) }.asTerm
42+
* val newRhs =
43+
* given Quotes = tree.symbol.asQuotes
44+
* val cacheRefExpr = Ref(cacheSymbol).asExprOf[mutable.Map[t, u]]
45+
* val paramRefExpr = Ref(param.symbol).asExprOf[t]
46+
* val rhsExpr = rhsTree.asExprOf[u]
47+
* '{ $cacheRefExpr.getOrElseUpdate($paramRefExpr, $rhsExpr) }.asTerm
4248
* val newTree = DefDef.copy(tree)(name, TermParamClause(param :: Nil) :: Nil, tpt, Some(newRhs))
4349
* List(cacheVal, newTree)
4450
* case _ =>

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

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

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

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

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

+3-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ class useInlinedIdentity extends MacroAnnotation {
77
import quotes.reflect.*
88
tree match
99
case DefDef(name, params, tpt, Some(rhs)) =>
10-
val newRhs = '{ inlinedIdentity(${rhs.asExpr}) }.asTerm
10+
val newRhs =
11+
given Quotes = tree.symbol.asQuotes
12+
'{ inlinedIdentity(${rhs.asExpr}) }.asTerm
1113
List(DefDef.copy(tree)(name, params, tpt, Some(newRhs)))
1214
}
1315

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

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ class print(msg: String) extends MacroAnnotation:
77
import quotes.reflect._
88
tree match
99
case DefDef(name, params, tpt, Some(rhsTree)) =>
10+
given Quotes = tree.symbol.asQuotes
1011
rhsTree.asExpr match
1112
case '{ $rhsExpr: t } =>
1213
val newRhs = '{ println(${Expr(msg)}); $rhsExpr }.asTerm

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ class bind(str: String) extends MacroAnnotation:
77
import quotes.reflect._
88
tree match
99
case ValDef(name, tpt, Some(rhsTree)) =>
10-
val valSym = Symbol.newUniqueVal(tree.symbol.owner, str, tpt.tpe, Flags.Private, Symbol.noSymbol)
10+
val valSym = Symbol.newUniqueVal(Symbol.spliceOwner, str, tpt.tpe, Flags.Private, Symbol.noSymbol)
1111
val valDef = ValDef(valSym, Some(rhsTree))
1212
val newRhs = Ref(valSym)
1313
val newTree = ValDef.copy(tree)(name, tpt, Some(newRhs))

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

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ class hello extends MacroAnnotation {
77
import quotes.reflect._
88
tree match
99
case DefDef(name, params, tpt, Some(t)) =>
10+
given Quotes = tree.symbol.asQuotes
1011
val rhs = '{
1112
${t.asExprOf[String]} + "hello"
1213
}.asTerm

‎tests/run-macros/annot-gen2/Macro_2.scala

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ class foo extends MacroAnnotation {
77
import quotes.reflect._
88
tree match
99
case DefDef(name, params, tpt, Some(t)) =>
10+
given Quotes = tree.symbol.asQuotes
1011
val s = Ref(params.head.params.head.symbol).asExprOf[String]
1112
val rhs = '{
1213
@hello def foo1(s: String): String = ${

‎tests/run-macros/annot-generate/Macro_1.scala

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

‎tests/run-macros/annot-generate/Macro_2.scala

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ class foo extends MacroAnnotation {
77
import quotes.reflect._
88
tree match
99
case DefDef(name, params, tpt, Some(t)) =>
10+
given Quotes = tree.symbol.asQuotes
1011
val rhs = '{
1112
@hello def foo(x: Int): Int = x + 1
1213
${t.asExprOf[Int]}

‎tests/run-macros/annot-memo/Macro_1.scala

+13-7
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,20 @@ class memoize extends MacroAnnotation:
77
def transform(using Quotes)(tree: quotes.reflect.Definition): List[quotes.reflect.Definition] =
88
import quotes.reflect._
99
tree match
10-
case DefDef(name, TermParamClause(param :: Nil) :: Nil, tpt, Some(rhsTree)) =>
11-
(Ref(param.symbol).asExpr, rhsTree.asExpr) match
12-
case ('{ $paramRefExpr: t }, '{ $rhsExpr: u }) =>
13-
val cacheSymbol = Symbol.newUniqueVal(tree.symbol.owner, name + "Cache", TypeRepr.of[mutable.Map[t, u]], Flags.Private, Symbol.noSymbol)
14-
val cacheRhs = '{ mutable.Map.empty[t, u] }.asTerm
10+
case DefDef(name, TermParamClause(param :: Nil) :: Nil, tpt, Some(rhsTree)) =>
11+
(param.tpt.tpe.asType, tpt.tpe.asType) match
12+
case ('[t], '[u]) =>
13+
val cacheSymbol = Symbol.newUniqueVal(Symbol.spliceOwner, name + "Cache", TypeRepr.of[mutable.Map[t, u]], Flags.Private, Symbol.noSymbol)
14+
val cacheRhs =
15+
given Quotes = cacheSymbol.asQuotes
16+
'{ mutable.Map.empty[t, u] }.asTerm
1517
val cacheVal = ValDef(cacheSymbol, Some(cacheRhs))
16-
val cacheRefExpr = Ref(cacheSymbol).asExprOf[mutable.Map[t, u]]
17-
val newRhs = '{ $cacheRefExpr.getOrElseUpdate($paramRefExpr, $rhsExpr) }.asTerm
18+
val newRhs =
19+
given Quotes = tree.symbol.asQuotes
20+
val cacheRefExpr = Ref(cacheSymbol).asExprOf[mutable.Map[t, u]]
21+
val paramRefExpr = Ref(param.symbol).asExprOf[t]
22+
val rhsExpr = rhsTree.asExprOf[u]
23+
'{ $cacheRefExpr.getOrElseUpdate($paramRefExpr, $rhsExpr) }.asTerm
1824
val newTree = DefDef.copy(tree)(name, TermParamClause(param :: Nil) :: Nil, tpt, Some(newRhs))
1925
List(cacheVal, newTree)
2026
case _ =>

‎tests/run-macros/annot-memo/Test_2.scala

+7
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@ class Bar:
44
println(s"compute fib of $n")
55
if n <= 1 then n
66
else fib(n - 1) + fib(n - 2)
7+
//> private val fibCache$macro$1: mutable.Map[Int, Int] = mutable.Map.empty[Int, Int]
8+
//> @memoize def fib(n: Int): Int =
9+
//> fibCache$macro$1.getOrElseUpdate(n, {
10+
//> println(s"compute fib of $n")
11+
//> if n <= 1 then n
12+
//> else fib(n - 1) + fib(n - 2)
13+
//> })
714

815
@memoize
916
def fib(n: Long): Long =

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

+4-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ class print(msg: String) extends MacroAnnotation:
66
def transform(using Quotes)(tree: quotes.reflect.Definition): List[quotes.reflect.Definition] =
77
import quotes.reflect._
88
def printMsg(msg: String) =
9-
val valSym = Symbol.newUniqueVal(tree.symbol.owner, tree.symbol.name + "$print$" + msg, TypeRepr.of[Unit], Flags.Private, Symbol.noSymbol)
10-
val valRhs = '{ println(${Expr(msg)}) }.asTerm
9+
val valSym = Symbol.newUniqueVal(Symbol.spliceOwner, tree.symbol.name + "$print$" + msg, TypeRepr.of[Unit], Flags.Private, Symbol.noSymbol)
10+
val valRhs =
11+
given Quotes = valSym.asQuotes
12+
'{ println(${Expr(msg)}) }.asTerm
1113
ValDef(valSym, Some(valRhs))
1214
List(printMsg(s"before: $msg"), tree, printMsg(s"after: $msg"))

‎tests/run-macros/annot-simple-fib/Macro_1.scala

+16-12
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,23 @@ class memoize extends MacroAnnotation {
88
import quotes.reflect._
99
tree match
1010
case DefDef(name, params, tpt, Some(fibTree)) =>
11-
val cacheRhs = '{Map.empty[Int, Int]}.asTerm
12-
val cacheSymbol = Symbol.newUniqueVal(tree.symbol.owner, name + "Cache", TypeRepr.of[Map[Int, Int]], Flags.EmptyFlags, Symbol.noSymbol)
11+
val cacheSymbol = Symbol.newUniqueVal(Symbol.spliceOwner, name + "Cache", TypeRepr.of[Map[Int, Int]], Flags.EmptyFlags, Symbol.noSymbol)
12+
val cacheRhs =
13+
given Quotes = cacheSymbol.asQuotes
14+
'{Map.empty[Int, Int]}.asTerm
1315
val cacheVal = ValDef(cacheSymbol, Some(cacheRhs))
14-
val fibCache = Ref(cacheSymbol).asExprOf[Map[Int, Int]]
15-
val n = Ref(params.head.params.head.symbol).asExprOf[Int]
16-
val rhs = '{
17-
if $fibCache.contains($n) then
18-
$fibCache($n)
19-
else
20-
val res = ${fibTree.asExprOf[Int]}
21-
$fibCache($n) = res
22-
res
23-
}.asTerm
16+
val rhs =
17+
given Quotes = tree.symbol.asQuotes
18+
val fibCache = Ref(cacheSymbol).asExprOf[Map[Int, Int]]
19+
val n = Ref(params.head.params.head.symbol).asExprOf[Int]
20+
'{
21+
if $fibCache.contains($n) then
22+
$fibCache($n)
23+
else
24+
val res = ${fibTree.asExprOf[Int]}
25+
$fibCache($n) = res
26+
res
27+
}.asTerm
2428
val newFib = DefDef.copy(tree)(name, params, tpt, Some(rhs))
2529
List(cacheVal, newFib)
2630
}

0 commit comments

Comments
 (0)