From 16184eb429bfe12b942d52bc8cdd9417a542cd7c Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 26 Jun 2019 19:34:32 +0200 Subject: [PATCH 01/21] Fix classpath when running `dotc -with-compiler` from sbt --- project/Build.scala | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/project/Build.scala b/project/Build.scala index d31856935194..21ca977e6177 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -636,16 +636,21 @@ object Build { else if (debugFromTasty) "dotty.tools.dotc.fromtasty.Debug" else "dotty.tools.dotc.Main" - var extraClasspath = s"$scalaLib${File.pathSeparator}$dottyLib" - if ((decompile || printTasty) && !args.contains("-classpath")) extraClasspath += s"${File.pathSeparator}." + var extraClasspath = Seq(scalaLib, dottyLib) + + if ((decompile || printTasty) && !args.contains("-classpath")) + extraClasspath ++= Seq(".") + if (args0.contains("-with-compiler")) { if (scalaVersion.value == referenceVersion) { log.error("-with-compiler should only be used with a bootstrapped compiler") } - extraClasspath += s"${File.pathSeparator}$dottyCompiler" + val dottyInterfaces = jars("dotty-interfaces") + val asm = findLib(attList, "scala-asm") + extraClasspath ++= Seq(dottyCompiler, dottyInterfaces, asm) } - val fullArgs = main :: insertClasspathInArgs(args, extraClasspath) + val fullArgs = main :: insertClasspathInArgs(args, extraClasspath.mkString(File.pathSeparator)) (runMain in Compile).toTask(fullArgs.mkString(" ", " ", "")) } From e7e37187df122a5d51a2b594e739beb93b2e114d Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 26 Jun 2019 19:21:02 +0200 Subject: [PATCH 02/21] Add -Ydebug-type-error to print the stack when a TypeError is caught --- .../tools/dotc/ast/TreeMapWithImplicits.scala | 2 +- .../dotty/tools/dotc/config/ScalaSettings.scala | 1 + .../dotty/tools/dotc/reporting/Reporter.scala | 7 +++++++ .../src/dotty/tools/dotc/sbt/ExtractAPI.scala | 2 +- .../tools/dotc/transform/MacroTransform.scala | 2 +- .../dotty/tools/dotc/transform/MegaPhase.scala | 4 ++-- .../tools/dotc/transform/OverridingPairs.scala | 2 +- .../dotty/tools/dotc/typer/ErrorReporting.scala | 16 ++++++++++++---- .../src/dotty/tools/dotc/typer/RefChecks.scala | 2 +- compiler/src/dotty/tools/dotc/typer/Typer.scala | 4 ++-- .../dotty/tools/dotc/typer/VarianceChecker.scala | 2 +- 11 files changed, 30 insertions(+), 14 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeMapWithImplicits.scala b/compiler/src/dotty/tools/dotc/ast/TreeMapWithImplicits.scala index 06db968a7a3d..2e03b44c3b1d 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeMapWithImplicits.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeMapWithImplicits.scala @@ -119,7 +119,7 @@ class TreeMapWithImplicits extends tpd.TreeMap { } catch { case ex: TypeError => - ctx.error(ex.toMessage, tree.sourcePos) + ctx.error(ex, tree.sourcePos) tree } } diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index a3157b1fb396..4171ff2c5886 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -103,6 +103,7 @@ class ScalaSettings extends Settings.SettingGroup { val YdebugNames: Setting[Boolean] = BooleanSetting("-Ydebug-names", "Show internal representation of names") val YdebugPos: Setting[Boolean] = BooleanSetting("-Ydebug-pos", "Show full source positions including spans") val YdebugTreeWithId: Setting[Int] = IntSetting("-Ydebug-tree-with-id", "Print the stack trace when the tree with the given id is created", Int.MinValue) + val YdebugTypeError: Setting[Boolean] = BooleanSetting("-Ydebug-type-error", "Print the stack trace when a TypeError is caught", false) val YtermConflict: Setting[String] = ChoiceSetting("-Yresolve-term-conflict", "strategy", "Resolve term conflicts", List("package", "object", "error"), "error") val Ylog: Setting[List[String]] = PhasesSetting("-Ylog", "Log operations during") val YemitTastyInClass: Setting[Boolean] = BooleanSetting("-Yemit-tasty-in-class", "Generate tasty in the .class file and add an empty *.hasTasty file.") diff --git a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala index b742d59d392e..006567d89371 100644 --- a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala +++ b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala @@ -5,6 +5,7 @@ package reporting import scala.annotation.internal.sharable import core.Contexts._ +import core.TypeError import util.{SourcePosition, NoSourcePosition} import core.Decorators.PhaseListDecorator import collection.mutable @@ -136,6 +137,12 @@ trait Reporting { this: Context => reporter.report(if (sticky) new StickyError(msg, fullPos) else new Error(msg, fullPos)) } + def error(ex: TypeError, pos: SourcePosition): Unit = { + error(ex.toMessage, pos, sticky = true) + if (ctx.settings.YdebugTypeError.value) + ex.printStackTrace + } + def errorOrMigrationWarning(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit = if (ctx.scala2Mode) migrationWarning(msg, pos) else error(msg, pos) diff --git a/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala b/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala index e3adbb68472c..907c859e5c5c 100644 --- a/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala +++ b/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala @@ -247,7 +247,7 @@ private class ExtractAPICollector(implicit val ctx: Context) extends ThunkHolder case ex: TypeError => // See neg/i1750a for an example where a cyclic error can arise. // The root cause in this example is an illegal "override" of an inner trait - ctx.error(ex.toMessage, csym.sourcePos, sticky = true) + ctx.error(ex, csym.sourcePos) defn.ObjectType :: Nil } if (ValueClasses.isDerivedValueClass(csym)) { diff --git a/compiler/src/dotty/tools/dotc/transform/MacroTransform.scala b/compiler/src/dotty/tools/dotc/transform/MacroTransform.scala index d567d48461e4..78a3be662bbd 100644 --- a/compiler/src/dotty/tools/dotc/transform/MacroTransform.scala +++ b/compiler/src/dotty/tools/dotc/transform/MacroTransform.scala @@ -64,7 +64,7 @@ abstract class MacroTransform extends Phase { } catch { case ex: TypeError => - ctx.error(ex.toMessage, tree.sourcePos, sticky = true) + ctx.error(ex, tree.sourcePos) tree } diff --git a/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala b/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala index b93be0f09a95..d5b101043218 100644 --- a/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala +++ b/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala @@ -170,7 +170,7 @@ class MegaPhase(val miniPhases: Array[MiniPhase]) extends Phase { } catch { case ex: TypeError => - ctx.error(ex.toMessage, tree.sourcePos) + ctx.error(ex, tree.sourcePos) tree } def goUnnamed(tree: Tree, start: Int) = @@ -205,7 +205,7 @@ class MegaPhase(val miniPhases: Array[MiniPhase]) extends Phase { } catch { case ex: TypeError => - ctx.error(ex.toMessage, tree.sourcePos, sticky = true) + ctx.error(ex, tree.sourcePos) tree } if (tree.isInstanceOf[NameTree]) goNamed(tree, start) else goUnnamed(tree, start) diff --git a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala index bf7d9481d6d9..4fc4355752e3 100644 --- a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala +++ b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala @@ -134,7 +134,7 @@ object OverridingPairs { case ex: TypeError => // See neg/i1750a for an example where a cyclic error can arise. // The root cause in this example is an illegal "override" of an inner trait - ctx.error(ex.toMessage, base.sourcePos, sticky = true) + ctx.error(ex, base.sourcePos) } } else { curEntry = curEntry.prev diff --git a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala index ab72bdd2f5b8..ce9ba2ab7b60 100644 --- a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala +++ b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala @@ -16,17 +16,25 @@ object ErrorReporting { import tpd._ - def errorTree(tree: untpd.Tree, msg: => Message, pos: SourcePosition, sticky: Boolean = false)(implicit ctx: Context): tpd.Tree = - tree.withType(errorType(msg, pos, sticky)) + def errorTree(tree: untpd.Tree, msg: => Message, pos: SourcePosition)(implicit ctx: Context): tpd.Tree = + tree.withType(errorType(msg, pos)) def errorTree(tree: untpd.Tree, msg: => Message)(implicit ctx: Context): tpd.Tree = errorTree(tree, msg, tree.sourcePos) - def errorType(msg: => Message, pos: SourcePosition, sticky: Boolean = false)(implicit ctx: Context): ErrorType = { - ctx.error(msg, pos, sticky) + def errorTree(tree: untpd.Tree, msg: TypeError, pos: SourcePosition)(implicit ctx: Context): tpd.Tree = + tree.withType(errorType(msg, pos)) + + def errorType(msg: => Message, pos: SourcePosition)(implicit ctx: Context): ErrorType = { + ctx.error(msg, pos) ErrorType(msg) } + def errorType(ex: TypeError, pos: SourcePosition)(implicit ctx: Context): ErrorType = { + ctx.error(ex, pos) + ErrorType(ex.toMessage) + } + def wrongNumberOfTypeArgs(fntpe: Type, expectedArgs: List[ParamInfo], actual: List[untpd.Tree], pos: SourcePosition)(implicit ctx: Context): ErrorType = errorType(WrongNumberOfTypeArgs(fntpe, expectedArgs, actual)(ctx), pos) diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 9b83ddcf02c9..eb06127580eb 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -971,7 +971,7 @@ class RefChecks extends MiniPhase { thisPhase => tree } catch { case ex: TypeError => - ctx.error(ex.toMessage, tree.sourcePos, sticky = true) + ctx.error(ex, tree.sourcePos) tree } diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 6d6404fb7316..271922a61dc1 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2419,7 +2419,7 @@ class Typer extends Namer try adapt(typedUnadapted(tree, pt, locked), pt, locked) catch { case ex: TypeError => - errorTree(tree, ex.toMessage, tree.sourcePos.focus, sticky = true) + errorTree(tree, ex, tree.sourcePos.focus) // This uses tree.span.focus instead of the default tree.span, because: // - since tree can be a top-level definition, tree.span can point to the whole definition // - that would in turn hide all other type errors inside tree. @@ -3195,7 +3195,7 @@ class Typer extends Namer } } catch { - case ex: TypeError => errorTree(tree, ex.toMessage, tree.sourcePos, sticky = true) + case ex: TypeError => errorTree(tree, ex, tree.sourcePos) } val nestedCtx = ctx.fresh.setNewTyperState() val app = tryExtension(nestedCtx) diff --git a/compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala b/compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala index aa44f8178b33..5987419e7b0a 100644 --- a/compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala +++ b/compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala @@ -214,7 +214,7 @@ class VarianceChecker()(implicit ctx: Context) { case _ => } catch { - case ex: TypeError => ctx.error(ex.toMessage, tree.sourcePos.focus) + case ex: TypeError => ctx.error(ex, tree.sourcePos.focus) } } } From 4050dd2cc03ab519ad3fc92d00fcb7bdb08fb92d Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 2 Jul 2019 14:27:20 +0200 Subject: [PATCH 03/21] Make JavaDefined an AfterLoadFlags It can't be a FromStartFlags since we don't know whether a top-level class is Java-defined before completing it once. --- compiler/src/dotty/tools/dotc/core/Flags.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/Flags.scala b/compiler/src/dotty/tools/dotc/core/Flags.scala index 1288171e2ec8..5cccde7e51c7 100644 --- a/compiler/src/dotty/tools/dotc/core/Flags.scala +++ b/compiler/src/dotty/tools/dotc/core/Flags.scala @@ -452,7 +452,7 @@ object Flags { * is completed) */ val AfterLoadFlags: FlagSet = commonFlags( - FromStartFlags, AccessFlags, Final, AccessorOrSealed, LazyOrTrait, SelfName) + FromStartFlags, AccessFlags, Final, AccessorOrSealed, LazyOrTrait, SelfName, JavaDefined) /** A value that's unstable unless complemented with a Stable flag */ From 98cd1799482c8cb35182648039f1c7a751e897c7 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Fri, 5 Jul 2019 22:44:45 +0200 Subject: [PATCH 04/21] Better default toString for LazyType LazyType inherits the toString of Function2 which is just "" and therefore not very helpful. By contrast, the exact class of a completer can be very useful when debugging. --- compiler/src/dotty/tools/dotc/core/SymDenotations.scala | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 19f7ee238d99..ab975dad6730 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -2174,6 +2174,8 @@ object SymDenotations { def withDecls(decls: Scope): this.type = { myDecls = decls; this } def withSourceModule(sourceModuleFn: Context => Symbol): this.type = { mySourceModuleFn = sourceModuleFn; this } def withModuleClass(moduleClassFn: Context => Symbol): this.type = { myModuleClassFn = moduleClassFn; this } + + override def toString: String = getClass.toString } /** A subtrait of LazyTypes where completerTypeParams yields a List[TypeSymbol], which From bdc5b6fac70d537047e61a44b055a9c244152b50 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 27 Jun 2019 17:22:11 +0200 Subject: [PATCH 05/21] Move "is this a value?" check from Typer to Erasure Doing the check in Typer leads to cyclic errors, and I can't figure out a nice way to do this in any other phase since it requires knowledge about the expected type. --- .../dotty/tools/dotc/transform/Erasure.scala | 16 ++++++++++++++-- .../tools/dotc/transform/TreeChecker.scala | 5 ++++- .../src/dotty/tools/dotc/typer/Checking.scala | 13 ------------- .../src/dotty/tools/dotc/typer/Typer.scala | 3 +-- .../dotc/reporting/ErrorMessagesTests.scala | 18 +----------------- tests/fuzzy/parser-stability-15.scala | 6 ++++++ tests/neg/parser-stability-15.scala | 7 ------- 7 files changed, 26 insertions(+), 42 deletions(-) create mode 100644 tests/fuzzy/parser-stability-15.scala delete mode 100644 tests/neg/parser-stability-15.scala diff --git a/compiler/src/dotty/tools/dotc/transform/Erasure.scala b/compiler/src/dotty/tools/dotc/transform/Erasure.scala index 55e8008f0363..1f36d76a151b 100644 --- a/compiler/src/dotty/tools/dotc/transform/Erasure.scala +++ b/compiler/src/dotty/tools/dotc/transform/Erasure.scala @@ -328,6 +328,18 @@ object Erasure { case _ => tree.symbol.isEffectivelyErased } + /** Check that Java statics and packages can only be used in selections. + */ + private def checkValue(tree: Tree, proto: Type)(implicit ctx: Context): tree.type = { + if (!proto.isInstanceOf[SelectionProto] && !proto.isInstanceOf[ApplyingProto]) { + val sym = tree.tpe.termSymbol + // The check is avoided inside Java compilation units because it always fails + // on the singleton type Module.type. + if ((sym is Flags.Package) || (sym.isAllOf(Flags.JavaModule) && !ctx.compilationUnit.isJava)) ctx.error(reporting.diagnostic.messages.JavaSymbolIsNotAValue(sym), tree.sourcePos) + } + tree + } + private def checkNotErased(tree: Tree)(implicit ctx: Context): tree.type = { if (!ctx.mode.is(Mode.Type)) { if (isErased(tree)) @@ -390,7 +402,7 @@ object Erasure { super.typedLiteral(tree) override def typedIdent(tree: untpd.Ident, pt: Type)(implicit ctx: Context): Tree = { - checkNotErased(super.typedIdent(tree, pt)) + checkValue(checkNotErased(super.typedIdent(tree, pt)), pt) } /** Type check select nodes, applying the following rewritings exhaustively @@ -484,7 +496,7 @@ object Erasure { } } - checkNotErased(recur(qual1)) + checkValue(checkNotErased(recur(qual1)), pt) } override def typedThis(tree: untpd.This)(implicit ctx: Context): Tree = diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index 730da592de2f..9a221390da34 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -263,7 +263,10 @@ class TreeChecker extends Phase with SymTransformer { override def typed(tree: untpd.Tree, pt: Type = WildcardType)(implicit ctx: Context): Tree = { val tpdTree = super.typed(tree, pt) - checkIdentNotJavaClass(tpdTree) + if (ctx.erasedTypes) + // Can't be checked in earlier phases since `checkValue` is only run in + // Erasure (because running it in Typer would force too much) + checkIdentNotJavaClass(tpdTree) tpdTree } diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index eddff408ff6f..3a18d41e7fba 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -594,18 +594,6 @@ trait Checking { def checkNonCyclicInherited(joint: Type, parents: List[Type], decls: Scope, posd: Positioned)(implicit ctx: Context): Unit = Checking.checkNonCyclicInherited(joint, parents, decls, posd) - /** Check that Java statics and packages can only be used in selections. - */ - def checkValue(tree: Tree, proto: Type)(implicit ctx: Context): tree.type = { - if (!proto.isInstanceOf[SelectionProto] && !proto.isInstanceOf[ApplyingProto]) { - val sym = tree.tpe.termSymbol - // The check is avoided inside Java compilation units because it always fails - // on the singleton type Module.type. - if ((sym is Package) || (sym.isAllOf(JavaModule) && !ctx.compilationUnit.isJava)) ctx.error(JavaSymbolIsNotAValue(sym), tree.sourcePos) - } - tree - } - /** Check that type `tp` is stable. */ def checkStable(tp: Type, pos: SourcePosition)(implicit ctx: Context): Unit = if (!tp.isStable) ctx.error(ex"$tp is not stable", pos) @@ -1171,7 +1159,6 @@ trait NoChecking extends ReChecking { import tpd._ override def checkNonCyclic(sym: Symbol, info: TypeBounds, reportErrors: Boolean)(implicit ctx: Context): Type = info override def checkNonCyclicInherited(joint: Type, parents: List[Type], decls: Scope, posd: Positioned)(implicit ctx: Context): Unit = () - override def checkValue(tree: Tree, proto: Type)(implicit ctx: Context): tree.type = tree override def checkStable(tp: Type, pos: SourcePosition)(implicit ctx: Context): Unit = () override def checkClassType(tp: Type, pos: SourcePosition, traitReq: Boolean, stablePrefixReq: Boolean)(implicit ctx: Context): Type = tp override def checkImplicitConversionDefOK(sym: Symbol)(implicit ctx: Context): Unit = () diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 271922a61dc1..6857bb901b55 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -429,7 +429,6 @@ class Typer extends Namer } checkStableIdentPattern(tree1, pt) - checkValue(tree1, pt) } /** Check that a stable identifier pattern is indeed stable (SLS 8.1.5) @@ -453,7 +452,7 @@ class Typer extends Namer } case qual => if (tree.name.isTypeName) checkStable(qual.tpe, qual.sourcePos) - val select = checkValue(assignType(cpy.Select(tree)(qual, tree.name), qual), pt) + val select = assignType(cpy.Select(tree)(qual, tree.name), qual) if (select.tpe ne TryDynamicCallType) ConstFold(checkStableIdentPattern(select, pt)) else if (pt.isInstanceOf[FunOrPolyProto] || pt == AssignProto) select else typedDynamicSelect(tree, Nil, pt) diff --git a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala index 9285e53f7042..d0ec7fbc87e6 100644 --- a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala +++ b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala @@ -7,7 +7,7 @@ import dotty.tools.dotc.core.Contexts.Context import dotty.tools.dotc.core.Types.WildcardType import dotty.tools.dotc.parsing.Tokens import dotty.tools.dotc.reporting.diagnostic.messages._ -import dotty.tools.dotc.transform.{CheckStatic, PostTyper, TailRec} +import dotty.tools.dotc.transform.{CheckStatic, Erasure, PostTyper, TailRec} import dotty.tools.dotc.typer.{FrontEnd, RefChecks} import org.junit.Assert._ import org.junit.Test @@ -1475,22 +1475,6 @@ class ErrorMessagesTests extends ErrorMessagesTest { assertEquals("class Object", parentSym.show) } - @Test def javaSymbolIsNotAValue = - checkMessagesAfter(CheckStatic.name) { - """ - |package p - |object O { - | val v = p - |} - """.stripMargin - }.expect { (itcx, messages) => - implicit val ctx: Context = itcx - - assertMessageCount(1, messages) - val JavaSymbolIsNotAValue(symbol) = messages.head - assertEquals(symbol.show, "package p") - } - @Test def i3187 = checkMessagesAfter(GenBCode.name) { """ diff --git a/tests/fuzzy/parser-stability-15.scala b/tests/fuzzy/parser-stability-15.scala new file mode 100644 index 000000000000..674b8f9d3dc8 --- /dev/null +++ b/tests/fuzzy/parser-stability-15.scala @@ -0,0 +1,6 @@ +package x0 +class x0 { +def x1 = +x2 match +] +case x3[] => x0 diff --git a/tests/neg/parser-stability-15.scala b/tests/neg/parser-stability-15.scala deleted file mode 100644 index acfe017cd39e..000000000000 --- a/tests/neg/parser-stability-15.scala +++ /dev/null @@ -1,7 +0,0 @@ -package x0 -class x0 { -def x1 = -x2 match // error -] // error -case x3[] => x0 // error // error -// error \ No newline at end of file From 85f0f4f0661f8efc955e7c9b1667bb0e7c2d15ff Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 5 Jun 2019 14:58:23 +0200 Subject: [PATCH 06/21] Consistently use markAbsent/isAbsent --- .../src/dotty/tools/dotc/core/Definitions.scala | 13 +++++++------ .../src/dotty/tools/dotc/core/SymDenotations.scala | 2 +- compiler/src/dotty/tools/dotc/typer/Namer.scala | 4 ++-- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index b257aa62d362..f1d705b9674c 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -189,7 +189,7 @@ class Definitions { private def completeClass(cls: ClassSymbol, ensureCtor: Boolean = true): ClassSymbol = { if (ensureCtor) ensureConstructor(cls, EmptyScope) - if (cls.linkedClass.exists) cls.linkedClass.info = NoType + if (cls.linkedClass.exists) cls.linkedClass.markAbsent() cls } @@ -296,12 +296,13 @@ class Definitions { cls.info = ClassInfo(cls.owner.thisType, cls, AnyClass.typeRef :: Nil, newScope) cls.setFlag(NoInits) - // The companion object doesn't really exist, `NoType` is the general - // technique to do that. Here we need to set it before completing - // attempt to load Object's classfile, which causes issue #1648. + // The companion object doesn't really exist, so it needs to be marked as + // absent. Here we need to set it before completing attempt to load Object's + // classfile, which causes issue #1648. val companion = JavaLangPackageVal.info.decl(nme.Object).symbol - companion.moduleClass.info = NoType // to indicate that it does not really exist - companion.info = NoType // to indicate that it does not really exist + companion.moduleClass.markAbsent() + companion.markAbsent() + completeClass(cls) } def ObjectType: TypeRef = ObjectClass.typeRef diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index ab975dad6730..6afcdb13c799 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -792,7 +792,7 @@ object SymDenotations { } if (pre eq NoPrefix) true - else if (info eq NoType) false + else if (isAbsent) false else { val boundary = accessBoundary(owner) diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 7831f2358e4a..a4dc90650a24 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -459,7 +459,7 @@ class Namer { typer: Typer => def invalidate(name: TypeName) = if (!(definedNames contains name)) { val member = pkg.info.decl(name).asSymDenotation - if (member.isClass && !member.is(Package)) member.info = NoType + if (member.isClass && !(member.is(Package))) member.markAbsent() } xstats foreach { case stat: TypeDef if stat.isClassDef => @@ -828,7 +828,7 @@ class Namer { typer: Typer => alt != denot.symbol && alt.info.matchesLoosely(denot.info)) if (isClashingSynthetic) { typr.println(i"invalidating clashing $denot in ${denot.owner}") - denot.info = NoType + denot.markAbsent() } } From eb8e8ff6480ee10fec1bd1efbc1dc145ad63cea9 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 4 Jul 2019 18:22:26 +0200 Subject: [PATCH 07/21] Move SymDenotation#proxy to SymbolLoader#proxy It's only ever used on SymbolLoaders, and more importantly it means that a proxy to a SymbolLoader is now also a SymbolLoader which we'll use to enforce restrictions on which completers are allowed to set `privateWithin` in a later commit. --- .../src/dotty/tools/dotc/core/SymDenotations.scala | 7 ------- .../src/dotty/tools/dotc/core/SymbolLoaders.scala | 14 +++++++++++--- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 6afcdb13c799..42297607869b 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -2155,13 +2155,6 @@ object SymDenotations { private[this] var mySourceModuleFn: Context => Symbol = NoSymbolFn private[this] var myModuleClassFn: Context => Symbol = NoSymbolFn - /** A proxy to this lazy type that keeps the complete operation - * but provides fresh slots for scope/sourceModule/moduleClass - */ - def proxy: LazyType = new LazyType { - override def complete(denot: SymDenotation)(implicit ctx: Context) = self.complete(denot) - } - /** The type parameters computed by the completer before completion has finished */ def completerTypeParams(sym: Symbol)(implicit ctx: Context): List[TypeParamInfo] = if (sym.is(Touched)) Nil // return `Nil` instead of throwing a cyclic reference diff --git a/compiler/src/dotty/tools/dotc/core/SymbolLoaders.scala b/compiler/src/dotty/tools/dotc/core/SymbolLoaders.scala index 55f7409f91ec..7512328ad2ca 100644 --- a/compiler/src/dotty/tools/dotc/core/SymbolLoaders.scala +++ b/compiler/src/dotty/tools/dotc/core/SymbolLoaders.scala @@ -200,6 +200,7 @@ object SymbolLoaders { */ class PackageLoader(_sourceModule: TermSymbol, classPath: ClassPath) extends SymbolLoader { + override def sourceFileOrNull: AbstractFile = null override def sourceModule(implicit ctx: Context): TermSymbol = _sourceModule def description(implicit ctx: Context): String = "package loader " + sourceModule.fullName @@ -302,18 +303,25 @@ object SymbolLoaders { /** A lazy type that completes itself by calling parameter doComplete. * Any linked modules/classes or module classes are also initialized. */ -abstract class SymbolLoader extends LazyType { - +abstract class SymbolLoader extends LazyType { self => /** Load source or class file for `root`, return */ def doComplete(root: SymDenotation)(implicit ctx: Context): Unit - def sourceFileOrNull: AbstractFile = null + def sourceFileOrNull: AbstractFile /** Description of the resource (ClassPath, AbstractFile) * being processed by this loader */ def description(implicit ctx: Context): String + /** A proxy to this loader that keeps the doComplete operation + * but provides fresh slots for scope/sourceModule/moduleClass + */ + def proxy: SymbolLoader = new SymbolLoader { + export self.{doComplete, sourceFileOrNull} + def description(implicit ctx: Context): String = "proxy to ${self.description}" + } + override def complete(root: SymDenotation)(implicit ctx: Context): Unit = { def signalError(ex: Exception): Unit = { if (ctx.debug) ex.printStackTrace() From 5c37a38396af6ac4b283914141694258040edb10 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Fri, 5 Jul 2019 22:15:07 +0200 Subject: [PATCH 08/21] Add new SymbolLoader: NoLoader When a SymbolLoader gets completed, we sometimes use NoCompleter as an intermediate info, we now use NoLoader instead. This will be useful in the next commit where we restrict which completers are allowed to set `isAbsent`. We must allow SymbolLoader to set `isAbsent` , but we don't want to allow any NoCompleter to set it, so instead we use NoLoader which is a subtype of both SymbolLoader and NoCompleter. It also turns out that even without the changes to `isAbsent`, using a `SymbolLoader` here is a good idea since `SymDenotation#isCurrent` handles SymbolLoader specially and did not take into account that they were sometimes replaced by NoCompleter. This means that checking for the presence of a flag can now lead to more completion, which required adding an early exit in `Types#isArgPrefixOf` to avoid a cyclic reference. --- .../src/dotty/tools/dotc/core/SymDenotations.scala | 4 ++-- .../src/dotty/tools/dotc/core/SymbolLoaders.scala | 12 +++++++++++- compiler/src/dotty/tools/dotc/core/Types.scala | 1 + .../tools/dotc/core/classfile/ClassfileParser.scala | 4 ++-- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 42297607869b..a174a034c7d2 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -2180,11 +2180,11 @@ object SymDenotations { } /** A missing completer */ - @sharable class NoCompleter extends LazyType { + trait NoCompleter extends LazyType { def complete(denot: SymDenotation)(implicit ctx: Context): Unit = unsupported("complete") } - object NoCompleter extends NoCompleter + @sharable object NoCompleter extends NoCompleter /** A lazy type for modules that points to the module class. * Needed so that `moduleClass` works before completion. diff --git a/compiler/src/dotty/tools/dotc/core/SymbolLoaders.scala b/compiler/src/dotty/tools/dotc/core/SymbolLoaders.scala index 7512328ad2ca..ca6b9ccdc9c9 100644 --- a/compiler/src/dotty/tools/dotc/core/SymbolLoaders.scala +++ b/compiler/src/dotty/tools/dotc/core/SymbolLoaders.scala @@ -375,7 +375,7 @@ abstract class SymbolLoader extends LazyType { self => else ctx.newModuleSymbol( rootDenot.owner, rootDenot.name.toTermName, Synthetic, Synthetic, - (module, _) => new NoCompleter() withDecls newScope withSourceModule (_ => module)) + (module, _) => NoLoader().withDecls(newScope).withSourceModule(_ => module)) .moduleClass.denot.asClass } if (rootDenot.is(ModuleClass)) (linkedDenot, rootDenot) @@ -416,3 +416,13 @@ class SourcefileLoader(val srcfile: AbstractFile) extends SymbolLoader { def doComplete(root: SymDenotation)(implicit ctx: Context): Unit = ctx.run.lateCompile(srcfile, typeCheck = ctx.settings.YretainTrees.value) } + +/** A NoCompleter which is also a SymbolLoader. */ +class NoLoader extends SymbolLoader with NoCompleter { + def description(implicit ctx: Context): String = "NoLoader" + override def sourceFileOrNull: AbstractFile = null + override def complete(root: SymDenotation)(implicit ctx: Context): Unit = + super[NoCompleter].complete(root) + def doComplete(root: SymDenotation)(implicit ctx: Context): Unit = + unsupported("doComplete") +} diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index bea3f53df929..ddad9f743c6f 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -235,6 +235,7 @@ object Types { * from the ThisType of `symd`'s owner. */ def isArgPrefixOf(symd: SymDenotation)(implicit ctx: Context): Boolean = + symd.exists && !symd.owner.is(Package) && // Early exit if possible because the next check would force SymbolLoaders symd.isAllOf(ClassTypeParam) && { this match { case tp: ThisType => tp.cls ne symd.owner diff --git a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala index a4ddf9d6b5b9..ddc060241288 100644 --- a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala +++ b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala @@ -72,8 +72,8 @@ class ClassfileParser( private[this] var Scala2UnpicklingMode = Mode.Scala2Unpickling - classRoot.info = (new NoCompleter).withDecls(instanceScope) - moduleRoot.info = (new NoCompleter).withDecls(staticScope).withSourceModule(_ => staticModule) + classRoot.info = NoLoader().withDecls(instanceScope) + moduleRoot.info = NoLoader().withDecls(staticScope).withSourceModule(_ => staticModule) private def currentIsTopLevel(implicit ctx: Context) = classRoot.owner.is(Flags.PackageClass) From a92a209760abea78d44df54aad28fad3b82aa2ed Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Fri, 5 Jul 2019 23:39:40 +0200 Subject: [PATCH 09/21] Make isAbsent force less After the previous commits, only completers for ModuleCompleter and SymbolLoader actually need to be completed to avoid isAbsent returning the wrong result. A version if `isAbsent` that doesn't force at all is still necessary, but instead of having a separate `unforcedIsAbsent` method we roll its functionality into `isAbsent` which gains a `canForce` parameter --- .../tools/dotc/core/SymDenotations.scala | 65 +++++++++++++------ .../dotc/core/classfile/ClassfileParser.scala | 2 +- .../tools/dotc/interactive/Completion.scala | 2 +- .../tools/dotc/sbt/ExtractDependencies.scala | 4 +- .../dotty/tools/dotc/transform/Pickler.scala | 2 +- .../tools/dotc/transform/TreeChecker.scala | 2 +- .../src/dotty/tools/dotc/typer/Checking.scala | 2 +- .../dotty/tools/dotc/typer/TypeAssigner.scala | 2 +- 8 files changed, 54 insertions(+), 27 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index a174a034c7d2..255f7b634402 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -353,7 +353,21 @@ object SymDenotations { /** The completer of this denotation. @pre: Denotation is not yet completed */ final def completer: LazyType = myInfo.asInstanceOf[LazyType] - /** Make sure this denotation is completed */ + /** If this denotation is not completed, run the completer. + * The resulting info might be another completer. + * + * @see ensureCompleted + */ + final def completeOnce()(implicit ctx: Context): Unit = myInfo match { + case myInfo: LazyType => + completeFrom(myInfo) + case _ => + } + + /** Make sure this denotation is fully completed. + * + * @see completeOnce + */ final def ensureCompleted()(implicit ctx: Context): Unit = info /** The symbols defined in this class or object. @@ -370,7 +384,7 @@ object SymDenotations { case cinfo: LazyType => val knownDecls = cinfo.decls if (knownDecls ne EmptyScope) knownDecls - else { completeFrom(cinfo); unforcedDecls } // complete-once + else { completeOnce(); unforcedDecls } case _ => info.decls } @@ -505,20 +519,33 @@ object SymDenotations { /** is this symbol the result of an erroneous definition? */ def isError: Boolean = false - /** Make denotation not exist */ - final def markAbsent(): Unit = + /** Make denotation not exist. + * @pre `isCompleting` is false, or this is a ModuleCompleter or SymbolLoader + */ + final def markAbsent()(implicit ctx: Context): Unit = { + if (isCompleting) + assert(myInfo.isInstanceOf[ModuleCompleter | SymbolLoader], + s"Illegal call to `markAbsent()` while completing $this using completer $myInfo") myInfo = NoType + } - /** Is symbol known to not exist, or potentially not completed yet? */ - final def unforcedIsAbsent(implicit ctx: Context): Boolean = - myInfo == NoType || - (this.is(ModuleVal, butNot = Package)) && moduleClass.unforcedIsAbsent - - /** Is symbol known to not exist? */ - final def isAbsent(implicit ctx: Context): Boolean = { - ensureCompleted() - (myInfo `eq` NoType) || - (this.is(ModuleVal, butNot = Package)) && moduleClass.isAbsent + /** Is symbol known to not exist? + * @param canForce If this is true, the info may be forced to avoid a false-negative result + */ + @tailrec + final def isAbsent(canForce: Boolean = true)(implicit ctx: Context): Boolean = myInfo match { + case myInfo: ModuleCompleter => + // Instead of completing the ModuleCompleter, we can check whether + // the module class is absent, which might require less completions. + myInfo.moduleClass.isAbsent(canForce) + case _: SymbolLoader if canForce => + // Completing a SymbolLoader might call `markAbsent()` + completeOnce() + isAbsent(canForce) + case _ => + // Otherwise, no completion is necessary, see the preconditions of `markAbsent()`. + (myInfo `eq` NoType) || + is(ModuleVal, butNot = Package) && moduleClass.isAbsent(canForce) } /** Is this symbol the root class or its companion object? */ @@ -639,7 +666,7 @@ object SymDenotations { final def isCoDefinedWith(other: Symbol)(implicit ctx: Context): Boolean = (this.effectiveOwner == other.effectiveOwner) && ( !this.effectiveOwner.is(PackageClass) - || this.unforcedIsAbsent || other.unforcedIsAbsent + || this.isAbsent(canForce = false) || other.isAbsent(canForce = false) || { // check if they are defined in the same file(or a jar) val thisFile = this.symbol.associatedFile val thatFile = other.associatedFile @@ -792,7 +819,7 @@ object SymDenotations { } if (pre eq NoPrefix) true - else if (isAbsent) false + else if (isAbsent()) false else { val boundary = accessBoundary(owner) @@ -1611,7 +1638,7 @@ object SymDenotations { } final override def derivesFrom(base: Symbol)(implicit ctx: Context): Boolean = - !isAbsent && + !isAbsent() && base.isClass && ( (symbol eq base) || (baseClassSet contains base) @@ -1990,7 +2017,7 @@ object SymDenotations { /** Register companion class */ override def registerCompanion(companion: Symbol)(implicit ctx: Context) = - if (companion.isClass && !unforcedIsAbsent && !companion.unforcedIsAbsent) + if (companion.isClass && !isAbsent(canForce = false) && !companion.isAbsent(canForce = false)) myCompanion = companion override def registeredCompanion(implicit ctx: Context) = { ensureCompleted(); myCompanion } @@ -2072,7 +2099,7 @@ object SymDenotations { } case nil => val directMembers = super.computeNPMembersNamed(name) - if (acc.exists) acc.union(directMembers.filterWithPredicate(!_.symbol.isAbsent)) + if (acc.exists) acc.union(directMembers.filterWithPredicate(!_.symbol.isAbsent())) else directMembers } if (symbol `eq` defn.ScalaPackageClass) { diff --git a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala index ddc060241288..71ef739a4ae7 100644 --- a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala +++ b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala @@ -932,7 +932,7 @@ class ClassfileParser( staticScope.lookup(name) else { var module = sym.companionModule - if (!module.exists && sym.isAbsent) + if (!module.exists && sym.isAbsent()) module = sym.scalacLinkedClass module.info.member(name).symbol } diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index 7a35ce87b1ad..d88afe0a60d4 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -251,7 +251,7 @@ object Completion { */ private def include(sym: Symbol, nameInScope: Name)(implicit ctx: Context): Boolean = nameInScope.startsWith(prefix) && - !sym.isAbsent && + !sym.isAbsent() && !sym.isPrimaryConstructor && sym.sourceSymbol.exists && (!sym.is(Package) || sym.is(ModuleClass)) && diff --git a/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala b/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala index e0a86f9d196a..83655185b515 100644 --- a/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala +++ b/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala @@ -326,8 +326,8 @@ private class ExtractDependenciesCollector extends tpd.TreeTraverser { thisTreeT private def ignoreDependency(sym: Symbol)(implicit ctx: Context) = !sym.exists || - sym.unforcedIsAbsent || // ignore dependencies that have a symbol but do not exist. - // e.g. java.lang.Object companion object + sym.isAbsent(canForce = false) || // ignore dependencies that have a symbol but do not exist. + // e.g. java.lang.Object companion object sym.isEffectiveRoot || sym.isAnonymousFunction || sym.isAnonymousClass diff --git a/compiler/src/dotty/tools/dotc/transform/Pickler.scala b/compiler/src/dotty/tools/dotc/transform/Pickler.scala index 71049da316e2..a3c2a3dbb064 100644 --- a/compiler/src/dotty/tools/dotc/transform/Pickler.scala +++ b/compiler/src/dotty/tools/dotc/transform/Pickler.scala @@ -41,7 +41,7 @@ class Pickler extends Phase { /** Drop any elements of this list that are linked module classes of other elements in the list */ private def dropCompanionModuleClasses(clss: List[ClassSymbol])(implicit ctx: Context): List[ClassSymbol] = { val companionModuleClasses = - clss.filterNot(_.is(Module)).map(_.linkedClass).filterNot(_.unforcedIsAbsent) + clss.filterNot(_.is(Module)).map(_.linkedClass).filterNot(_.isAbsent()) clss.filterNot(companionModuleClasses.contains) } diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index 9a221390da34..7dc697c44d6f 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -68,7 +68,7 @@ class TreeChecker extends Phase with SymTransformer { def transformSym(symd: SymDenotation)(implicit ctx: Context): SymDenotation = { val sym = symd.symbol - if (sym.isClass && !sym.isAbsent) { + if (sym.isClass && !sym.isAbsent()) { val validSuperclass = sym.isPrimitiveValueClass || defn.syntheticCoreClasses.contains(sym) || (sym eq defn.ObjectClass) || sym.isOneOf(NoSuperClassFlags) || (sym.asClass.superClass.exists) || sym.isRefinementClass diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 3a18d41e7fba..fefa6a36bc64 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -860,7 +860,7 @@ trait Checking { typr.println(i"check no double declarations $cls") def checkDecl(decl: Symbol): Unit = { - for (other <- seen(decl.name) if (!decl.isAbsent && !other.isAbsent)) { + for (other <- seen(decl.name) if (!decl.isAbsent() && !other.isAbsent())) { typr.println(i"conflict? $decl $other") def javaFieldMethodPair = decl.is(JavaDefined) && other.is(JavaDefined) && diff --git a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala index d850c410a2dd..b36a6a7097aa 100644 --- a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -170,7 +170,7 @@ trait TypeAssigner { final def reallyExists(denot: Denotation)(implicit ctx: Context): Boolean = try denot match { case denot: SymDenotation => - denot.exists && !denot.isAbsent + denot.exists && !denot.isAbsent() case denot: SingleDenotation => val sym = denot.symbol (sym eq NoSymbol) || reallyExists(sym.denot) From ccde31877c3b1dfd396d021f9fe94c3f8a32b9fd Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 2 Jul 2019 14:28:11 +0200 Subject: [PATCH 10/21] accessBoundary: Avoid forcing completion when possible Calling `this.flags` requires completing the denotation, but directly caling `is(...)` doesn't if the flags are FromStartFlags/AfterLoadFlags. Most calls to `accessBoundary` will still require forcing because of the call to `privateWithin`, this is fixed by the next few commits by making `privateWithin` require less forcing. --- .../src/dotty/tools/dotc/core/SymDenotations.scala | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 255f7b634402..378de07c969a 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1269,14 +1269,12 @@ object SymDenotations { * as public. * @param base The access boundary to assume if this symbol is protected */ - final def accessBoundary(base: Symbol)(implicit ctx: Context): Symbol = { - val fs = flags - if (fs.is(Private)) owner - else if (fs.isAllOf(StaticProtected)) defn.RootClass + final def accessBoundary(base: Symbol)(implicit ctx: Context): Symbol = + if (this.is(Private)) owner + else if (this.isAllOf(StaticProtected)) defn.RootClass else if (privateWithin.exists && !ctx.phase.erasedTypes) privateWithin - else if (fs.is(Protected)) base + else if (this.is(Protected)) base else defn.RootClass - } final def isPublic(implicit ctx: Context): Boolean = accessBoundary(owner) == defn.RootClass From f648faa27ca0c5a93452386a1a2730f12ea80518 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 4 Jul 2019 16:49:07 +0200 Subject: [PATCH 11/21] Namer: set privateWithin at symbol creation time First step towards making `privateWithin` force less. --- .../src/dotty/tools/dotc/typer/Namer.scala | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index a4dc90650a24..cd1a975b9795 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -311,32 +311,31 @@ class Namer { typer: Typer => else name } - /** If effective owner is a package `p`, widen `private` to `private[p]` */ - def widenToplevelPrivate(sym: Symbol): Unit = { - var owner = sym.effectiveOwner - if (owner.is(Package)) { - sym.resetFlag(Private) - sym.privateWithin = owner - } - } - /** Create new symbol or redefine existing symbol under lateCompile. */ def createOrRefine[S <: Symbol]( - tree: MemberDef, name: Name, flags: FlagSet, infoFn: S => Type, + tree: MemberDef, name: Name, flags: FlagSet, owner: Symbol, infoFn: S => Type, symFn: (FlagSet, S => Type, Symbol) => S): Symbol = { val prev = if (lateCompile && ctx.owner.is(Package)) ctx.effectiveScope.lookup(name) else NoSymbol + + var flags1 = flags + var privateWithin = privateWithinClass(tree.mods) + val effectiveOwner = owner.skipWeakOwner + if (flags.is(Private) && effectiveOwner.is(Package)) { + // If effective owner is a package p, widen private to private[p] + flags1 = flags1 &~ Private + privateWithin = effectiveOwner + } + val sym = if (prev.exists) { - prev.flags = flags + prev.flags = flags1 prev.info = infoFn(prev.asInstanceOf[S]) - prev.privateWithin = privateWithinClass(tree.mods) + prev.privateWithin = privateWithin prev } - else symFn(flags, infoFn, privateWithinClass(tree.mods)) - if (sym.is(Private)) - widenToplevelPrivate(sym) + else symFn(flags1, infoFn, privateWithin) recordSym(sym, tree) } @@ -345,7 +344,7 @@ class Namer { typer: Typer => val name = checkNoConflict(tree.name).asTypeName val flags = checkFlags(tree.mods.flags &~ DelegateOrImplicit) val cls = - createOrRefine[ClassSymbol](tree, name, flags, + createOrRefine[ClassSymbol](tree, name, flags, ctx.owner, cls => adjustIfModule(new ClassCompleter(cls, tree)(ctx), tree), ctx.newClassSymbol(ctx.owner, name, _, _, _, tree.nameSpan, ctx.source.file)) cls.completer.asInstanceOf[ClassCompleter].init() @@ -381,7 +380,7 @@ class Namer { typer: Typer => } val info = adjustIfModule(completer, tree) createOrRefine[Symbol](tree, name, flags | deferred | method | higherKinded, - _ => info, + ctx.owner, _ => info, (fs, _, pwithin) => ctx.newSymbol(ctx.owner, name, fs, info, pwithin, tree.nameSpan)) case tree: Import => recordSym(ctx.newImportSymbol(ctx.owner, new Completer(tree), tree.span), tree) From 9fc9f2175570909f5c3be0c2ce12768e41889a44 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 4 Jul 2019 17:00:08 +0200 Subject: [PATCH 12/21] Scala2Unpickler: set privateWithin at symbol creation time Second step towards making `privateWithin` force less. --- .../core/unpickleScala2/Scala2Unpickler.scala | 40 +++++++++++-------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala index 7e671d19c810..873c6b56ed34 100644 --- a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala @@ -468,7 +468,7 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas //if (isModuleRoot) println(s"moduleRoot of $moduleRoot found at $readIndex, flags = ${flags.flagsString}") // !!! DEBUG //if (isModuleClassRoot) println(s"moduleClassRoot of $moduleClassRoot found at $readIndex, flags = ${flags.flagsString}") // !!! DEBUG - def completeRoot(denot: ClassDenotation, completer: LazyType): Symbol = { + def completeRoot(denot: ClassDenotation, completer: LazyType, privateWithin: Symbol): Symbol = { denot.setFlag(flags) denot.resetFlag(Touched) // allow one more completion @@ -479,6 +479,7 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas if (owner == defn.ScalaPackageClass && ((name eq tpnme.Serializable) || (name eq tpnme.Product))) denot.setFlag(NoInits) + denot.privateWithin = privateWithin denot.info = completer denot.symbol } @@ -497,24 +498,32 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas sym } + val (privateWithin, infoRef) = { + val ref = readNat() + if (!isSymbolRef(ref)) + (NoSymbol, ref) + else { + val pw = at(ref, () => readSymbol()) + (pw, readNat()) + } + } + finishSym(tag match { case TYPEsym | ALIASsym => var name1 = name.asTypeName var flags1 = flags if (flags.is(TypeParam)) flags1 |= owner.typeParamCreationFlags - ctx.newSymbol(owner, name1, flags1, localMemberUnpickler, coord = start) + ctx.newSymbol(owner, name1, flags1, localMemberUnpickler, privateWithin, coord = start) case CLASSsym => - var infoRef = readNat() - if (isSymbolRef(infoRef)) infoRef = readNat() if (isClassRoot) completeRoot( - classRoot, rootClassUnpickler(start, classRoot.symbol, NoSymbol, infoRef)) + classRoot, rootClassUnpickler(start, classRoot.symbol, NoSymbol, infoRef), privateWithin) else if (isModuleClassRoot) completeRoot( - moduleClassRoot, rootClassUnpickler(start, moduleClassRoot.symbol, moduleClassRoot.sourceModule, infoRef)) + moduleClassRoot, rootClassUnpickler(start, moduleClassRoot.symbol, moduleClassRoot.sourceModule, infoRef), privateWithin) else if (name == tpnme.REFINE_CLASS) // create a type alias instead - ctx.newSymbol(owner, name, flags, localMemberUnpickler, coord = start) + ctx.newSymbol(owner, name, flags, localMemberUnpickler, privateWithin, coord = start) else { def completer(cls: Symbol) = { val unpickler = new ClassUnpickler(infoRef) withDecls symScope(cls) @@ -524,10 +533,10 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas .suchThat(_.is(Module)).symbol) else unpickler } - ctx.newClassSymbol(owner, name.asTypeName, flags, completer, coord = start) + ctx.newClassSymbol(owner, name.asTypeName, flags, completer, privateWithin, coord = start) } case VALsym => - ctx.newSymbol(owner, name.asTermName, flags, localMemberUnpickler, coord = start) + ctx.newSymbol(owner, name.asTermName, flags, localMemberUnpickler, privateWithin, coord = start) case MODULEsym => if (isModuleRoot) { moduleRoot setFlag flags @@ -536,7 +545,7 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas new LocalUnpickler() withModuleClass(implicit ctx => owner.info.decls.lookup(name.moduleClassName) .suchThat(_.is(Module)).symbol) - , coord = start) + , privateWithin, coord = start) case _ => errorBadSignature("bad symbol tag: " + tag) }) @@ -552,16 +561,13 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas val unusedNameref = readNat() val unusedOwnerref = readNat() val unusedFlags = readLongNat() + var inforef = readNat() - denot.privateWithin = - if (!isSymbolRef(inforef)) NoSymbol - else { - val pw = at(inforef, () => readSymbol()) - inforef = readNat() - pw - } + if (isSymbolRef(inforef)) inforef = readNat() + // println("reading type for " + denot) // !!! DEBUG val tp = at(inforef, () => readType()(ctx)) + denot match { case denot: ClassDenotation => val selfInfo = if (atEnd) NoType else readTypeRef() From 1e8860a05586b2f414969a5b19da4bffefccf5d4 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 4 Jul 2019 17:07:45 +0200 Subject: [PATCH 13/21] ClassfileParser: set privateWithin at symbol creation time for members Third step towards making `privateWithin` force less. --- .../dotc/core/classfile/ClassfileParser.scala | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala index 71ef739a4ae7..b7cc33472b65 100644 --- a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala +++ b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala @@ -156,9 +156,12 @@ class ClassfileParser( classRoot.setFlag(sflags) moduleRoot.setFlag(Flags.JavaDefined | Flags.ModuleClassCreationFlags) - setPrivateWithin(classRoot, jflags) - setPrivateWithin(moduleRoot, jflags) - setPrivateWithin(moduleRoot.sourceModule, jflags) + + val privateWithin = getPrivateWithin(jflags) + + classRoot.privateWithin = privateWithin + moduleRoot.privateWithin = privateWithin + moduleRoot.sourceModule.privateWithin = privateWithin for (i <- 0 until in.nextChar) parseMember(method = false) for (i <- 0 until in.nextChar) parseMember(method = true) @@ -212,7 +215,8 @@ class ClassfileParser( val name = pool.getName(in.nextChar) if (!sflags.is(Flags.Private) || name == nme.CONSTRUCTOR) { val member = ctx.newSymbol( - getOwner(jflags), name, sflags, memberCompleter, coord = start) + getOwner(jflags), name, sflags, memberCompleter, + getPrivateWithin(jflags), coord = start) getScope(jflags).enter(member) } // skip rest of member for now @@ -263,7 +267,6 @@ class ClassfileParser( denot.info = pool.getType(in.nextChar) if (isEnum) denot.info = ConstantType(Constant(sym)) if (isConstructor) normalizeConstructorParams() - setPrivateWithin(denot, jflags) denot.info = translateTempPoly(parseAttributes(sym, denot.info)) if (isConstructor) normalizeConstructorInfo() @@ -983,10 +986,11 @@ class ClassfileParser( protected def getScope(flags: Int): MutableScope = if (isStatic(flags)) staticScope else instanceScope - private def setPrivateWithin(denot: SymDenotation, jflags: Int)(implicit ctx: Context): Unit = { + private def getPrivateWithin(jflags: Int)(implicit ctx: Context): Symbol = if ((jflags & (JAVA_ACC_PRIVATE | JAVA_ACC_PUBLIC)) == 0) - denot.privateWithin = denot.enclosingPackageClass - } + classRoot.enclosingPackageClass + else + NoSymbol private def isPrivate(flags: Int) = (flags & JAVA_ACC_PRIVATE) != 0 private def isStatic(flags: Int) = (flags & JAVA_ACC_STATIC) != 0 From 7fc5deb477fcb3a7814eb14da722ca923b5d1738 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 4 Jul 2019 18:37:29 +0200 Subject: [PATCH 14/21] Fix #6603: Make privateWithin force less At this point, ModuleCompleters and SymbolLoaders are the only two kinds of completers which need to set privateWithin, this is now enforced by the setter (which is therefore renamed to `setPrivateWithin` to make it clearer that it doesn't just set a field) and taken advantage of by the getter to avoid forcing unless necessary. This is the last piece needed to finally fix #6603. Unfortunately, the cumulative changes to completions done so far have caused cyclic reference errors to pop up in new situations, including bootstrapping Dotty itself. This is fixed by the remaining commits in this PR. --- .../tools/dotc/core/SymDenotations.scala | 32 +++++++++++++++---- .../dotc/core/classfile/ClassfileParser.scala | 6 ++-- .../tools/dotc/core/tasty/TreeUnpickler.scala | 2 +- .../core/unpickleScala2/Scala2Unpickler.scala | 2 +- .../src/dotty/tools/dotc/typer/Namer.scala | 2 +- .../BootstrappedOnlyCompilationTests.scala | 9 ++++++ tests/neg/exports.check | 12 +++---- tests/neg/exports.scala | 4 +-- tests/neg/i4368.scala | 10 +++--- tests/neg/i4370.scala | 4 +-- 10 files changed, 56 insertions(+), 27 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 378de07c969a..55e48616894f 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -277,11 +277,31 @@ object SymDenotations { /** The privateWithin boundary, NoSymbol if no boundary is given. */ - final def privateWithin(implicit ctx: Context): Symbol = { ensureCompleted(); myPrivateWithin } + @tailrec + final def privateWithin(implicit ctx: Context): Symbol = myInfo match { + case myInfo: ModuleCompleter => + // Instead of completing the ModuleCompleter, we can get `privateWithin` + // directly from the module class, which might require less completions. + myInfo.moduleClass.privateWithin + case _: SymbolLoader => + // Completing a SymbolLoader might call `setPrivateWithin()` + completeOnce() + privateWithin + case _ => + // Otherwise, no completion is necessary, see the preconditions of `markAbsent()`. + myPrivateWithin + } - /** Set privateWithin. */ - protected[dotc] final def privateWithin_=(sym: Symbol): Unit = - myPrivateWithin = sym + /** Set privateWithin, prefer setting it at symbol-creation time instead if + * possible. + * @pre `isCompleting` is false, or this is a ModuleCompleter or SymbolLoader + */ + protected[dotc] final def setPrivateWithin(pw: Symbol)(implicit ctx: Context): Unit = { + if (isCompleting) + assert(myInfo.isInstanceOf[ModuleCompleter | SymbolLoader], + s"Illegal call to `setPrivateWithin($pw)` while completing $this using completer $myInfo") + myPrivateWithin = pw + } /** The annotations of this denotation */ final def annotations(implicit ctx: Context): List[Annotation] = { @@ -2227,7 +2247,7 @@ object SymDenotations { // is to have the module class completer set the annotations of both the // class and the module. denot.info = moduleClass.typeRef - denot.privateWithin = from.privateWithin + denot.setPrivateWithin(from.privateWithin) } } @@ -2241,7 +2261,7 @@ object SymDenotations { case _ => ErrorType(errMsg) } - denot.privateWithin = NoSymbol + denot.setPrivateWithin(NoSymbol) } def complete(denot: SymDenotation)(implicit ctx: Context): Unit = { diff --git a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala index b7cc33472b65..bcd4eb058f91 100644 --- a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala +++ b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala @@ -159,9 +159,9 @@ class ClassfileParser( val privateWithin = getPrivateWithin(jflags) - classRoot.privateWithin = privateWithin - moduleRoot.privateWithin = privateWithin - moduleRoot.sourceModule.privateWithin = privateWithin + classRoot.setPrivateWithin(privateWithin) + moduleRoot.setPrivateWithin(privateWithin) + moduleRoot.sourceModule.setPrivateWithin(privateWithin) for (i <- 0 until in.nextChar) parseMember(method = false) for (i <- 0 until in.nextChar) parseMember(method = true) diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index 9fae3f855669..51cff426e959 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -546,7 +546,7 @@ class TreeUnpickler(reader: TastyReader, rootd.info = adjustIfModule( new Completer(subReader(start, end)) with SymbolLoaders.SecondCompleter) rootd.flags = flags &~ Touched // allow one more completion - rootd.privateWithin = privateWithin + rootd.setPrivateWithin(privateWithin) seenRoots += rootd.symbol rootd.symbol case _ => diff --git a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala index 873c6b56ed34..aae156583882 100644 --- a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala @@ -479,7 +479,7 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas if (owner == defn.ScalaPackageClass && ((name eq tpnme.Serializable) || (name eq tpnme.Product))) denot.setFlag(NoInits) - denot.privateWithin = privateWithin + denot.setPrivateWithin(privateWithin) denot.info = completer denot.symbol } diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index cd1a975b9795..1522fc9b0ffb 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -332,7 +332,7 @@ class Namer { typer: Typer => if (prev.exists) { prev.flags = flags1 prev.info = infoFn(prev.asInstanceOf[S]) - prev.privateWithin = privateWithin + prev.setPrivateWithin(privateWithin) prev } else symFn(flags1, infoFn, privateWithin) diff --git a/compiler/test/dotty/tools/dotc/BootstrappedOnlyCompilationTests.scala b/compiler/test/dotty/tools/dotc/BootstrappedOnlyCompilationTests.scala index 1b21348fcb44..ecf3d238f412 100644 --- a/compiler/test/dotty/tools/dotc/BootstrappedOnlyCompilationTests.scala +++ b/compiler/test/dotty/tools/dotc/BootstrappedOnlyCompilationTests.scala @@ -57,6 +57,15 @@ class BootstrappedOnlyCompilationTests extends ParallelTesting { ), withCompilerOptions ), + compileList( + "testIssue6603", + List( + "compiler/src/dotty/tools/dotc/ast/Desugar.scala", + "compiler/src/dotty/tools/dotc/ast/Trees.scala", + "compiler/src/dotty/tools/dotc/core/Types.scala" + ), + withCompilerOptions + ), ).checkCompile() } diff --git a/tests/neg/exports.check b/tests/neg/exports.check index 9c4ac30bfec8..27ea9b0ea6a0 100644 --- a/tests/neg/exports.check +++ b/tests/neg/exports.check @@ -41,9 +41,9 @@ | ^^^ | no eligible member foo at this.foo | this.foo.foo cannot be exported because it is already a member of class Foo --- [E046] Cyclic Error: tests/neg/exports.scala:45:11 ------------------------------------------------------------------ -45 | val bar: Bar = new Bar // error: cyclic reference - | ^ - | Cyclic reference involving value bar - -longer explanation available when compiling with `-explain` +-- [E120] Duplicate Symbol Error: tests/neg/exports.scala:46:13 -------------------------------------------------------- +46 | export bar._ // error: double definition + | ^ + | Double definition: + | val bar: Bar in class Baz at line 45 and + | final def bar: => => Bar(Baz.this.bar.baz.bar)(Baz.this.bar.bar) in class Baz at line 46 diff --git a/tests/neg/exports.scala b/tests/neg/exports.scala index 70cfc3e62efc..c1e144e0e51e 100644 --- a/tests/neg/exports.scala +++ b/tests/neg/exports.scala @@ -42,8 +42,8 @@ class Foo { } class Baz { - val bar: Bar = new Bar // error: cyclic reference - export bar._ + val bar: Bar = new Bar + export bar._ // error: double definition } class Bar { val baz: Baz = new Baz diff --git a/tests/neg/i4368.scala b/tests/neg/i4368.scala index c5c0f5223d95..95c342d5fe2a 100644 --- a/tests/neg/i4368.scala +++ b/tests/neg/i4368.scala @@ -149,13 +149,13 @@ object Test9 { object i4369 { trait X { self => type R <: Z - type Z >: X { type R = self.R; type Z = self.R } + type Z >: X { type R = self.R; type Z = self.R } // error: cyclic // error: cyclic // error: cyclic } - class Foo extends X { type R = Foo; type Z = Foo } // error: too deep + class Foo extends X { type R = Foo; type Z = Foo } } object i4370 { - class Foo { type R = A } // error: cyclic - type A = List[Foo#R] + class Foo { type R = A } + type A = List[Foo#R] // error: cyclic } object i4371 { class Foo { type A = Boo#B } // error: cyclic @@ -170,4 +170,4 @@ object i318 { val y: Y = ??? val a: y.A = ??? // error: too deep val b: y.B = a // error: too deep -} \ No newline at end of file +} diff --git a/tests/neg/i4370.scala b/tests/neg/i4370.scala index 4c4924d95d7d..033373ae38e2 100644 --- a/tests/neg/i4370.scala +++ b/tests/neg/i4370.scala @@ -1,4 +1,4 @@ object App { - class Foo { type R = A } // error - type A = List[Foo#R] + class Foo { type R = A } + type A = List[Foo#R] // error } From f28ed392cc7d1897b0ca21ca5e635131dbe72b1d Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 11 Jul 2019 14:34:01 +0200 Subject: [PATCH 15/21] Remove unused suffix param of DerivedFromParamTree This reverts the remains of 393c620d0704f86e05507bedc5579a113b013ccd which are no longer needed now that Eql instances are generated using typeclass derivation. --- .../src/dotty/tools/dotc/ast/Desugar.scala | 24 +++++++------------ 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 35e62d523894..effb2167d2f4 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -74,11 +74,8 @@ object desugar { def derivedTree(sym: Symbol)(implicit ctx: Context): tpd.Tree = tpd.ref(sym) } - /** A type tree that computes its type from an existing parameter. - * @param suffix String difference between existing parameter (call it `P`) and parameter owning the - * DerivedTypeTree (call it `O`). We have: `O.name == P.name + suffix`. - */ - class DerivedFromParamTree(suffix: String)(implicit @constructorOnly src: SourceFile) extends DerivedTypeTree { + /** A type tree that computes its type from an existing parameter. */ + class DerivedFromParamTree(implicit @constructorOnly src: SourceFile) extends DerivedTypeTree { /** Make sure that for all enclosing module classes their companion classes * are completed. Reason: We need the constructor of such companion classes to @@ -96,16 +93,12 @@ object desugar { /** Return info of original symbol, where all references to siblings of the * original symbol (i.e. sibling and original symbol have the same owner) - * are rewired to like-named* parameters or accessors in the scope enclosing + * are rewired to same-named parameters or accessors in the scope enclosing * the current scope. The current scope is the scope owned by the defined symbol * itself, that's why we have to look one scope further out. If the resulting * type is an alias type, dealias it. This is necessary because the * accessor of a type parameter is a private type alias that cannot be accessed * from subclasses. - * - * (*) like-named means: - * - * parameter name == reference name ++ suffix */ def derivedTree(sym: Symbol)(implicit ctx: Context): tpd.TypeTree = { val relocate = new TypeMap { @@ -113,7 +106,7 @@ object desugar { def apply(tp: Type) = tp match { case tp: NamedType if tp.symbol.exists && (tp.symbol.owner eq originalOwner) => val defctx = ctx.outersIterator.dropWhile(_.scope eq ctx.scope).next() - var local = defctx.denotNamed(tp.name ++ suffix).suchThat(_.isParamOrAccessor).symbol + var local = defctx.denotNamed(tp.name).suchThat(_.isParamOrAccessor).symbol if (local.exists) (defctx.owner.thisType select local).dealiasKeepAnnots else { def msg = @@ -129,20 +122,19 @@ object desugar { } /** A type definition copied from `tdef` with a rhs typetree derived from it */ - def derivedTypeParam(tdef: TypeDef, suffix: String = "")(implicit ctx: Context): TypeDef = + def derivedTypeParam(tdef: TypeDef)(implicit ctx: Context): TypeDef = cpy.TypeDef(tdef)( - name = tdef.name ++ suffix, - rhs = DerivedFromParamTree(suffix).withSpan(tdef.rhs.span).watching(tdef) + rhs = DerivedFromParamTree().withSpan(tdef.rhs.span).watching(tdef) ) /** A derived type definition watching `sym` */ def derivedTypeParam(sym: TypeSymbol)(implicit ctx: Context): TypeDef = - TypeDef(sym.name, DerivedFromParamTree("").watching(sym)).withFlags(TypeParam) + TypeDef(sym.name, DerivedFromParamTree().watching(sym)).withFlags(TypeParam) /** A value definition copied from `vdef` with a tpt typetree derived from it */ def derivedTermParam(vdef: ValDef)(implicit ctx: Context): ValDef = cpy.ValDef(vdef)( - tpt = DerivedFromParamTree("").withSpan(vdef.tpt.span).watching(vdef)) + tpt = DerivedFromParamTree().withSpan(vdef.tpt.span).watching(vdef)) // ----- Desugar methods ------------------------------------------------- From d8391c69d69eb396ba56dda64e60835dffdb5d9f Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 11 Jul 2019 16:48:41 +0200 Subject: [PATCH 16/21] TempClassInfo#finalize: remove selfInfo parameter It turns out that in practice we always use the same selfInfo that was used to create the TempClassInfo, so no need to pass it again as a parameter. --- compiler/src/dotty/tools/dotc/core/Types.scala | 2 +- .../dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala | 2 +- compiler/src/dotty/tools/dotc/typer/Namer.scala | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index ddad9f743c6f..0d3901e9c252 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -4043,7 +4043,7 @@ object Types { extends CachedClassInfo(prefix, cls, Nil, decls, selfInfo) { /** Install classinfo with known parents in `denot` s */ - def finalize(denot: SymDenotation, parents: List[Type], selfInfo: TypeOrSymbol)(implicit ctx: Context): Unit = + def finalize(denot: SymDenotation, parents: List[Type])(implicit ctx: Context): Unit = denot.info = ClassInfo(prefix, cls, parents, decls, selfInfo) override def derivedClassInfo(prefix: Type)(implicit ctx: Context): ClassInfo = diff --git a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala index aae156583882..b6718327a3df 100644 --- a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala @@ -134,7 +134,7 @@ object Scala2Unpickler { else registerCompanionPair(scalacCompanion, denot.classSymbol) - tempInfo.finalize(denot, normalizedParents, ost) // install final info, except possibly for typeparams ordering + tempInfo.finalize(denot, normalizedParents) // install final info, except possibly for typeparams ordering denot.ensureTypeParamsInCorrectOrder() } } diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 1522fc9b0ffb..d6f1db827aed 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -1144,7 +1144,7 @@ class Namer { typer: Typer => original.putAttachment(Deriver, deriver) } - tempInfo.finalize(denot, parentTypes, selfInfo) + tempInfo.finalize(denot, parentTypes) Checking.checkWellFormed(cls) if (isDerivedValueClass(cls)) cls.setFlag(Final) From d9ade19d882f2a05a5fa38d78a239cea6464ffd8 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 11 Jul 2019 15:37:19 +0200 Subject: [PATCH 17/21] When typing a DerivedFromParamTree, only complete constructors Parameter accessors have a derived rhs based on the corresponding class constructor parameter. Before this commit, typing such a derived tree would involve completing the enclosing class, which requires typing the class parents, which might end up requiring the info of the original parameter accessor. This doesn't always cause cyclic reference since `Namer#typeDefSig` sets temporary empty bounds when completing a type definition, but it can lead to puzzling compilation errors. In particular, after the changes related to completions in this PR, it broke the bootstrap. We fix this by making `DerivedFromParamTree#ensureCompletions` force less: we only really need to force the constructors, this requires some refactoring in `ClassCompleter` to allow such partial completions. --- .../src/dotty/tools/dotc/ast/Desugar.scala | 24 ++++--- .../src/dotty/tools/dotc/typer/Namer.scala | 69 ++++++++++++------- 2 files changed, 59 insertions(+), 34 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index effb2167d2f4..e9e7e8a15e1c 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -7,7 +7,7 @@ import util.Spans._, Types._, Contexts._, Constants._, Names._, NameOps._, Flags import Symbols._, StdNames._, Trees._ import Decorators._, transform.SymUtils._ import NameKinds.{UniqueName, EvidenceParamName, DefaultGetterName} -import typer.FrontEnd +import typer.{FrontEnd, Namer} import util.{Property, SourceFile, SourcePosition} import util.NameTransformer.avoidIllegalChars import collection.mutable.ListBuffer @@ -75,21 +75,27 @@ object desugar { } /** A type tree that computes its type from an existing parameter. */ - class DerivedFromParamTree(implicit @constructorOnly src: SourceFile) extends DerivedTypeTree { + class DerivedFromParamTree()(implicit @constructorOnly src: SourceFile) extends DerivedTypeTree { - /** Make sure that for all enclosing module classes their companion classes - * are completed. Reason: We need the constructor of such companion classes to - * be completed so that OriginalSymbol attachments are pushed to DerivedTypeTrees - * in apply/unapply methods. + /** Complete the appropriate constructors so that OriginalSymbol attachments are + * pushed to DerivedTypeTrees. */ - override def ensureCompletions(implicit ctx: Context): Unit = + override def ensureCompletions(implicit ctx: Context): Unit = { + def completeConstructor(sym: Symbol) = + sym.infoOrCompleter match { + case completer: Namer#ClassCompleter => + completer.completeConstructor(sym) + case _ => + } + if (!ctx.owner.is(Package)) if (ctx.owner.isClass) { - ctx.owner.ensureCompleted() + completeConstructor(ctx.owner) if (ctx.owner.is(ModuleClass)) - ctx.owner.linkedClass.ensureCompleted() + completeConstructor(ctx.owner.linkedClass) } else ensureCompletions(ctx.outer) + } /** Return info of original symbol, where all references to siblings of the * original symbol (i.e. sibling and original symbol have the same owner) diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index d6f1db827aed..a0975089abaf 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -913,6 +913,10 @@ class Namer { typer: Typer => protected implicit val ctx: Context = localContext(cls).setMode(ictx.mode &~ Mode.InSuperCall) + private[this] var localCtx: Context = _ + /** info to be used temporarily while completing the class, to avoid cyclic references. */ + private[this] var tempInfo: TempClassInfo = _ + val TypeDef(name, impl @ Template(constr, _, self, _)) = original private val (params, rest): (List[Tree], List[Tree]) = impl.body.span { @@ -1040,6 +1044,42 @@ class Namer { typer: Typer => forwarderss.foreach(_.foreach(fwdr => fwdr.symbol.entered)) } + /** Ensure constructor is completed so that any parameter accessors + * which have type trees deriving from its parameters can be + * completed in turn. Note that parent types access such parameter + * accessors, that's why the constructor needs to be completed before + * the parent types are elaborated. + */ + def completeConstructor(denot: SymDenotation): Unit = { + if (tempInfo != null) // Constructor has been completed already + return + + addAnnotations(denot.symbol) + + val selfInfo: TypeOrSymbol = + if (self.isEmpty) NoType + else if (cls.is(Module)) { + val moduleType = cls.owner.thisType select sourceModule + if (self.name == nme.WILDCARD) moduleType + else recordSym( + ctx.newSymbol(cls, self.name, self.mods.flags, moduleType, coord = self.span), + self) + } + else createSymbol(self) + + val savedInfo = denot.infoOrCompleter + tempInfo = new TempClassInfo(cls.owner.thisType, cls, decls, selfInfo) + denot.info = tempInfo + + localCtx = ctx.inClassContext(selfInfo) + + index(constr) + index(rest)(localCtx) + symbolOfTree(constr).ensureCompleted() + + denot.info = savedInfo + } + /** The type signature of a ClassDef with given symbol */ override def completeInCreationContext(denot: SymDenotation): Unit = { val parents = impl.parents @@ -1102,34 +1142,11 @@ class Namer { typer: Typer => } } - addAnnotations(denot.symbol) + if (tempInfo == null) // Constructor has not been completed yet + completeConstructor(denot) - val selfInfo: TypeOrSymbol = - if (self.isEmpty) NoType - else if (cls.is(Module)) { - val moduleType = cls.owner.thisType select sourceModule - if (self.name == nme.WILDCARD) moduleType - else recordSym( - ctx.newSymbol(cls, self.name, self.mods.flags, moduleType, coord = self.span), - self) - } - else createSymbol(self) - - // pre-set info, so that parent types can refer to type params - val tempInfo = new TempClassInfo(cls.owner.thisType, cls, decls, selfInfo) denot.info = tempInfo - val localCtx = ctx.inClassContext(selfInfo) - - // Ensure constructor is completed so that any parameter accessors - // which have type trees deriving from its parameters can be - // completed in turn. Note that parent types access such parameter - // accessors, that's why the constructor needs to be completed before - // the parent types are elaborated. - index(constr) - index(rest)(localCtx) - symbolOfTree(constr).ensureCompleted() - val parentTypes = defn.adjustForTuple(cls, cls.typeParams, ensureFirstIsClass(parents.map(checkedParentType(_)), cls.span)) typr.println(i"completing $denot, parents = $parents%, %, parentTypes = $parentTypes%, %") @@ -1145,6 +1162,8 @@ class Namer { typer: Typer => } tempInfo.finalize(denot, parentTypes) + // The temporary info can now be garbage-collected + tempInfo = null Checking.checkWellFormed(cls) if (isDerivedValueClass(cls)) cls.setFlag(Final) From b264bf0217e6288edd26186d2a5498c1f5d21182 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Sun, 7 Jul 2019 21:37:31 +0200 Subject: [PATCH 18/21] Add explicit check against self-annotated classes Following the completions-related changes in this PR, we no longer encounter cyclic reference errors when annotating a class with itself (i1212.scala) as well as in some situation where an annotation class indirectly refers to itself (i3506.scala). While the latter is OK, the former is problematic because when unpickling from tasty, the modifiers and annotations of a symbol are read before the symbol itself is created, so we can't support self-annotation classes, we therefore need to disallow them explicitly. --- compiler/src/dotty/tools/dotc/typer/Namer.scala | 12 ++++++++---- tests/neg/i1212.scala | 2 +- tests/neg/i3506.scala | 2 -- tests/pos/i3506.scala | 2 ++ 4 files changed, 11 insertions(+), 7 deletions(-) delete mode 100644 tests/neg/i3506.scala create mode 100644 tests/pos/i3506.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index a0975089abaf..ea24c7925fac 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -792,10 +792,14 @@ class Namer { typer: Typer => lazy val annotCtx = annotContext(original, sym) for (annotTree <- untpd.modsDeco(original).mods.annotations) { val cls = typedAheadAnnotationClass(annotTree)(annotCtx) - val ann = Annotation.deferred(cls, implicit ctx => typedAnnotation(annotTree)) - sym.addAnnotation(ann) - if (cls == defn.ForceInlineAnnot && sym.is(Method, butNot = Accessor)) - sym.setFlag(Inline) + if (cls eq sym) + ctx.error("An annotation class cannot be annotated with iself", annotTree.sourcePos) + else { + val ann = Annotation.deferred(cls, implicit ctx => typedAnnotation(annotTree)) + sym.addAnnotation(ann) + if (cls == defn.ForceInlineAnnot && sym.is(Method, butNot = Accessor)) + sym.setFlag(Inline) + } } case _ => } diff --git a/tests/neg/i1212.scala b/tests/neg/i1212.scala index b009a4d2c5a3..e2c98219c390 100644 --- a/tests/neg/i1212.scala +++ b/tests/neg/i1212.scala @@ -1 +1 @@ -@ann class ann extends scala.annotation.Annotation // error: cyclic reference +@ann class ann extends scala.annotation.Annotation // error: An annotation class cannot be annotated with iself diff --git a/tests/neg/i3506.scala b/tests/neg/i3506.scala deleted file mode 100644 index 51f0da36a64c..000000000000 --- a/tests/neg/i3506.scala +++ /dev/null @@ -1,2 +0,0 @@ -trait C2[@specialized A] // error -class specialized extends C2[Char] // error diff --git a/tests/pos/i3506.scala b/tests/pos/i3506.scala new file mode 100644 index 000000000000..fcfd86c42541 --- /dev/null +++ b/tests/pos/i3506.scala @@ -0,0 +1,2 @@ +trait C2[@specialized A] +class specialized extends scala.annotation.StaticAnnotation with C2[Char] From 9fcceb4e9f0ea2ea40b6497cce5f3b3ec4a0d645 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Sun, 7 Jul 2019 00:35:51 +0200 Subject: [PATCH 19/21] Make checkNonCyclic force less `checkInfo` is just the identity on ClassInfo so no need to force the info in this case. This avoids some cyclic reference errors which started appearing after other completions-related changes in this PR. --- compiler/src/dotty/tools/dotc/typer/Checking.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index fefa6a36bc64..dd24a155346b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -258,7 +258,7 @@ object Checking { if (locked.contains(tp) || tp.symbol.infoOrCompleter.isInstanceOf[NoCompleter]) throw CyclicReference(tp.symbol) locked += tp - try checkInfo(tp.info) + try if (!tp.symbol.isClass) checkInfo(tp.info) finally locked -= tp tp.withPrefix(pre1) } From 777fa57cdaf5d8b0b9a3b56555789263ab76d591 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Mon, 8 Jul 2019 20:14:38 +0200 Subject: [PATCH 20/21] Make checkNonCyclicInherited force less According to its documentation, it should only be necessary when there's more than one parent, we can use this knowledge to exit early from the method. This avoids a cyclic reference involving a refinement type in t1957.scala which started happening after the completions-related changes in this PR. --- compiler/src/dotty/tools/dotc/typer/Checking.scala | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index dd24a155346b..d68e771292de 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -358,10 +358,14 @@ object Checking { } /** Check type members inherited from different `parents` of `joint` type for cycles, - * unless a type with the same name aleadry appears in `decls`. + * unless a type with the same name already appears in `decls`. * @return true iff no cycles were detected */ def checkNonCyclicInherited(joint: Type, parents: List[Type], decls: Scope, posd: Positioned)(implicit ctx: Context): Unit = { + // If we don't have more than one parent, then there's nothing to check + if (parents.lengthCompare(1) <= 0) + return + def qualifies(sym: Symbol) = sym.name.isTypeName && !sym.is(Private) val abstractTypeNames = for (parent <- parents; mbr <- parent.abstractTypeMembers if qualifies(mbr.symbol)) From b602d83b8972955578f3e49c3be182a8d351c1e6 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Sat, 6 Jul 2019 20:58:37 +0200 Subject: [PATCH 21/21] Drop annotations from constructor parameters Scala 2 preserves annotations both on the param accessors and on compiler-generated methods such as copy, so we should do the same. But there doesn't seem to be much point in preserving them on the type parameters of the constructors. Removing them avoids a cyclic reference error involving `@specialized` which happened in the past and resurfaced with the completions-related changes in this PR. --- .../src/dotty/tools/dotc/ast/Desugar.scala | 61 +++++++++++++------ compiler/src/dotty/tools/dotc/ast/Trees.scala | 2 + 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index e9e7e8a15e1c..5db09105d965 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -267,8 +267,8 @@ object desugar { def defaultGetter: DefDef = DefDef( name = DefaultGetterName(methName, n), - tparams = meth.tparams.map(tparam => dropContextBounds(toDefParam(tparam))), - vparamss = takeUpTo(normalizedVparamss.nestedMap(toDefParam), n), + tparams = meth.tparams.map(tparam => dropContextBounds(toDefParam(tparam, keepAnnotations = true))), + vparamss = takeUpTo(normalizedVparamss.nestedMap(toDefParam(_, keepAnnotations = true)), n), tpt = TypeTree(), rhs = vparam.rhs ) @@ -372,10 +372,16 @@ object desugar { @sharable private val synthetic = Modifiers(Synthetic) - private def toDefParam(tparam: TypeDef): TypeDef = - tparam.withMods(tparam.rawMods & EmptyFlags | Param) - private def toDefParam(vparam: ValDef): ValDef = - vparam.withMods(vparam.rawMods & (GivenOrImplicit | Erased) | Param) + private def toDefParam(tparam: TypeDef, keepAnnotations: Boolean): TypeDef = { + var mods = tparam.rawMods + if (!keepAnnotations) mods = mods.withAnnotations(Nil) + tparam.withMods(mods & EmptyFlags | Param) + } + private def toDefParam(vparam: ValDef, keepAnnotations: Boolean): ValDef = { + var mods = vparam.rawMods + if (!keepAnnotations) mods = mods.withAnnotations(Nil) + vparam.withMods(mods & (GivenOrImplicit | Erased) | Param) + } /** The expansion of a class definition. See inline comments for what is involved */ def classDef(cdef: TypeDef)(implicit ctx: Context): Tree = { @@ -437,7 +443,7 @@ object desugar { else originalTparams } else originalTparams - val constrTparams = impliedTparams.map(toDefParam) + val constrTparams = impliedTparams.map(toDefParam(_, keepAnnotations = false)) val constrVparamss = if (originalVparamss.isEmpty) { // ensure parameter list is non-empty if (isCaseClass && originalTparams.isEmpty) @@ -447,7 +453,7 @@ object desugar { ctx.error("Case classes should have a non-implicit parameter list", namePos) ListOfNil } - else originalVparamss.nestedMap(toDefParam) + else originalVparamss.nestedMap(toDefParam(_, keepAnnotations = false)) val constr = cpy.DefDef(constr1)(tparams = constrTparams, vparamss = constrVparamss) val (normalizedBody, enumCases, enumCompanionRef) = { @@ -459,7 +465,7 @@ object desugar { defDef( addEvidenceParams( cpy.DefDef(ddef)(tparams = constrTparams), - evidenceParams(constr1).map(toDefParam)))) + evidenceParams(constr1).map(toDefParam(_, keepAnnotations = false))))) case stat => stat } @@ -482,8 +488,19 @@ object desugar { def anyRef = ref(defn.AnyRefAlias.typeRef) - val derivedTparams = constrTparams.map(derivedTypeParam(_)) - val derivedVparamss = constrVparamss.nestedMap(derivedTermParam(_)) + // Annotations are dropped from the constructor parameters but should be + // preserved in all derived parameters. + val derivedTparams = { + val impliedTparamsIt = impliedTparams.toIterator + constrTparams.map(tparam => derivedTypeParam(tparam) + .withAnnotations(impliedTparamsIt.next().mods.annotations)) + } + val derivedVparamss = { + val constrVparamsIt = constrVparamss.toIterator.flatten + constrVparamss.nestedMap(vparam => derivedTermParam(vparam) + .withAnnotations(constrVparamsIt.next().mods.annotations)) + } + val arity = constrVparamss.head.length val classTycon: Tree = TypeRefTree() // watching is set at end of method @@ -772,16 +789,20 @@ object desugar { } val cdef1 = addEnumFlags { - val originalTparamsIt = impliedTparams.toIterator - val originalVparamsIt = originalVparamss.toIterator.flatten - val tparamAccessors = derivedTparams.map(_.withMods(originalTparamsIt.next().mods)) + val tparamAccessors = { + val impliedTparamsIt = impliedTparams.toIterator + derivedTparams.map(_.withMods(impliedTparamsIt.next().mods)) + } val caseAccessor = if (isCaseClass) CaseAccessor else EmptyFlags - val vparamAccessors = derivedVparamss match { - case first :: rest => - first.map(_.withMods(originalVparamsIt.next().mods | caseAccessor)) ++ - rest.flatten.map(_.withMods(originalVparamsIt.next().mods)) - case _ => - Nil + val vparamAccessors = { + val originalVparamsIt = originalVparamss.toIterator.flatten + derivedVparamss match { + case first :: rest => + first.map(_.withMods(originalVparamsIt.next().mods | caseAccessor)) ++ + rest.flatten.map(_.withMods(originalVparamsIt.next().mods)) + case _ => + Nil + } } cpy.TypeDef(cdef: TypeDef)( name = className, diff --git a/compiler/src/dotty/tools/dotc/ast/Trees.scala b/compiler/src/dotty/tools/dotc/ast/Trees.scala index eabaa6a3e82a..0d86517c16aa 100644 --- a/compiler/src/dotty/tools/dotc/ast/Trees.scala +++ b/compiler/src/dotty/tools/dotc/ast/Trees.scala @@ -327,6 +327,8 @@ object Trees { def rawComment: Option[Comment] = getAttachment(DocComment) + def withAnnotations(annots: List[untpd.Tree]): ThisTree[Untyped] = withMods(rawMods.withAnnotations(annots)) + def withMods(mods: untpd.Modifiers): ThisTree[Untyped] = { val tree = if (myMods == null || (myMods == mods)) this else cloneIn(source) tree.setMods(mods)