Skip to content

Commit 8d45fcf

Browse files
nicolasstuckinicolasstucki
committed
Fix macro annotation splice owner
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).
1 parent 01a5dd4 commit 8d45fcf

File tree

18 files changed

+119
-47
lines changed

18 files changed

+119
-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
@@ -91,7 +91,7 @@ class MacroAnnotations(thisPhase: DenotTransformer):
9191
// TODO: Remove when scala.annaotaion.MacroAnnotation is no longer experimental
9292
assert(annotInstance.getClass.getClassLoader.loadClass("scala.annotation.MacroAnnotation").isInstance(annotInstance))
9393

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

9797
/** 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/neg-macros/annot-quote-out-of-context.check

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
2+
-- Error: tests/neg-macros/annot-quote-out-of-context/Test_2.scala:2:6 -------------------------------------------------
3+
1 |@memberLeakAnnot
4+
2 |class A // error // TODO update once classes are supported
5+
|^
6+
|macro annotations are not supported on class
7+
-- Error: tests/neg-macros/annot-quote-out-of-context/Test_2.scala:5:2 -------------------------------------------------
8+
5 | @memberLeakAnnot // error
9+
| ^^^^^^^^^^^^^^^^
10+
|Failed to evaluate macro.
11+
| Caused by class java.lang.AssertionError: assertion failed: Quote cannot be created with Quotes that has a class spliceOwner.
12+
|The current splice owner is `class B`
13+
|
14+
|To set a valid splice owner use `given Quotes = owner.asQuotes` with the
15+
|`owner` symbol of a `def`, `val`, `var` or `lazy val` that will contain the quoted code.
16+
|
17+
| scala.runtime.Scala3RunTime$.assertFailed(Scala3RunTime.scala:8)
18+
| scala.quoted.runtime.impl.QuotesImpl.checkIfSpliceOwnerOfQuotes(QuotesImpl.scala:3072)
19+
| scala.quoted.runtime.impl.QuotesImpl.unpickleExprV2(QuotesImpl.scala:3056)
20+
| memberLeakAnnot.transform(Macro_1.scala:9)

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