From 0898c315e955bc993be3aadad2eaa3fd0ac5693b Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Thu, 21 Nov 2024 21:11:16 +0100 Subject: [PATCH 1/4] Make sure symbols in annotation trees are fresh before pickling --- .../tools/dotc/transform/PostTyper.scala | 35 ++++++++++++++----- tests/pos/annot-17939.scala | 8 +++++ tests/pos/annot-19846.scala | 9 +++++ tests/pos/annot-19846b.scala | 8 +++++ tests/pos/annot-i20272a.scala | 20 +++++++++++ 5 files changed, 71 insertions(+), 9 deletions(-) create mode 100644 tests/pos/annot-17939.scala create mode 100644 tests/pos/annot-19846.scala create mode 100644 tests/pos/annot-19846b.scala create mode 100644 tests/pos/annot-i20272a.scala diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala index 0e10c6c7dae7..73bedff77df1 100644 --- a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala @@ -2,7 +2,7 @@ package dotty.tools package dotc package transform -import dotty.tools.dotc.ast.{Trees, tpd, untpd, desugar} +import dotty.tools.dotc.ast.{Trees, tpd, untpd, desugar, TreeTypeMap} import scala.collection.mutable import core.* import dotty.tools.dotc.typer.Checking @@ -16,7 +16,7 @@ import Symbols.*, NameOps.* import ContextFunctionResults.annotateContextResults import config.Printers.typr import config.Feature -import util.SrcPos +import util.{SrcPos, Stats} import reporting.* import NameKinds.WildcardParamName @@ -132,17 +132,39 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase case _ => case _ => + /** Returns a copy of the given tree with all symbols fresh. + * + * Used to guarantee that no symbols are shared between trees in different + * annotations. + */ + private def copySymbols(tree: Tree)(using Context) = + Stats.trackTime("Annotations copySymbols"): + val ttm = + new TreeTypeMap: + override def withMappedSyms(syms: List[Symbol]) = + withMappedSyms(syms, mapSymbols(syms, this, true)) + ttm(tree) + + /** Transforms the given annotation tree. */ private def transformAnnot(annot: Tree)(using Context): Tree = { val saved = inJavaAnnot inJavaAnnot = annot.symbol.is(JavaDefined) if (inJavaAnnot) checkValidJavaAnnotation(annot) - try transform(annot) + try transform(copySymbols(annot)) finally inJavaAnnot = saved } private def transformAnnot(annot: Annotation)(using Context): Annotation = annot.derivedAnnotation(transformAnnot(annot.tree)) + /** Transforms all annotations in the given type. */ + private def transformAnnots(using Context) = + new TypeMap: + def apply(tp: Type) = tp match + case tp @ AnnotatedType(parent, annot) => + tp.derivedAnnotatedType(mapOver(parent), transformAnnot(annot)) + case _ => mapOver(tp) + private def processMemberDef(tree: Tree)(using Context): tree.type = { val sym = tree.symbol Checking.checkValidOperator(sym) @@ -460,12 +482,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase report.error(em"type ${alias.tpe} outside bounds $bounds", tree.srcPos) super.transform(tree) case tree: TypeTree => - tree.withType( - tree.tpe match { - case AnnotatedType(tpe, annot) => AnnotatedType(tpe, transformAnnot(annot)) - case tpe => tpe - } - ) + tree.withType(transformAnnotsIn(tpe)) case Typed(Ident(nme.WILDCARD), _) => withMode(Mode.Pattern)(super.transform(tree)) // The added mode signals that bounds in a pattern need not diff --git a/tests/pos/annot-17939.scala b/tests/pos/annot-17939.scala new file mode 100644 index 000000000000..604143183af2 --- /dev/null +++ b/tests/pos/annot-17939.scala @@ -0,0 +1,8 @@ +import scala.annotation.Annotation +class myRefined[T](f: T => Boolean) extends Annotation + +class Box[T](val x: T) +class Box2(val x: Int) + +class A(a: String @myRefined((x: Int) => Box(3).x == 3)) // crash +class A2(a2: String @myRefined((x: Int) => Box2(3).x == 3)) // works diff --git a/tests/pos/annot-19846.scala b/tests/pos/annot-19846.scala new file mode 100644 index 000000000000..ff2f8f632eab --- /dev/null +++ b/tests/pos/annot-19846.scala @@ -0,0 +1,9 @@ +package dependentAnnotation + +class lambdaAnnot(g: () => Int) extends annotation.StaticAnnotation + +def f(x: Int): Int @lambdaAnnot(() => x + 1) = x + +@main def main = + val y: Int = 5 + val z = f(y) diff --git a/tests/pos/annot-19846b.scala b/tests/pos/annot-19846b.scala new file mode 100644 index 000000000000..09c24a5cf3cf --- /dev/null +++ b/tests/pos/annot-19846b.scala @@ -0,0 +1,8 @@ +class qualified[T](predicate: T => Boolean) extends annotation.StaticAnnotation + +class EqualPair(val x: Int, val y: Int @qualified[Int](it => it == x)) + +@main def main = + val p = EqualPair(42, 42) + val y = p.y + println(42) diff --git a/tests/pos/annot-i20272a.scala b/tests/pos/annot-i20272a.scala new file mode 100644 index 000000000000..e04ee1efcd99 --- /dev/null +++ b/tests/pos/annot-i20272a.scala @@ -0,0 +1,20 @@ +import language.experimental.captureChecking + + trait Iterable[T] { self: Iterable[T]^ => + def map[U](f: T => U): Iterable[U]^{this, f} + } + + object Test { + def assertEquals[A, B](a: A, b: B): Boolean = ??? + + def foo[T](level: Int, lines: Iterable[T]) = + lines.map(x => x) + + def bar(messages: Iterable[String]) = + foo(1, messages) + + val it: Iterable[String] = ??? + val msgs = bar(it) + + assertEquals(msgs, msgs) + } From f11eac2a17d2028bb23f763895154c7a5a8c8252 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Thu, 13 Mar 2025 15:28:18 +0100 Subject: [PATCH 2/4] Make sure symbols in annotation trees are fresh before pickling [Cherry-picked cb08b469025d96cbc8b454d588f14c9dd91448d5][modified] From 5d65bf283a035b5366692af841412d1a3790a6e8 Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Fri, 22 Nov 2024 02:34:57 +0100 Subject: [PATCH 3/4] Do not copy symbols in BodyAnnotations --- .../tools/dotc/transform/PostTyper.scala | 16 +++++++++------ tests/pos/annot-body.scala | 15 ++++++++++++++ tests/pos/annot-i20272a.scala | 20 ------------------- 3 files changed, 25 insertions(+), 26 deletions(-) create mode 100644 tests/pos/annot-body.scala delete mode 100644 tests/pos/annot-i20272a.scala diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala index 73bedff77df1..72a3ff7324e8 100644 --- a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala @@ -146,19 +146,23 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase ttm(tree) /** Transforms the given annotation tree. */ - private def transformAnnot(annot: Tree)(using Context): Tree = { + private def transformAnnotTree(annot: Tree)(using Context): Tree = { val saved = inJavaAnnot inJavaAnnot = annot.symbol.is(JavaDefined) if (inJavaAnnot) checkValidJavaAnnotation(annot) - try transform(copySymbols(annot)) + try transform(annot) finally inJavaAnnot = saved } private def transformAnnot(annot: Annotation)(using Context): Annotation = - annot.derivedAnnotation(transformAnnot(annot.tree)) + val tree1 = + annot match + case _: BodyAnnotation => annot.tree + case _ => copySymbols(annot.tree) + annot.derivedAnnotation(transformAnnotTree(tree1)) /** Transforms all annotations in the given type. */ - private def transformAnnots(using Context) = + private def transformAnnotsIn(using Context) = new TypeMap: def apply(tp: Type) = tp match case tp @ AnnotatedType(parent, annot) => @@ -460,7 +464,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase Checking.checkRealizable(tree.tpt.tpe, tree.srcPos, "SAM type") super.transform(tree) case tree @ Annotated(annotated, annot) => - cpy.Annotated(tree)(transform(annotated), transformAnnot(annot)) + cpy.Annotated(tree)(transform(annotated), transformAnnotTree(annot)) case tree: AppliedTypeTree => if (tree.tpt.symbol == defn.andType) Checking.checkNonCyclicInherited(tree.tpe, tree.args.tpes, EmptyScope, tree.srcPos) @@ -482,7 +486,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase report.error(em"type ${alias.tpe} outside bounds $bounds", tree.srcPos) super.transform(tree) case tree: TypeTree => - tree.withType(transformAnnotsIn(tpe)) + tree.withType(transformAnnotsIn(tree.tpe)) case Typed(Ident(nme.WILDCARD), _) => withMode(Mode.Pattern)(super.transform(tree)) // The added mode signals that bounds in a pattern need not diff --git a/tests/pos/annot-body.scala b/tests/pos/annot-body.scala new file mode 100644 index 000000000000..d8f6f79ae674 --- /dev/null +++ b/tests/pos/annot-body.scala @@ -0,0 +1,15 @@ +// This test checks that symbols in `BodyAnnotation` are not copied in +// `transformAnnot` during `PostTyper`. + +package json + +trait Reads[A] { + def reads(a: Any): A +} + +object JsMacroImpl { + inline def reads[A]: Reads[A] = + new Reads[A] { self => + def reads(a: Any) = ??? + } +} diff --git a/tests/pos/annot-i20272a.scala b/tests/pos/annot-i20272a.scala deleted file mode 100644 index e04ee1efcd99..000000000000 --- a/tests/pos/annot-i20272a.scala +++ /dev/null @@ -1,20 +0,0 @@ -import language.experimental.captureChecking - - trait Iterable[T] { self: Iterable[T]^ => - def map[U](f: T => U): Iterable[U]^{this, f} - } - - object Test { - def assertEquals[A, B](a: A, b: B): Boolean = ??? - - def foo[T](level: Int, lines: Iterable[T]) = - lines.map(x => x) - - def bar(messages: Iterable[String]) = - foo(1, messages) - - val it: Iterable[String] = ??? - val msgs = bar(it) - - assertEquals(msgs, msgs) - } From 649ea26de83ffbad923ae78d2317d3258ea42bf9 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Thu, 13 Mar 2025 15:49:33 +0100 Subject: [PATCH 4/4] Do not copy symbols in BodyAnnotations [Cherry-picked ca3c7975ac68b0aba1893d82aadd02809bbc5ced][modified]