diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2014-02-13 11:44:14 +0100 |
---|---|---|
committer | Jason Zaugg <jzaugg@gmail.com> | 2014-02-13 11:44:14 +0100 |
commit | c83e01d47d941265fa5415c0f29a884c904fdfa0 (patch) | |
tree | 5e266fdffd2dad974a8120b266c73c2f89914369 | |
parent | d1c0b359ede63e9e5f5dc230d3ac354c9680c048 (diff) | |
parent | 436bbbe1b3168f18ccf6dee81cf4a8283c2daaec (diff) | |
download | scala-c83e01d47d941265fa5415c0f29a884c904fdfa0.tar.gz scala-c83e01d47d941265fa5415c0f29a884c904fdfa0.tar.bz2 scala-c83e01d47d941265fa5415c0f29a884c904fdfa0.zip |
Merge pull request #3389 from retronym/ticket/8134-2
SI-8134 SI-5954 Fix companions in package object under separate comp.
27 files changed, 118 insertions, 108 deletions
diff --git a/src/compiler/scala/tools/nsc/transform/Flatten.scala b/src/compiler/scala/tools/nsc/transform/Flatten.scala index b4329965fc..c3fbfae322 100644 --- a/src/compiler/scala/tools/nsc/transform/Flatten.scala +++ b/src/compiler/scala/tools/nsc/transform/Flatten.scala @@ -20,12 +20,16 @@ abstract class Flatten extends InfoTransform { /** Updates the owning scope with the given symbol, unlinking any others. */ private def replaceSymbolInCurrentScope(sym: Symbol): Unit = exitingFlatten { + removeSymbolInCurrentScope(sym) + sym.owner.info.decls enter sym + } + + private def removeSymbolInCurrentScope(sym: Symbol): Unit = exitingFlatten { val scope = sym.owner.info.decls val old = (scope lookupUnshadowedEntries sym.name).toList old foreach (scope unlink _) - scope enter sym def old_s = old map (_.sym) mkString ", " - debuglog(s"In scope of ${sym.owner}, unlinked $old_s and entered $sym") + if (old.nonEmpty) debuglog(s"In scope of ${sym.owner}, unlinked $old_s") } private def liftClass(sym: Symbol) { @@ -121,6 +125,8 @@ abstract class Flatten extends InfoTransform { val liftedBuffer = liftedDefs(tree.symbol.enclosingTopLevelClass.owner) val index = liftedBuffer.length liftedBuffer.insert(index, super.transform(tree)) + if (tree.symbol.sourceModule.isStaticModule) + removeSymbolInCurrentScope(tree.symbol.sourceModule) EmptyTree case _ => super.transform(tree) diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index 60cae0c880..9b5b0e1f37 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -169,6 +169,13 @@ trait Namers extends MethodSynthesis { def updatePosFlags(sym: Symbol, pos: Position, flags: Long): Symbol = { debuglog("[overwrite] " + sym) val newFlags = (sym.flags & LOCKED) | flags + sym.rawInfo match { + case tr: TypeRef => + // !!! needed for: pos/t5954d; the uniques type cache will happilly serve up the same TypeRef + // over this mutated symbol, and we witness a stale cache for `parents`. + tr.invalidateCaches() + case _ => + } sym reset NoType setFlag newFlags setPos pos sym.moduleClass andAlso (updatePosFlags(_, pos, moduleClassFlags(flags))) @@ -457,6 +464,17 @@ trait Namers extends MethodSynthesis { var m: Symbol = context.scope lookupModule tree.name val moduleFlags = tree.mods.flags | MODULE if (m.isModule && !m.isPackage && inCurrentScope(m) && (currentRun.canRedefine(m) || m.isSynthetic)) { + // This code accounts for the way the package objects found in the classpath are opened up + // early by the completer of the package itself. If the `packageobjects` phase then finds + // the same package object in sources, we have to clean the slate and remove package object + // members from the package class. + // + // TODO SI-4695 Pursue the approach in https://github.com/scala/scala/pull/2789 that avoids + // opening up the package object on the classpath at all if one exists in source. + if (m.isPackageObject) { + val packageScope = m.enclosingPackageClass.rawInfo.decls + packageScope.filter(_.owner != m.enclosingPackageClass).toList.foreach(packageScope unlink _) + } updatePosFlags(m, tree.pos, moduleFlags) setPrivateWithin(tree, m) m.moduleClass andAlso (setPrivateWithin(tree, _)) diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 02dd63f011..916b8a3e0c 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -858,7 +858,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans val baseClass = clazz.info.baseTypeSeq(i).typeSymbol seenTypes(i) match { case Nil => - println("??? base "+baseClass+" not found in basetypes of "+clazz) + devWarning(s"base $baseClass not found in basetypes of $clazz. This might indicate incorrect caching of TypeRef#parents.") case _ :: Nil => ;// OK case tp1 :: tp2 :: _ => diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 1a53fef4aa..5971782e37 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -1799,32 +1799,6 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper if (settings.isScala211 && mdef.symbol == PredefModule) ensurePredefParentsAreInSameSourceFile(impl2) - // SI-5954. On second compile of a companion class contained in a package object we end up - // with some confusion of names which leads to having two symbols with the same name in the - // same owner. Until that can be straightened out we will warn on companion objects in package - // objects. But this code also tries to be friendly by distinguishing between case classes and - // user written companion pairs - def warnPackageObjectMembers(mdef : ModuleDef) = for (m <- mdef.symbol.info.members) { - // ignore synthetic objects, because the "companion" object to a case class is synthetic and - // we only want one error per case class - if (!m.isSynthetic) { - // can't handle case classes in package objects - if (m.isCaseClass) pkgObjectWarning(m, mdef, "case") - // can't handle companion class/object pairs in package objects - else if ((m.isClass && m.companionModule != NoSymbol && !m.companionModule.isSynthetic) || - (m.isModule && m.companionClass != NoSymbol && !m.companionClass.isSynthetic)) - pkgObjectWarning(m, mdef, "companion") - } - - def pkgObjectWarning(m : Symbol, mdef : ModuleDef, restricted : String) = { - val pkgName = mdef.symbol.ownerChain find (_.isPackage) map (_.decodedName) getOrElse mdef.symbol.toString - context.warning(if (m.pos.isDefined) m.pos else mdef.pos, s"${m} should be placed directly in package ${pkgName} instead of package object ${pkgName}. Under some circumstances companion objects and case classes in package objects can fail to recompile. See https://issues.scala-lang.org/browse/SI-5954.") - } - } - - if (mdef.symbol.isPackageObject) - warnPackageObjectMembers(mdef) - treeCopy.ModuleDef(mdef, typedMods, mdef.name, impl2) setType NoType } diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index 218ad28a03..6010d4eb12 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -2103,6 +2103,10 @@ trait Types trivial = fromBoolean(!sym.isTypeParameter && pre.isTrivial && areTrivialTypes(args)) toBoolean(trivial) } + private[scala] def invalidateCaches(): Unit = { + parentsPeriod = NoPeriod + baseTypeSeqPeriod = NoPeriod + } private[reflect] var parentsCache: List[Type] = _ private[reflect] var parentsPeriod = NoPeriod private[reflect] var baseTypeSeqCache: BaseTypeSeq = _ diff --git a/test/files/neg/package-ob-case.check b/test/files/neg/package-ob-case.check deleted file mode 100644 index 9b0ede1c6d..0000000000 --- a/test/files/neg/package-ob-case.check +++ /dev/null @@ -1,10 +0,0 @@ -package-ob-case.scala:3: warning: it is not recommended to define classes/objects inside of package objects. -If possible, define class X in package foo instead. - case class X(z: Int) { } - ^ -package-ob-case.scala:3: warning: class X should be placed directly in package foo instead of package object foo. Under some circumstances companion objects and case classes in package objects can fail to recompile. See https://issues.scala-lang.org/browse/SI-5954. - case class X(z: Int) { } - ^ -error: No warnings can be incurred under -Xfatal-warnings. -two warnings found -one error found diff --git a/test/files/neg/package-ob-case.flags b/test/files/neg/package-ob-case.flags deleted file mode 100644 index 6c1dd108ae..0000000000 --- a/test/files/neg/package-ob-case.flags +++ /dev/null @@ -1 +0,0 @@ --Xfatal-warnings -Xlint
\ No newline at end of file diff --git a/test/files/neg/t5760-pkgobj-warn.check b/test/files/neg/t5760-pkgobj-warn.check deleted file mode 100644 index a89398c3f7..0000000000 --- a/test/files/neg/t5760-pkgobj-warn.check +++ /dev/null @@ -1,4 +0,0 @@ -stalepkg_2.scala:6: error: Foo is already defined as class Foo in package object stalepkg - class Foo - ^ -one error found diff --git a/test/files/neg/t5954.check b/test/files/neg/t5954.check deleted file mode 100644 index 3950d14e4e..0000000000 --- a/test/files/neg/t5954.check +++ /dev/null @@ -1,18 +0,0 @@ -t5954.scala:36: warning: class D should be placed directly in package A instead of package object A. Under some circumstances companion objects and case classes in package objects can fail to recompile. See https://issues.scala-lang.org/browse/SI-5954. - case class D() - ^ -t5954.scala:35: warning: object C should be placed directly in package A instead of package object A. Under some circumstances companion objects and case classes in package objects can fail to recompile. See https://issues.scala-lang.org/browse/SI-5954. - object C - ^ -t5954.scala:34: warning: trait C should be placed directly in package A instead of package object A. Under some circumstances companion objects and case classes in package objects can fail to recompile. See https://issues.scala-lang.org/browse/SI-5954. - trait C - ^ -t5954.scala:33: warning: object B should be placed directly in package A instead of package object A. Under some circumstances companion objects and case classes in package objects can fail to recompile. See https://issues.scala-lang.org/browse/SI-5954. - object B - ^ -t5954.scala:32: warning: class B should be placed directly in package A instead of package object A. Under some circumstances companion objects and case classes in package objects can fail to recompile. See https://issues.scala-lang.org/browse/SI-5954. - class B - ^ -error: No warnings can be incurred under -Xfatal-warnings. -5 warnings found -one error found diff --git a/test/files/neg/t5954.scala b/test/files/neg/t5954.scala deleted file mode 100644 index 3ccb5ed3ff..0000000000 --- a/test/files/neg/t5954.scala +++ /dev/null @@ -1,46 +0,0 @@ -// if you ever think you've fixed the underlying reason for the warning -// imposed by SI-5954, then here's a test that should pass with two "succes"es -// -//import scala.tools.partest._ -// -//object Test extends DirectTest { -// def code = ??? -// -// def problemCode = """ -// package object A { -// class B -// object B -// case class C() -// } -// """ -// -// def compileProblemCode() = { -// val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator") -// compileString(newCompiler("-cp", classpath, "-d", testOutput.path))(problemCode) -// } -// -// def show() : Unit = { -// for (i <- 0 until 2) { -// compileProblemCode() -// println(s"success ${i + 1}") -// } -// } -//} - -package object A { - // these should be prevented by the implementation restriction - class B - object B - trait C - object C - case class D() - // all the rest of these should be ok - class E - object F - val g = "omg" - var h = "wtf" - def i = "lol" - type j = String - class K(val k : Int) extends AnyVal - implicit class L(val l : Int) -} diff --git a/test/files/neg/t5954.flags b/test/files/pos/package-ob-case.flags index 85d8eb2ba2..85d8eb2ba2 100644 --- a/test/files/neg/t5954.flags +++ b/test/files/pos/package-ob-case.flags diff --git a/test/files/neg/package-ob-case.scala b/test/files/pos/package-ob-case/A_1.scala index 91a1fb7e48..91a1fb7e48 100644 --- a/test/files/neg/package-ob-case.scala +++ b/test/files/pos/package-ob-case/A_1.scala diff --git a/test/files/pos/package-ob-case/B_2.scala b/test/files/pos/package-ob-case/B_2.scala new file mode 100644 index 0000000000..91a1fb7e48 --- /dev/null +++ b/test/files/pos/package-ob-case/B_2.scala @@ -0,0 +1,5 @@ +package foo { + package object foo { + case class X(z: Int) { } + } +} diff --git a/test/files/neg/t5760-pkgobj-warn/stalepkg_1.scala b/test/files/pos/t5760-pkgobj-warn/stalepkg_1.scala index ed4b731bb0..ed4b731bb0 100644 --- a/test/files/neg/t5760-pkgobj-warn/stalepkg_1.scala +++ b/test/files/pos/t5760-pkgobj-warn/stalepkg_1.scala diff --git a/test/files/neg/t5760-pkgobj-warn/stalepkg_2.scala b/test/files/pos/t5760-pkgobj-warn/stalepkg_2.scala index 9abcdbab17..9abcdbab17 100644 --- a/test/files/neg/t5760-pkgobj-warn/stalepkg_2.scala +++ b/test/files/pos/t5760-pkgobj-warn/stalepkg_2.scala diff --git a/test/files/pos/t5954a/A_1.scala b/test/files/pos/t5954a/A_1.scala new file mode 100644 index 0000000000..10ead0b1ca --- /dev/null +++ b/test/files/pos/t5954a/A_1.scala @@ -0,0 +1,6 @@ +package p1 { + object `package` { + implicit class Foo(a: Any) + object Foo + } +} diff --git a/test/files/pos/t5954a/B_2.scala b/test/files/pos/t5954a/B_2.scala new file mode 100644 index 0000000000..10ead0b1ca --- /dev/null +++ b/test/files/pos/t5954a/B_2.scala @@ -0,0 +1,6 @@ +package p1 { + object `package` { + implicit class Foo(a: Any) + object Foo + } +} diff --git a/test/files/pos/t5954b/A_1.scala b/test/files/pos/t5954b/A_1.scala new file mode 100644 index 0000000000..8465e8f8c6 --- /dev/null +++ b/test/files/pos/t5954b/A_1.scala @@ -0,0 +1,6 @@ +package p { + package object base { + class B + object B + } +} diff --git a/test/files/pos/t5954b/B_2.scala b/test/files/pos/t5954b/B_2.scala new file mode 100644 index 0000000000..f7e4704b3e --- /dev/null +++ b/test/files/pos/t5954b/B_2.scala @@ -0,0 +1,5 @@ +package p { + package object base { + case class B() + } +} diff --git a/test/files/pos/t5954c.flags b/test/files/pos/t5954c.flags new file mode 100644 index 0000000000..85d8eb2ba2 --- /dev/null +++ b/test/files/pos/t5954c.flags @@ -0,0 +1 @@ +-Xfatal-warnings diff --git a/test/files/pos/t5954c/A_1.scala b/test/files/pos/t5954c/A_1.scala new file mode 100644 index 0000000000..29ad9547a2 --- /dev/null +++ b/test/files/pos/t5954c/A_1.scala @@ -0,0 +1,18 @@ +package object A { + // these used to should be prevented by the implementation restriction + // but are now allowed + class B + object B + trait C + object C + case class D() + // all the rest of these should be ok + class E + object F + val g = "omg" + var h = "wtf" + def i = "lol" + type j = String + class K(val k : Int) extends AnyVal + implicit class L(val l : Int) +} diff --git a/test/files/pos/t5954c/B_2.scala b/test/files/pos/t5954c/B_2.scala new file mode 100644 index 0000000000..29ad9547a2 --- /dev/null +++ b/test/files/pos/t5954c/B_2.scala @@ -0,0 +1,18 @@ +package object A { + // these used to should be prevented by the implementation restriction + // but are now allowed + class B + object B + trait C + object C + case class D() + // all the rest of these should be ok + class E + object F + val g = "omg" + var h = "wtf" + def i = "lol" + type j = String + class K(val k : Int) extends AnyVal + implicit class L(val l : Int) +} diff --git a/test/files/pos/t5954d.flags b/test/files/pos/t5954d.flags new file mode 100644 index 0000000000..6ced0e7090 --- /dev/null +++ b/test/files/pos/t5954d.flags @@ -0,0 +1 @@ +-Xfatal-warnings -Xdev diff --git a/test/files/pos/t5954d/A_1.scala b/test/files/pos/t5954d/A_1.scala new file mode 100644 index 0000000000..8465e8f8c6 --- /dev/null +++ b/test/files/pos/t5954d/A_1.scala @@ -0,0 +1,6 @@ +package p { + package object base { + class B + object B + } +} diff --git a/test/files/pos/t5954d/B_2.scala b/test/files/pos/t5954d/B_2.scala new file mode 100644 index 0000000000..a4aa2eb587 --- /dev/null +++ b/test/files/pos/t5954d/B_2.scala @@ -0,0 +1,7 @@ +package p { + trait T { + class B + object B + } + package object base extends T +} diff --git a/test/files/pos/t8134/A_1.scala b/test/files/pos/t8134/A_1.scala new file mode 100644 index 0000000000..32bce003fb --- /dev/null +++ b/test/files/pos/t8134/A_1.scala @@ -0,0 +1,4 @@ +// a.scala +package object pkg { + class AnyOps(val x: Any) extends AnyVal +} diff --git a/test/files/pos/t8134/B_2.scala b/test/files/pos/t8134/B_2.scala new file mode 100644 index 0000000000..32bce003fb --- /dev/null +++ b/test/files/pos/t8134/B_2.scala @@ -0,0 +1,4 @@ +// a.scala +package object pkg { + class AnyOps(val x: Any) extends AnyVal +} |