From 3e283a733015bb2340fb80ebb5d18e26c61c9e2b Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 14 Jan 2023 16:18:06 +0100 Subject: [PATCH 1/2] Make sure annotations are typed in expression contexts The previous logic tried but had holes. A local context was established in `typedStats#localCtx` but that context was not found in annotations that were typechecked in the completer of the definition they decorated. We are now more through and always allocate a localDummy as new owner if the owner would otherwise we a class. This means that closures in annotation arguments will have a term as owner, and therefore will be allocated in distinct local scopes. Fixes #15054 --- .../tools/dotc/core/tasty/TreeUnpickler.scala | 4 +++- .../src/dotty/tools/dotc/typer/Typer.scala | 22 +++++++++---------- tests/pos/i15054.scala | 15 +++++++++++++ 3 files changed, 28 insertions(+), 13 deletions(-) create mode 100644 tests/pos/i15054.scala diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index c0d105fecea0..26a9d2c3aa51 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -625,7 +625,9 @@ class TreeUnpickler(reader: TastyReader, else newSymbol(ctx.owner, name, flags, completer, privateWithin, coord) } - val annots = annotFns.map(_(sym.owner)) + val annotOwner = + if sym.owner.isClass then newLocalDummy(sym.owner) else sym.owner + val annots = annotFns.map(_(annotOwner)) sym.annotations = annots if sym.isOpaqueAlias then sym.setFlag(Deferred) val isScala2MacroDefinedInScala3 = flags.is(Macro, butNot = Inline) && flags.is(Erased) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 38e207dce14c..f2ebeb03359c 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2238,25 +2238,23 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer * since classes defined in a such arguments should not be entered into the * enclosing class. */ - def annotContext(mdef: untpd.Tree, sym: Symbol)(using Context): Context = { + def annotContext(mdef: untpd.Tree, sym: Symbol)(using Context): Context = def isInner(owner: Symbol) = owner == sym || sym.is(Param) && owner == sym.owner val outer = ctx.outersIterator.dropWhile(c => isInner(c.owner)).next() - var adjusted = outer.property(ExprOwner) match { + val adjusted = outer.property(ExprOwner) match { case Some(exprOwner) if outer.owner.isClass => outer.exprContext(mdef, exprOwner) case _ => outer } + def local: FreshContext = adjusted.fresh.setOwner(newLocalDummy(sym.owner)) sym.owner.infoOrCompleter match - case completer: Namer#Completer if sym.is(Param) => - val tparams = completer.completerTypeParams(sym) - if tparams.nonEmpty then - // Create a new local context with a dummy owner and a scope containing the - // type parameters of the enclosing method or class. Thus annotations can see - // these type parameters. See i12953.scala for a test case. - val dummyOwner = newLocalDummy(sym.owner) - adjusted = adjusted.fresh.setOwner(dummyOwner).setScope(newScopeWith(tparams*)) + case completer: Namer#Completer + if sym.is(Param) && completer.completerTypeParams(sym).nonEmpty => + // Create a new local context with a dummy owner and a scope containing the + // type parameters of the enclosing method or class. Thus annotations can see + // these type parameters. See i12953.scala for a test case. + local.setScope(newScopeWith(completer.completerTypeParams(sym)*)) case _ => - adjusted - } + if outer.owner.isClass then local else adjusted def completeAnnotations(mdef: untpd.MemberDef, sym: Symbol)(using Context): Unit = { // necessary to force annotation trees to be computed. diff --git a/tests/pos/i15054.scala b/tests/pos/i15054.scala new file mode 100644 index 000000000000..cda6e497b8f8 --- /dev/null +++ b/tests/pos/i15054.scala @@ -0,0 +1,15 @@ +import scala.annotation.Annotation + +class AnAnnotation(function: Int => String) extends Annotation + +@AnAnnotation(_.toString) +val a = 1 +@AnAnnotation(_.toString.length.toString) +val b = 2 + +def test = + @AnAnnotation(_.toString) + val a = 1 + @AnAnnotation(_.toString.length.toString) + val b = 2 + a + b From 4cfc06b8dcc81a21367d708602009fb1e9566369 Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 14 Jan 2023 16:21:30 +0100 Subject: [PATCH 2/2] Simplification: Drop ExprOwner property Since ExprOwner was only used in establishing annotation contexts, and was not fully effective there either, we better drop it. This means every annotation of a class member gets its own separate localDummy owner. --- .../src/dotty/tools/dotc/typer/Typer.scala | 26 +++++-------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index f2ebeb03359c..8490a43401c3 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -73,12 +73,6 @@ object Typer { /** An attachment for GADT constraints that were inferred for a pattern. */ val InferredGadtConstraints = new Property.StickyKey[core.GadtConstraint] - /** A context property that indicates the owner of any expressions to be typed in the context - * if that owner is different from the context's owner. Typically, a context with a class - * as owner would have a local dummy as ExprOwner value. - */ - private val ExprOwner = new Property.Key[Symbol] - /** An attachment on a Select node with an `apply` field indicating that the `apply` * was inserted by the Typer. */ @@ -2234,18 +2228,14 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer /** The context to be used for an annotation of `mdef`. * This should be the context enclosing `mdef`, or if `mdef` defines a parameter * the context enclosing the owner of `mdef`. - * Furthermore, we need to evaluate annotation arguments in an expression context, - * since classes defined in a such arguments should not be entered into the - * enclosing class. + * Furthermore, we need to make sure that annotation trees are evaluated + * with an owner that is not the enclosing class since otherwise locally + * defined symbols would be entered as class members. */ def annotContext(mdef: untpd.Tree, sym: Symbol)(using Context): Context = def isInner(owner: Symbol) = owner == sym || sym.is(Param) && owner == sym.owner val outer = ctx.outersIterator.dropWhile(c => isInner(c.owner)).next() - val adjusted = outer.property(ExprOwner) match { - case Some(exprOwner) if outer.owner.isClass => outer.exprContext(mdef, exprOwner) - case _ => outer - } - def local: FreshContext = adjusted.fresh.setOwner(newLocalDummy(sym.owner)) + def local: FreshContext = outer.fresh.setOwner(newLocalDummy(sym.owner)) sym.owner.infoOrCompleter match case completer: Namer#Completer if sym.is(Param) && completer.completerTypeParams(sym).nonEmpty => @@ -2254,7 +2244,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer // these type parameters. See i12953.scala for a test case. local.setScope(newScopeWith(completer.completerTypeParams(sym)*)) case _ => - if outer.owner.isClass then local else adjusted + if outer.owner.isClass then local else outer def completeAnnotations(mdef: untpd.MemberDef, sym: Symbol)(using Context): Unit = { // necessary to force annotation trees to be computed. @@ -3099,10 +3089,6 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer case nil => (buf.toList, ctx) } - val localCtx = { - val exprOwnerOpt = if (exprOwner == ctx.owner) None else Some(exprOwner) - ctx.withProperty(ExprOwner, exprOwnerOpt) - } def finalize(stat: Tree)(using Context): Tree = stat match { case stat: TypeDef if stat.symbol.is(Module) => val enumContext = enumContexts(stat.symbol.linkedClass) @@ -3115,7 +3101,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer case _ => stat } - val (stats0, finalCtx) = traverse(stats)(using localCtx) + val (stats0, finalCtx) = traverse(stats) val stats1 = stats0.mapConserve(finalize) if ctx.owner == exprOwner then checkNoTargetNameConflict(stats1) (stats1, finalCtx)