From 075f6f260ccfba83b118c308195d1ede2e66ad18 Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Mon, 4 Nov 2013 16:51:20 -0800 Subject: SI-6546 InnerClasses attribute refers to absent class At issue is that the optimizer would eliminate closure classes completely, then neglect to eliminate those classes from the container's InnerClasses attribute. This breaks tooling which expects those entries to correspond to real classes. The code change is essentially mgarcia's - I minimized it and put the caches in perRunCaches, and added the test case which verifies that after being compiled under -optimise, there are no inner classes. Before/after: 7,8d6 < InnerClasses: < public final #22; //class A_1$$anonfun$f$1 37,45c35,40 < #21 = Utf8 A_1$$anonfun$f$1 < #22 = Class #21 // A_1$$anonfun$f$1 < #23 = Utf8 Code --- > #21 = Utf8 Code --- src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala | 4 +++- .../scala/tools/nsc/backend/opt/DeadCodeElimination.scala | 8 +++++++- test/files/run/t6546.flags | 1 + test/files/run/t6546/A_1.scala | 6 ++++++ test/files/run/t6546/B_2.scala | 8 ++++++++ 5 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 test/files/run/t6546.flags create mode 100644 test/files/run/t6546/A_1.scala create mode 100644 test/files/run/t6546/B_2.scala diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala index 7c46d648fe..97140aca2e 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala @@ -86,6 +86,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { if (settings.Xdce.value) for ((sym, cls) <- icodes.classes if inliner.isClosureClass(sym) && !deadCode.liveClosures(sym)) { log(s"Optimizer eliminated ${sym.fullNameString}") + deadCode.elidedClosures += sym icodes.classes -= sym } @@ -624,7 +625,8 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { innerClassBuffer += m } - val allInners: List[Symbol] = innerClassBuffer.toList + val allInners: List[Symbol] = innerClassBuffer.toList filterNot deadCode.elidedClosures + if (allInners.nonEmpty) { debuglog(csym.fullName('.') + " contains " + allInners.size + " inner classes.") diff --git a/src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala b/src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala index 1beed3f420..db56f61f16 100644 --- a/src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala +++ b/src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala @@ -40,7 +40,13 @@ abstract class DeadCodeElimination extends SubComponent { } /** closures that are instantiated at least once, after dead code elimination */ - val liveClosures: mutable.Set[Symbol] = new mutable.HashSet() + val liveClosures = perRunCaches.newSet[Symbol]() + + /** closures that are eliminated, populated by GenASM.AsmPhase.run() + * these class symbols won't have a .class physical file, thus shouldn't be included in InnerClasses JVM attribute, + * otherwise some tools get confused or slow (SI-6546) + * */ + val elidedClosures = perRunCaches.newSet[Symbol]() /** Remove dead code. */ diff --git a/test/files/run/t6546.flags b/test/files/run/t6546.flags new file mode 100644 index 0000000000..eb4d19bcb9 --- /dev/null +++ b/test/files/run/t6546.flags @@ -0,0 +1 @@ +-optimise \ No newline at end of file diff --git a/test/files/run/t6546/A_1.scala b/test/files/run/t6546/A_1.scala new file mode 100644 index 0000000000..bd086c08f8 --- /dev/null +++ b/test/files/run/t6546/A_1.scala @@ -0,0 +1,6 @@ +final class Opt { + @inline def getOrElse(x: => String): String = "" +} +class A_1 { + def f(x: Opt): String = x getOrElse null +} diff --git a/test/files/run/t6546/B_2.scala b/test/files/run/t6546/B_2.scala new file mode 100644 index 0000000000..64ec966f75 --- /dev/null +++ b/test/files/run/t6546/B_2.scala @@ -0,0 +1,8 @@ +import scala.tools.partest.BytecodeTest + +object Test extends BytecodeTest { + def show: Unit = { + val node = loadClassNode("A_1") + assert(node.innerClasses.isEmpty, node.innerClasses) + } +} -- cgit v1.2.3 From e09a8a2b7f821be43703bd5bf3a064e171d8f66c Mon Sep 17 00:00:00 2001 From: Vlad Ureche Date: Mon, 2 Sep 2013 19:08:49 +0200 Subject: SI-4012 Mixin and specialization work well The bug was fixed along with SI-7638 in 504b5f3. --- test/files/pos/SI-4012-a.scala | 7 +++++++ test/files/pos/SI-4012-b.scala | 15 +++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 test/files/pos/SI-4012-a.scala create mode 100644 test/files/pos/SI-4012-b.scala diff --git a/test/files/pos/SI-4012-a.scala b/test/files/pos/SI-4012-a.scala new file mode 100644 index 0000000000..7fceeea3c3 --- /dev/null +++ b/test/files/pos/SI-4012-a.scala @@ -0,0 +1,7 @@ +trait C1[+A] { + def head: A = sys.error("") +} +trait C2[@specialized +A] extends C1[A] { + override def head: A = super.head +} +class C3 extends C2[Char] diff --git a/test/files/pos/SI-4012-b.scala b/test/files/pos/SI-4012-b.scala new file mode 100644 index 0000000000..6bc8592766 --- /dev/null +++ b/test/files/pos/SI-4012-b.scala @@ -0,0 +1,15 @@ +trait Super[@specialized(Int) A] { + def superb = 0 +} + +object Sub extends Super[Int] { + // it is expected that super[Super].superb crashes, since + // specialization does parent class rewiring, and the super + // of Sub becomes Super$mcII$sp and not Super. But I consider + // this normal behavior -- if you want, I can modify duplicatiors + // to make this work, but I consider it's best to keep this + // let the user know Super is not the superclass anymore. + // super[Super].superb - Vlad + super.superb // okay + override def superb: Int = super.superb // okay +} -- cgit v1.2.3