From 5a6fbc2619da4131e27d6e485ad56a6eabd6e452 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Tue, 9 Jun 2020 16:19:22 +0200 Subject: [PATCH 1/8] Fix incorrect code in phase Constructors `cls.copy` creates an unused new symbol. Instead, `cls.copySymDenotation` should be called. --- compiler/src/dotty/tools/dotc/transform/Constructors.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Constructors.scala b/compiler/src/dotty/tools/dotc/transform/Constructors.scala index 5e63f93e5d10..ac241b7e45ff 100644 --- a/compiler/src/dotty/tools/dotc/transform/Constructors.scala +++ b/compiler/src/dotty/tools/dotc/transform/Constructors.scala @@ -251,9 +251,9 @@ class Constructors extends MiniPhase with IdentityDenotTransformer { thisPhase = // Drop accessors that are not retained from class scope if (dropped.nonEmpty) { val clsInfo = cls.classInfo - cls.copy( - info = clsInfo.derivedClassInfo( - decls = clsInfo.decls.filteredScope(!dropped.contains(_)))) + val decls = clsInfo.decls.filteredScope(!dropped.contains(_)) + val clsInfo2 = clsInfo.derivedClassInfo(decls = decls) + cls.copySymDenotation(info = clsInfo2).installAfter(thisPhase) // TODO: this happens to work only because Constructors is the last phase in group } From cd6341e43f15a616f3fce8d5d93bbc652d25fd9a Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Wed, 10 Jun 2020 12:05:17 +0200 Subject: [PATCH 2/8] Enhance -Ycheck to check all members --- .../dotty/tools/dotc/transform/TreeChecker.scala | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index fadf853e77ad..c9d3b0a7287b 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -426,18 +426,19 @@ class TreeChecker extends Phase with SymTransformer { checkOwner(impl) checkOwner(impl.constr) - def isNonMagicalMethod(x: Symbol) = - x.is(Method) && + def isNonMagicalMember(x: Symbol) = !x.isValueClassConvertMethod && - !(x.is(Macro) && ctx.phase.refChecked) && !x.name.is(DocArtifactName) - val symbolsNotDefined = cls.classInfo.decls.toList.toSet.filter(isNonMagicalMethod) -- impl.body.map(_.symbol) - constr.symbol + val decls = cls.classInfo.decls.toList.toSet.filter(isNonMagicalMember) + val defined = impl.body.map(_.symbol) + + val symbolsNotDefined = decls -- defined - constr.symbol assert(symbolsNotDefined.isEmpty, i" $cls tree does not define methods: ${symbolsNotDefined.toList}%, %\n" + - i"expected: ${cls.classInfo.decls.toList.toSet.filter(isNonMagicalMethod)}%, %\n" + - i"defined: ${impl.body.map(_.symbol)}%, %") + i"expected: $decls%, %\n" + + i"defined: $defined%, %") super.typedClassDef(cdef, cls) } From d91e42f43a5de58683271aa0634b0ceddd26aa72 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Wed, 10 Jun 2020 22:36:41 +0200 Subject: [PATCH 3/8] Fix setter name for Scala 2 traits Solution: only use simple name for fields Before the change, given the following code class A extends App The compiler will generate the following setter in the phase Mixin: scala[Qualified $ App][Qualified $_setter_$ scala$App$$initCode_=] However, the field created by the getter will be: scala[Qualified $ App][Qualified $$ initCode][Suffix $$local] It will cause a problem in the phase Memoize in the line below: val field = sym.field.orElse(newField).asTerm The problem is that, for the setter, `sym.field` will be unable to find the field generated by the getter, due to the name mismatch. Consequently, the method `newField` will be called to create another field symbol without corresponding trees. After the change, the following field is generated instead: scala$App$$initCode[Suffix $$local] Now, the setter will be able to find the field via `sym.field`. This fix avoids generate a phantom symbol without corresponding trees. --- compiler/src/dotty/tools/dotc/core/NameOps.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/NameOps.scala b/compiler/src/dotty/tools/dotc/core/NameOps.scala index 2faf8d28f404..550f3243df6e 100644 --- a/compiler/src/dotty/tools/dotc/core/NameOps.scala +++ b/compiler/src/dotty/tools/dotc/core/NameOps.scala @@ -270,7 +270,7 @@ object NameOps { original.fieldName } else getterName.fieldName - else FieldName(name) + else FieldName(name.toSimpleName) def stripScala2LocalSuffix: TermName = if (name.isScala2LocalSuffix) name.asSimpleName.dropRight(1) else name From a664f52ffe55287c17b754e6943fa14c76f649c0 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Thu, 11 Jun 2020 00:20:30 +0200 Subject: [PATCH 4/8] Fix Mixin: remove Module flag in mixin synthesis Module flag will cause the companion class defined in a trait to be incorrectly entered as a member of the class that implement the trait. The method `Symbol.entered` has the logic to enter the companion class: if (this.is(Module)) this.owner.asClass.enter(this.moduleClass) The following is a simple test case: trait A{ object O } class B extends A { def foo = O } --- compiler/src/dotty/tools/dotc/transform/MixinOps.scala | 2 +- .../src/dotty/tools/dotc/transform/TreeChecker.scala | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/MixinOps.scala b/compiler/src/dotty/tools/dotc/transform/MixinOps.scala index 784a21ba248d..16a767cd1932 100644 --- a/compiler/src/dotty/tools/dotc/transform/MixinOps.scala +++ b/compiler/src/dotty/tools/dotc/transform/MixinOps.scala @@ -22,7 +22,7 @@ class MixinOps(cls: ClassSymbol, thisPhase: DenotTransformer)(implicit ctx: Cont val res = member.copy( owner = cls, name = member.name.stripScala2LocalSuffix, - flags = member.flags &~ Deferred | Synthetic | extraFlags, + flags = member.flags &~ Deferred &~ Module | Synthetic | extraFlags, info = cls.thisType.memberInfo(member)).enteredAfter(thisPhase).asTerm res.addAnnotations(member.annotations.filter(_.symbol != defn.TailrecAnnot)) res diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index c9d3b0a7287b..a52d8ef1ed9a 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -427,8 +427,8 @@ class TreeChecker extends Phase with SymTransformer { checkOwner(impl.constr) def isNonMagicalMember(x: Symbol) = - !x.isValueClassConvertMethod && - !x.name.is(DocArtifactName) + !x.isValueClassConvertMethod && + !x.name.is(DocArtifactName) val decls = cls.classInfo.decls.toList.toSet.filter(isNonMagicalMember) val defined = impl.body.map(_.symbol) @@ -436,9 +436,9 @@ class TreeChecker extends Phase with SymTransformer { val symbolsNotDefined = decls -- defined - constr.symbol assert(symbolsNotDefined.isEmpty, - i" $cls tree does not define methods: ${symbolsNotDefined.toList}%, %\n" + - i"expected: $decls%, %\n" + - i"defined: $defined%, %") + i" $cls tree does not define members: ${symbolsNotDefined.toList}%, %\n" + + i"expected: ${decls.toList}%, %\n" + + i"defined: ${defined}%, %") super.typedClassDef(cdef, cls) } From 44cb5fd64b87afe13abb7075c72477b342c17dae Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Thu, 11 Jun 2020 00:53:02 +0200 Subject: [PATCH 5/8] Use correct context for checking annotations Without this change, for the following code: class foo(x: Any) extends annotation.StaticAnnotation @foo({ val x = 3; x}) class A The compiler incorrectly enters `x` as a member of class `A`. --- compiler/src/dotty/tools/dotc/typer/Namer.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 3802bea63f2c..71198bb884bd 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -859,7 +859,7 @@ class Namer { typer: Typer => if (cls eq sym) ctx.error("An annotation class cannot be annotated with iself", annotTree.sourcePos) else { - val ann = Annotation.deferred(cls)(typedAnnotation(annotTree)) + val ann = Annotation.deferred(cls)(typedAnnotation(annotTree)(using annotCtx))(using annotCtx) sym.addAnnotation(ann) } } From 5d41e7815fc77cd2b9629cc86e7d4de878ab82a6 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Thu, 11 Jun 2020 11:02:09 +0200 Subject: [PATCH 6/8] Avoid retype checking of annotations Previously the method `typedAheadAnnotation` is never called, which results in retyping of annotations when type checking a class. The problem is exhibited in the test tests/pos/t7426.scala: class foo(x: Any) extends annotation.StaticAnnotation @foo(new AnyRef { }) trait A Retype checking the annotation would resulting in entering the same symbol for the annonymous class twice. Previously, it does not cause problem, because we use an incorrect context for typeAhead checking the annotation, where the symbol is incorrectly entered in the class `A`. In the typer, a duplicate symbol is entered in the empty package, thus there is no conflict. Since we never check that a class member has a corresponding tree, the bug is thus hidden. --- compiler/src/dotty/tools/dotc/typer/Namer.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 71198bb884bd..a47937f33e98 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -859,7 +859,7 @@ class Namer { typer: Typer => if (cls eq sym) ctx.error("An annotation class cannot be annotated with iself", annotTree.sourcePos) else { - val ann = Annotation.deferred(cls)(typedAnnotation(annotTree)(using annotCtx))(using annotCtx) + val ann = Annotation.deferred(cls)(typedAheadAnnotation(annotTree)(using annotCtx)) sym.addAnnotation(ann) } } From 235e393965bc9680f0d0c1a386f9d7c87a6b22d0 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Thu, 11 Jun 2020 11:31:06 +0200 Subject: [PATCH 7/8] Fix incorrect owner in ReifyQuotes The problem is exhibited in the following test case: tests/pos/i7052.scala It is not discovered before, because previously we are using the wrong context for typing ahead annotations. --- compiler/src/dotty/tools/dotc/transform/ReifyQuotes.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/ReifyQuotes.scala b/compiler/src/dotty/tools/dotc/transform/ReifyQuotes.scala index 7baf74ec90c9..7edbeb81201b 100644 --- a/compiler/src/dotty/tools/dotc/transform/ReifyQuotes.scala +++ b/compiler/src/dotty/tools/dotc/transform/ReifyQuotes.scala @@ -270,7 +270,7 @@ class ReifyQuotes extends MacroTransform { val tpe = MethodType(defn.SeqType.appliedTo(defn.AnyType) :: Nil, tree.tpe.widen) val meth = ctx.newSymbol(lambdaOwner, UniqueName.fresh(nme.ANON_FUN), Synthetic | Method, tpe) - Closure(meth, tss => body(tss.head.head)(ctx.withOwner(meth)).changeOwner(ctx.owner, meth)).withSpan(tree.span) + Closure(meth, tss => body(tss.head.head)(ctx.withOwner(meth)).changeNonLocalOwners(meth)).withSpan(tree.span) } private def transformWithCapturer(tree: Tree)(capturer: mutable.Map[Symbol, Tree] => Tree => Tree)(implicit ctx: Context): Tree = { From 3c61bcaae28b6969d5437e7a0498f4921904c77d Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Thu, 11 Jun 2020 13:50:00 +0200 Subject: [PATCH 8/8] Fix context for unpickling symbol annotations For member definitions, the owner for unpickling annotations should be the owner of the symbol. Note that the type checking has a more complex mechanism to avoid entering argument definitions in the enclosing class. See `Typer.annotContext` and `Context.exprContext`. In the context of unpickling, it is unclear how to get that mechanism to work. --- compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index 75b29cd4d78f..2e7822053056 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -578,7 +578,7 @@ class TreeUnpickler(reader: TastyReader, else ctx.newSymbol(ctx.owner, name, flags, completer, privateWithin, coord) } - sym.annotations = annotFns.map(_(sym)) + sym.annotations = annotFns.map(_(sym.owner)) if sym.isOpaqueAlias then sym.setFlag(Deferred) val isScala2MacroDefinedInScala3 = flags.is(Macro, butNot = Inline) && flags.is(Erased) ctx.owner match {