From 731ed385dea0196305a0c527303649ea0325de63 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 15 Jan 2014 16:00:30 +0100 Subject: SI-8134 SI-5954 Fix companions in package object under separate comp. The tests cases enclosed exhibited two failures modes under separate compilation. 1. When a synthetic companion object for a case- or implicit-class defined in a package object is called for, `Namer#ensureCompanionObject` is used to check for an explicitly defined companion before decided to create a synthetic one. This lookup of an existing companion symbol by `companionObjectOf` would locate a symbol backed by a class file which was in the scope of the enclosing package class. Furthermore, because the owner of that symbol is the package object class that has now been noted as corresponding to a source file in the current run, the class-file backed module symbol is *also* deemed to be from the current run. (This logic is in `Run#compiles`.) Thinking the companion module already existed, no synthetic module was created, which would lead to a crash in extension methods, which needs to add methods to it. 2. In cases when the code explicitly contains the companion pair, we still ran into problems in the backend whereby the class-file based and source-file based symbols for the module ended up in the same scope (of the package class). This tripped an assertion in `Symbol#companionModule`. We get into these problems because of the eager manner in which class-file based package object are opened in `openPackageModule`. The members of the module are copied into the scope of the enclosing package: scala> ScalaPackage.info.member(nme.List) res0: $r#59116.intp#45094.global#28436.Symbol#29451 = value List#2462 scala> ScalaPackage.info.member(nme.PACKAGE).info.member(nme.List) res1: $r#59116.intp#45094.global#28436.Symbol#29451 = value List#2462 This seems to require a two-pronged defense: 1. When we attach a pre-existing symbol for a package object symbol to the tree of its new source, unlink the "forwarder" symbols (its decls from the enclosing package class. 2. In `Flatten`, in the spirit of `replaceSymbolInCurrentScope`, remove static member modules from the scope of the enclosing package object (aka `exitingFlatten(nestedModule.owner)`). This commit also removes the warnings about defining companions in package objects and converts those neg tests to pos (with -Xfatal-warnings to prove they are warning free.) Defining nested classes/objects in package objects still has a drawback: you can't shift a class from the package to the package object, or vice versa, in a binary compatible manner, because of the `package$` prefix on the flattened name of nested classes. For this reason, the `-Xlint` warning about this remains. This issue is tracked as SI-4344. However, if one heeds this warning and incrementatlly recompiles, we no longer need to run into a DoubleDefinition error (which was dressed up with a more specific diagnostic in SI-5760.) The neg test case for that bug has been converted to a pos. --- test/files/neg/package-ob-case.check | 10 ----- test/files/neg/package-ob-case.flags | 1 - test/files/neg/package-ob-case.scala | 5 --- test/files/neg/t5760-pkgobj-warn.check | 4 -- test/files/neg/t5760-pkgobj-warn/stalepkg_1.scala | 11 ------ test/files/neg/t5760-pkgobj-warn/stalepkg_2.scala | 11 ------ test/files/neg/t5954.check | 18 --------- test/files/neg/t5954.flags | 1 - test/files/neg/t5954.scala | 46 ----------------------- test/files/pos/package-ob-case.flags | 1 + test/files/pos/package-ob-case/A_1.scala | 5 +++ test/files/pos/package-ob-case/B_2.scala | 5 +++ test/files/pos/t5760-pkgobj-warn/stalepkg_1.scala | 11 ++++++ test/files/pos/t5760-pkgobj-warn/stalepkg_2.scala | 11 ++++++ test/files/pos/t5954a/A_1.scala | 6 +++ test/files/pos/t5954a/B_2.scala | 6 +++ test/files/pos/t5954b/A_1.scala | 6 +++ test/files/pos/t5954b/B_2.scala | 5 +++ test/files/pos/t5954c.flags | 1 + test/files/pos/t5954c/A_1.scala | 18 +++++++++ test/files/pos/t5954c/B_2.scala | 18 +++++++++ test/files/pos/t8134/A_1.scala | 4 ++ test/files/pos/t8134/B_2.scala | 4 ++ 23 files changed, 101 insertions(+), 107 deletions(-) delete mode 100644 test/files/neg/package-ob-case.check delete mode 100644 test/files/neg/package-ob-case.flags delete mode 100644 test/files/neg/package-ob-case.scala delete mode 100644 test/files/neg/t5760-pkgobj-warn.check delete mode 100644 test/files/neg/t5760-pkgobj-warn/stalepkg_1.scala delete mode 100644 test/files/neg/t5760-pkgobj-warn/stalepkg_2.scala delete mode 100644 test/files/neg/t5954.check delete mode 100644 test/files/neg/t5954.flags delete mode 100644 test/files/neg/t5954.scala create mode 100644 test/files/pos/package-ob-case.flags create mode 100644 test/files/pos/package-ob-case/A_1.scala create mode 100644 test/files/pos/package-ob-case/B_2.scala create mode 100644 test/files/pos/t5760-pkgobj-warn/stalepkg_1.scala create mode 100644 test/files/pos/t5760-pkgobj-warn/stalepkg_2.scala create mode 100644 test/files/pos/t5954a/A_1.scala create mode 100644 test/files/pos/t5954a/B_2.scala create mode 100644 test/files/pos/t5954b/A_1.scala create mode 100644 test/files/pos/t5954b/B_2.scala create mode 100644 test/files/pos/t5954c.flags create mode 100644 test/files/pos/t5954c/A_1.scala create mode 100644 test/files/pos/t5954c/B_2.scala create mode 100644 test/files/pos/t8134/A_1.scala create mode 100644 test/files/pos/t8134/B_2.scala (limited to 'test') 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/package-ob-case.scala b/test/files/neg/package-ob-case.scala deleted file mode 100644 index 91a1fb7e48..0000000000 --- a/test/files/neg/package-ob-case.scala +++ /dev/null @@ -1,5 +0,0 @@ -package foo { - package object foo { - case class X(z: Int) { } - } -} 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/t5760-pkgobj-warn/stalepkg_1.scala b/test/files/neg/t5760-pkgobj-warn/stalepkg_1.scala deleted file mode 100644 index ed4b731bb0..0000000000 --- a/test/files/neg/t5760-pkgobj-warn/stalepkg_1.scala +++ /dev/null @@ -1,11 +0,0 @@ - -package object stalepkg { - class Foo -} - -package stalepkg { - object Test { - def main(args: Array[String]) { - } - } -} diff --git a/test/files/neg/t5760-pkgobj-warn/stalepkg_2.scala b/test/files/neg/t5760-pkgobj-warn/stalepkg_2.scala deleted file mode 100644 index 9abcdbab17..0000000000 --- a/test/files/neg/t5760-pkgobj-warn/stalepkg_2.scala +++ /dev/null @@ -1,11 +0,0 @@ - -package object stalepkg { -} - -package stalepkg { - class Foo - object Test { - def main(args: Array[String]) { - } - } -} 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.flags b/test/files/neg/t5954.flags deleted file mode 100644 index 85d8eb2ba2..0000000000 --- a/test/files/neg/t5954.flags +++ /dev/null @@ -1 +0,0 @@ --Xfatal-warnings 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/pos/package-ob-case.flags b/test/files/pos/package-ob-case.flags new file mode 100644 index 0000000000..85d8eb2ba2 --- /dev/null +++ b/test/files/pos/package-ob-case.flags @@ -0,0 +1 @@ +-Xfatal-warnings diff --git a/test/files/pos/package-ob-case/A_1.scala b/test/files/pos/package-ob-case/A_1.scala new file mode 100644 index 0000000000..91a1fb7e48 --- /dev/null +++ b/test/files/pos/package-ob-case/A_1.scala @@ -0,0 +1,5 @@ +package foo { + package object foo { + case class X(z: Int) { } + } +} 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/pos/t5760-pkgobj-warn/stalepkg_1.scala b/test/files/pos/t5760-pkgobj-warn/stalepkg_1.scala new file mode 100644 index 0000000000..ed4b731bb0 --- /dev/null +++ b/test/files/pos/t5760-pkgobj-warn/stalepkg_1.scala @@ -0,0 +1,11 @@ + +package object stalepkg { + class Foo +} + +package stalepkg { + object Test { + def main(args: Array[String]) { + } + } +} diff --git a/test/files/pos/t5760-pkgobj-warn/stalepkg_2.scala b/test/files/pos/t5760-pkgobj-warn/stalepkg_2.scala new file mode 100644 index 0000000000..9abcdbab17 --- /dev/null +++ b/test/files/pos/t5760-pkgobj-warn/stalepkg_2.scala @@ -0,0 +1,11 @@ + +package object stalepkg { +} + +package stalepkg { + class Foo + object Test { + def main(args: Array[String]) { + } + } +} 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/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 +} -- cgit v1.2.3