From 8b09bab5261b849f79448d43845b08fe358fb885 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Sat, 11 May 2019 15:04:16 +0200 Subject: [PATCH 1/2] Make moduleVal/moduleClass return the current symbol when possible This is more intuitive in my opinion. As a side-effect, this makes the logic for creating stub package symbols in `staticRef` more robust which broke the `missingDottyLib` test. This is fixed by moving the `isPackageFromCoreLibMissing` check earlier. --- .../dotty/tools/dotc/core/Denotations.scala | 2 +- .../tools/dotc/core/SymDenotations.scala | 41 ++++++++++++------- .../tools/dotc/interactive/Completion.scala | 2 +- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index ccff9fd1c3c2..9d75b93ff334 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -1301,12 +1301,12 @@ object Denotations { if (owner.exists) { val result = if (isPackage) owner.info.decl(selector) else owner.info.member(selector) if (result.exists) result + else if (isPackageFromCoreLibMissing) throw new MissingCoreLibraryException(selector.toString) else { val alt = if (generateStubs) missingHook(owner.symbol.moduleClass, selector) else NoSymbol if (alt.exists) alt.denot - else if (isPackageFromCoreLibMissing) throw new MissingCoreLibraryException(selector.toString) else MissingRef(owner, selector) } } diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 78f5be9d8b76..185c5fad2625 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -852,7 +852,9 @@ object SymDenotations { * During completion, references to moduleClass and sourceModules are stored in * the completers. */ - /** The class implementing this module, NoSymbol if not applicable. */ + /** If this a module, return the corresponding class, if this is a module, return itself, + * otherwise NoSymbol + */ final def moduleClass(implicit ctx: Context): Symbol = { def notFound = { if (Config.showCompletions) println(s"missing module class for $name: $myInfo") @@ -870,23 +872,34 @@ object SymDenotations { } case _ => notFound } - else NoSymbol + else if (this is ModuleClass) + symbol + else + NoSymbol } - /** The module implemented by this module class, NoSymbol if not applicable. */ - final def sourceModule(implicit ctx: Context): Symbol = myInfo match { - case ClassInfo(_, _, _, _, selfType) if this is ModuleClass => - def sourceOfSelf(tp: TypeOrSymbol): Symbol = tp match { - case tp: TermRef => tp.symbol - case tp: Symbol => sourceOfSelf(tp.info) - case tp: RefinedType => sourceOfSelf(tp.parent) + /** If this a module class, return the corresponding module, if this is a module, return itself, + * otherwise NoSymbol + */ + final def sourceModule(implicit ctx: Context): Symbol = + if (this is ModuleClass) + myInfo match { + case ClassInfo(_, _, _, _, selfType) => + def sourceOfSelf(tp: TypeOrSymbol): Symbol = tp match { + case tp: TermRef => tp.symbol + case tp: Symbol => sourceOfSelf(tp.info) + case tp: RefinedType => sourceOfSelf(tp.parent) + } + sourceOfSelf(selfType) + case info: LazyType => + info.sourceModule + case _ => + NoSymbol } - sourceOfSelf(selfType) - case info: LazyType => - info.sourceModule - case _ => + else if (this is ModuleVal) + symbol + else NoSymbol - } /** The field accessed by this getter or setter, or if it does not exist, the getter */ def accessedFieldOrGetter(implicit ctx: Context): Symbol = { diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index 79e78443378f..869d2017f161 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -254,7 +254,7 @@ object Completion { !sym.isAbsent && !sym.isPrimaryConstructor && sym.sourceSymbol.exists && - (!sym.is(Package) || !sym.moduleClass.exists) && + (!sym.is(Package) || sym.is(ModuleClass)) && !sym.is(allOf(Mutable, Accessor)) && !sym.isPackageObject && !sym.is(Artifact) && From a11de49a6dd6a2cc64cef419b0db76a864bbb0fa Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Fri, 10 May 2019 19:59:43 +0200 Subject: [PATCH 2/2] Fix #6460: less forcing in checkNonCyclic checkNonCyclic was forcing too many infos because it didn't account for the case where the prefix is a ModuleVal and not a ModuleClass. Fixing this solves the incremental compilation issue in the Dotty build reported in #6460 were the root cause was a completion cycle when jointly compiling SymbolLoaders and Types that looked like this: Complete `object tpd` Complete its parent `Trees.Instance` Complete its type parameter `T >: Untyped <: Type` run `checkNonCyclic` on the the type parameter definition Complete `Type` Complete `import ast.tpd._` in `Types.scala` Eventually this requires unpickling the selection `Trees.Instance#T` which gets a `NoDenotation` since `T` hasn't been entered yet. As a side-effect this also means that the neg/toplevel-cyclic test no longer leads to an "illegal cyclic reference" error, instead it now causes "recursion limit exceeded" errors, which are less nice but still acceptable. --- compiler/src/dotty/tools/dotc/typer/Checking.scala | 2 +- .../tools/dotc/BootstrappedOnlyCompilationTests.scala | 10 +++++++++- tests/neg/toplevel-cyclic/defs_1.scala | 2 +- tests/neg/toplevel-cyclic/moredefs_1.scala | 2 +- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 6fc0de6b9cfa..3c4cd6194de4 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -240,7 +240,7 @@ object Checking { ) case prefix: NamedType => (!sym.is(Private) && prefix.derivesFrom(sym.owner)) || - (!prefix.symbol.isStaticOwner && isInteresting(prefix.prefix)) + (!prefix.symbol.moduleClass.isStaticOwner && isInteresting(prefix.prefix)) case SuperType(thistp, _) => isInteresting(thistp) case AndType(tp1, tp2) => isInteresting(tp1) || isInteresting(tp2) case OrType(tp1, tp2) => isInteresting(tp1) && isInteresting(tp2) diff --git a/compiler/test/dotty/tools/dotc/BootstrappedOnlyCompilationTests.scala b/compiler/test/dotty/tools/dotc/BootstrappedOnlyCompilationTests.scala index 29262975d6f0..e672decb4779 100644 --- a/compiler/test/dotty/tools/dotc/BootstrappedOnlyCompilationTests.scala +++ b/compiler/test/dotty/tools/dotc/BootstrappedOnlyCompilationTests.scala @@ -48,7 +48,15 @@ class BootstrappedOnlyCompilationTests extends ParallelTesting { compileDir("compiler/src/dotty/tools/dotc/reporting", withCompilerOptions), compileDir("compiler/src/dotty/tools/dotc/typer", withCompilerOptions), compileDir("compiler/src/dotty/tools/dotc/util", withCompilerOptions), - compileDir("compiler/src/dotty/tools/io", withCompilerOptions) + compileDir("compiler/src/dotty/tools/io", withCompilerOptions), + compileList( + "testIssue6460", + List( + "compiler/src/dotty/tools/dotc/core/SymbolLoaders.scala", + "compiler/src/dotty/tools/dotc/core/Types.scala" + ), + withCompilerOptions + ), ).checkCompile() } diff --git a/tests/neg/toplevel-cyclic/defs_1.scala b/tests/neg/toplevel-cyclic/defs_1.scala index e2a52496b659..4c78584a5bdf 100644 --- a/tests/neg/toplevel-cyclic/defs_1.scala +++ b/tests/neg/toplevel-cyclic/defs_1.scala @@ -1,2 +1,2 @@ -type A = B // error: illegal cyclic reference +type A = B // error: recursion limit exceeded diff --git a/tests/neg/toplevel-cyclic/moredefs_1.scala b/tests/neg/toplevel-cyclic/moredefs_1.scala index adca31817456..03cc49e531ec 100644 --- a/tests/neg/toplevel-cyclic/moredefs_1.scala +++ b/tests/neg/toplevel-cyclic/moredefs_1.scala @@ -1 +1 @@ -type B = A \ No newline at end of file +type B = A // error: recursion limit exceeded // error: recursion limit exceeded