From 292134a4ab5a10052046a722c0b3192b3bfabae7 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 6 Sep 2013 16:26:16 +0200 Subject: SI-7817 Tests to show the foibles of mkAttributedRef To `package`, or not `package`, that is the question. This lines marked with !!! get this wrong, and be remedied by the next commit. This problem is culpable for the crash in the enclosed `pending` test. --- test/files/run/t7817-tree-gen.check | 104 ++++++++++++++++++++++++++++++++++++ test/files/run/t7817-tree-gen.flags | 1 + test/files/run/t7817-tree-gen.scala | 65 ++++++++++++++++++++++ 3 files changed, 170 insertions(+) create mode 100644 test/files/run/t7817-tree-gen.check create mode 100644 test/files/run/t7817-tree-gen.flags create mode 100644 test/files/run/t7817-tree-gen.scala (limited to 'test/files/run') diff --git a/test/files/run/t7817-tree-gen.check b/test/files/run/t7817-tree-gen.check new file mode 100644 index 0000000000..e5fd6a10a0 --- /dev/null +++ b/test/files/run/t7817-tree-gen.check @@ -0,0 +1,104 @@ + + +Joint Compilation: + + typer [ O] - O.this + pickler [ O] - O.this + refchecks [ O] - O.this + uncurry [ O] - O.this + specialize [ O] - O.this + explicitouter [ O] - O.this + erasure [ O] - O.this + posterasure [ O] - C.this.O() + flatten [ O] - C.this.O() + mixin [ O] !!! test.`package`.O() + cleanup [ O] !!! test.`package`.O() + + typer [ P] - P.this + pickler [ P] - P.this + refchecks [ P] - P.this + uncurry [ P] - P.this + specialize [ P] - P.this + explicitouter [ P] - P.this + erasure [ P] - P.this + posterasure [ P] - D.this.P() + flatten [ P] - D.this.P() + mixin [ P] - P() + cleanup [ P] - P() + + typer [ test2.PO] - PO.this + pickler [ test2.PO] - PO.this + refchecks [ test2.PO] - PO.this + uncurry [ test2.PO] - PO.this + specialize [ test2.PO] - PO.this + explicitouter [ test2.PO] - PO.this + erasure [ test2.PO] - PO.this + posterasure [ test2.PO] - test2.`package`.PO + flatten [ test2.PO] - test2.`package`.PO + mixin [ test2.PO] - test2.package$PO + cleanup [ test2.PO] - test2.package$PO + + typer [ test2.bar] - `package`.this.bar + pickler [ test2.bar] - `package`.this.bar + refchecks [ test2.bar] - `package`.this.bar + uncurry [ test2.bar] - `package`.this.bar + specialize [ test2.bar] - `package`.this.bar + explicitouter [ test2.bar] - `package`.this.bar + erasure [ test2.bar] - `package`.this.bar + posterasure [ test2.bar] - test2.`package`.bar + flatten [ test2.bar] - test2.`package`.bar + mixin [ test2.bar] - test2.`package`.bar + cleanup [ test2.bar] - test2.`package`.bar + + + +Separate Compilation: + + typer [ O] - O.this + pickler [ O] - O.this + refchecks [ O] - O.this + uncurry [ O] - O.this + specialize [ O] - O.this + explicitouter [ O] - O.this + erasure [ O] - O.this + posterasure [ O] - C.this.O() + flatten [ O] - C.this.O() + mixin [ O] !!! testSep.`package`.O() + cleanup [ O] !!! testSep.`package`.O() + + typer [ P] - P.this + pickler [ P] - P.this + refchecks [ P] - P.this + uncurry [ P] - P.this + specialize [ P] - P.this + explicitouter [ P] - P.this + erasure [ P] - P.this + posterasure [ P] - DSep.this.P() + flatten [ P] - DSep.this.P() + mixin [ P] - P() + cleanup [ P] - P() + + typer [ PO] - PO.this + pickler [ PO] - PO.this + refchecks [ PO] - PO.this + uncurry [ PO] - PO.this + specialize [ PO] - PO.this + explicitouter [ PO] - PO.this + erasure [ PO] - PO.this + posterasure [ PO] - test2.`package`.PO + flatten [ PO] - test2.`package`.PO + mixin [ PO] - test2.package$PO + cleanup [ PO] - test2.package$PO + + typer [testSep2.bar] - `package`.this.bar + pickler [testSep2.bar] - `package`.this.bar + refchecks [testSep2.bar] - `package`.this.bar + uncurry [testSep2.bar] - `package`.this.bar + specialize [testSep2.bar] - `package`.this.bar + explicitouter [testSep2.bar] - `package`.this.bar + erasure [testSep2.bar] - `package`.this.bar + posterasure [testSep2.bar] - test2.`package`.bar + flatten [testSep2.bar] - test2.`package`.bar + mixin [testSep2.bar] - test2.`package`.bar + cleanup [testSep2.bar] - test2.`package`.bar + diff --git a/test/files/run/t7817-tree-gen.flags b/test/files/run/t7817-tree-gen.flags new file mode 100644 index 0000000000..ce6e93b3da --- /dev/null +++ b/test/files/run/t7817-tree-gen.flags @@ -0,0 +1 @@ +-Ynooptimise \ No newline at end of file diff --git a/test/files/run/t7817-tree-gen.scala b/test/files/run/t7817-tree-gen.scala new file mode 100644 index 0000000000..a8317fda6e --- /dev/null +++ b/test/files/run/t7817-tree-gen.scala @@ -0,0 +1,65 @@ +import scala.tools.partest._ + +// Testing that `mkAttributedRef` doesn't incude the package object test.`package`, +// under joint and separate compilation. + +package testSep { class C { object O } } +package testSep2 { object `package` { object PO; def bar = 0 } } +class DSep { object P } + +object Test extends CompilerTest { + import global._ + override def extraSettings = super.extraSettings + " -d " + testOutput.path + override def sources = List( + """ + package test { class C { object O } } + class D { object P } + package test2 { object `package` { object PO; def bar = 0 } } + """ + ) + def check(source: String, unit: CompilationUnit) = enteringTyper { + def checkTree(msg: String, t: => Tree) = { + val run = currentRun + import run._ + val phases = List(typerPhase, picklerPhase, refchecksPhase, uncurryPhase, specializePhase, + explicitouterPhase, erasurePhase, posterasurePhase, flattenPhase, mixinPhase, cleanupPhase) + for (phase <- phases) { + enteringPhase(phase) { + val error = t.exists(t => t.symbol == NoSymbol) + val errorStr = if (error) "!!!" else " - " + println(f"$phase%18s [$msg%12s] $errorStr $t") + } + } + println("") + } + import rootMirror._ + + println("\n\nJoint Compilation:\n") + + { + val c = staticClass("test.C") + val o = c.info.decl(TermName("O")) + checkTree("O", gen.mkAttributedQualifier(o.moduleClass.thisType)) + val d = staticClass("D") + val p = d.info.decl(TermName("P")) + checkTree("P", gen.mkAttributedQualifier(p.moduleClass.thisType)) + val po = staticModule("test2.package").moduleClass.info.decl(TermName("PO")) + checkTree("test2.PO", gen.mkAttributedQualifier(po.moduleClass.thisType)) + checkTree("test2.bar", gen.mkAttributedRef(po.owner.info.decl(TermName("bar")))) + } + + println("\n\nSeparate Compilation:\n") + + { + val c = typeOf[testSep.C].typeSymbol + val o = c.info.decl(TermName("O")) + checkTree("O", gen.mkAttributedQualifier(o.moduleClass.thisType)) + val d = staticClass("DSep") + val p = d.info.decl(TermName("P")) + checkTree("P", gen.mkAttributedQualifier(p.moduleClass.thisType)) + val po = staticModule("test2.package").moduleClass.info.decl(TermName("PO")) + checkTree("PO", gen.mkAttributedQualifier(po.moduleClass.thisType)) + checkTree("testSep2.bar", gen.mkAttributedRef(po.owner.info.decl(TermName("bar")))) + } + } +} -- cgit v1.2.3 From b255617a6b56af810a3f065ccb887a2b4d36e631 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 6 Sep 2013 16:30:09 +0200 Subject: SI-7817 Fix regression in structural types Calls to structural types are converted to reflective calls in the `cleanup` phase, which, along with `mixin`, does its work after `flatten`. `Symbol#owner` behaves in a phase dependent manner; after flatten the owner of lifted class is given as the enclosing package. Except when they're not. `ModuleSymbol`s representing an object nested inside a class are viewed dually as modules *and* methods (see the comments on `isModuleNotMethod` for some background). When it comes time to flatten, we're presented with a quandary: the method must clearly stay owned by the enclosing class, but surely the lifted module should be owned by the enclosing package, to have the same owner as its associated module class. The `method` nature of this symbol seems to win: override def owner = { if (Statistics.hotEnabled) Statistics.incCounter(ownerCount) if (!isMethod && needsFlatClasses) rawowner.owner else rawowner This wrinkle leads to a wrong turn in `TreeGen#mkAttributedRef`, which incorrectly rewrites `REF(O)` to `p1.`package`.O`. It seems this problem has gone unnoticed because the tree emitted referred to a static symbol (the reflection cache for structural types), and the backend simply elided the qualifier `p1.package`. A recent change to the backend makes it more conservative about dropping qualifiers on the floor, and it started emitting a reference to a package object that doesn't exist. This commit despairingly checks `isDefinedInPackage` of both the module *and* the module class. The test cases from the previous commit illustrated the status quo, and this commit updates the joint compilation test with the bug fix. A new test is to show that the symptom (structural type crash) is now fixed. --- src/reflect/scala/reflect/internal/TreeGen.scala | 2 +- test/files/run/t7817-tree-gen.check | 8 +++--- test/files/run/t7817.scala | 31 ++++++++++++++++++++++++ test/pending/run/t7817.scala | 31 ------------------------ 4 files changed, 36 insertions(+), 36 deletions(-) create mode 100644 test/files/run/t7817.scala delete mode 100644 test/pending/run/t7817.scala (limited to 'test/files/run') diff --git a/src/reflect/scala/reflect/internal/TreeGen.scala b/src/reflect/scala/reflect/internal/TreeGen.scala index 1af8c225f5..4d3e4d9563 100644 --- a/src/reflect/scala/reflect/internal/TreeGen.scala +++ b/src/reflect/scala/reflect/internal/TreeGen.scala @@ -185,7 +185,7 @@ abstract class TreeGen extends macros.TreeBuilder { val needsPackageQualifier = ( (sym ne null) && qualsym.isPackage - && !sym.isDefinedInPackage + && !(sym.isDefinedInPackage || sym.moduleClass.isDefinedInPackage) // SI-7817 work around strangeness in post-flatten `Symbol#owner` ) val pkgQualifier = if (needsPackageQualifier) { diff --git a/test/files/run/t7817-tree-gen.check b/test/files/run/t7817-tree-gen.check index e5fd6a10a0..4ed4b0d94a 100644 --- a/test/files/run/t7817-tree-gen.check +++ b/test/files/run/t7817-tree-gen.check @@ -11,8 +11,8 @@ Joint Compilation: erasure [ O] - O.this posterasure [ O] - C.this.O() flatten [ O] - C.this.O() - mixin [ O] !!! test.`package`.O() - cleanup [ O] !!! test.`package`.O() + mixin [ O] - test.O() + cleanup [ O] - test.O() typer [ P] - P.this pickler [ P] - P.this @@ -63,8 +63,8 @@ Separate Compilation: erasure [ O] - O.this posterasure [ O] - C.this.O() flatten [ O] - C.this.O() - mixin [ O] !!! testSep.`package`.O() - cleanup [ O] !!! testSep.`package`.O() + mixin [ O] - testSep.O() + cleanup [ O] - testSep.O() typer [ P] - P.this pickler [ P] - P.this diff --git a/test/files/run/t7817.scala b/test/files/run/t7817.scala new file mode 100644 index 0000000000..905b8aeb09 --- /dev/null +++ b/test/files/run/t7817.scala @@ -0,0 +1,31 @@ +import language.reflectiveCalls + +package test { + class C1 { + object O { + def struct(s: {def foo: Any}) = s.foo + } + } + trait T { + object O { + def struct(s: {def foo: Any}) = s.foo + } + } + object O1 extends T + + object O2 { + object O { + def struct(s: {def foo: Any}) = s.foo + } + } +} + +object Test extends App { + object fooable { def foo = "foo" } + def check(result: Any) = assert(result == "foo", result.toString) + + val s = new test.C1 + check(s.O.struct(fooable)) + check(test.O1.O.struct(fooable)) + check(test.O2.O.struct(fooable)) +} diff --git a/test/pending/run/t7817.scala b/test/pending/run/t7817.scala deleted file mode 100644 index 905b8aeb09..0000000000 --- a/test/pending/run/t7817.scala +++ /dev/null @@ -1,31 +0,0 @@ -import language.reflectiveCalls - -package test { - class C1 { - object O { - def struct(s: {def foo: Any}) = s.foo - } - } - trait T { - object O { - def struct(s: {def foo: Any}) = s.foo - } - } - object O1 extends T - - object O2 { - object O { - def struct(s: {def foo: Any}) = s.foo - } - } -} - -object Test extends App { - object fooable { def foo = "foo" } - def check(result: Any) = assert(result == "foo", result.toString) - - val s = new test.C1 - check(s.O.struct(fooable)) - check(test.O1.O.struct(fooable)) - check(test.O2.O.struct(fooable)) -} -- cgit v1.2.3