Merge pull request #3745 from lrytz/t8582

SI-8582 emit InnerClasses attribute in GenBCode

16 files changed, 235 insertions, 41 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/AsmUtils.scala b/src/compiler/scala/tools/nsc/backend/jvm/AsmUtils.scala
new file mode 100644
index 0000000000..856f85d9e3
--- /dev/null
+++ b/src/compiler/scala/tools/nsc/backend/jvm/AsmUtils.scala
@@ -0,0 +1,53 @@
+/* NSC -- new Scala compiler
+ * Copyright 2005-2014 LAMP/EPFL
+ * @author  Martin Odersky
+ */
+
+package scala.tools.nsc.backend.jvm
+
+import scala.tools.asm.tree.{ClassNode, MethodNode}
+import java.io.PrintWriter
+import scala.tools.asm.util.{TraceClassVisitor, TraceMethodVisitor, Textifier}
+
+object AsmUtils {
+
+  /**
+   * Print the bytecode of methods generated by GenBCode to the standard output. Only methods
+   * whose name contains `traceMethodPattern` are traced.
+   */
+  final val traceMethodEnabled = false
+  final val traceMethodPattern = ""
+
+  /**
+   * Print the bytecode of classes generated by GenBCode to the standard output.
+   */
+  final val traceClassEnabled = false
+  final val traceClassPattern = ""
+
+  /**
+   * Print the bytedcode of classes as they are serialized by the ASM library. The serialization
+   * performed by `asm.ClassWriter` can change the code generated by GenBCode. For example, it
+   * introduces stack map frames, it computes the maximal stack sizes, and it replaces dead
+   * code by NOPs (see also https://github.com/scala/scala/pull/3726#issuecomment-42861780).
+   */
+  final val traceSerializedClassEnabled = false
+  final val traceSerializedClassPattern = ""
+
+  def traceMethod(mnode: MethodNode): Unit = {
+    println(s"Bytecode for method ${mnode.name}")
+    val p = new Textifier
+    val tracer = new TraceMethodVisitor(p)
+    mnode.accept(tracer)
+    val w = new PrintWriter(System.out)
+    p.print(w)
+    w.flush()
+  }
+
+  def traceClass(cnode: ClassNode): Unit = {
+    println(s"Bytecode for class ${cnode.name}")
+    val w = new PrintWriter(System.out)
+    cnode.accept(new TraceClassVisitor(w))
+    w.flush()
+  }
+
+}
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
index 92ebe5027a..3d1b646069 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
@@ -808,7 +808,7 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
         emit(asm.Opcodes.ATHROW) // ICode enters here into enterIgnoreMode, we'll rely instead on DCE at ClassNode level.
       } else if (from.isNullType) {
         bc drop from
-        mnode.visitInsn(asm.Opcodes.ACONST_NULL)
+        emit(asm.Opcodes.ACONST_NULL)
       }
       else (from, to) match  {
         case (BYTE, LONG) | (SHORT, LONG) | (CHAR, LONG) | (INT, LONG) => bc.emitT2T(INT, LONG)
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala
index f800dbf9cd..1ede914288 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala
@@ -1108,7 +1108,7 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
 
       constructor.visitVarInsn(asm.Opcodes.ALOAD, 0)
       // push the class
-      constructor.visitLdcInsn(exemplar(cls).c)
+      constructor.visitLdcInsn(exemplar(cls).c.toASMType)
 
       // push the string array of field information
       constructor.visitLdcInsn(new java.lang.Integer(fieldList.length))
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala
index 360ce58ecc..ee9be5b11c 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala
@@ -14,6 +14,8 @@ import scala.tools.nsc.symtab._
 import scala.annotation.switch
 
 import scala.tools.asm
+import scala.tools.asm.util.{TraceMethodVisitor, ASMifier}
+import java.io.PrintWriter
 
 /*
  *
@@ -114,9 +116,13 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
       addClassFields()
 
       innerClassBufferASM ++= trackMemberClasses(claszSymbol, Nil)
-
       gen(cd.impl)
+      addInnerClassesASM(cnode, innerClassBufferASM.toList)
+
+      if (AsmUtils.traceClassEnabled && cnode.name.contains(AsmUtils.traceClassPattern))
+        AsmUtils.traceClass(cnode)
 
+      cnode.innerClasses
       assert(cd.symbol == claszSymbol, "Someone messed up BCodePhase.claszSymbol during genPlainClass().")
 
     } // end of method genPlainClass()
@@ -639,6 +645,10 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
         // Note we don't invoke visitMax, thus there are no FrameNode among mnode.instructions.
         // The only non-instruction nodes to be found are LabelNode and LineNumberNode.
       }
+
+      if (AsmUtils.traceMethodEnabled && mnode.name.contains(AsmUtils.traceMethodPattern))
+        AsmUtils.traceMethod(mnode)
+
       mnode = null
     } // end of method genDefDef()
 
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BytecodeWriters.scala b/src/compiler/scala/tools/nsc/backend/jvm/BytecodeWriters.scala
index 8e6c09213f..1d29fdee10 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BytecodeWriters.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BytecodeWriters.scala
@@ -1,6 +1,6 @@
 /* NSC -- new Scala compiler
  * Copyright 2005-2013 LAMP/EPFL
- * @author  Paul Phillips
+ * @author  Martin Odersky
  */
 
 package scala.tools.nsc
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala
index 193100474c..61cf76f524 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala
@@ -243,6 +243,17 @@ abstract class GenBCode extends BCodeSyncAndTry {
         val plainC  = SubItem3(plain.name, getByteArray(plain))
         val beanC   = if (bean == null)   null else SubItem3(bean.name, getByteArray(bean))
 
+        if (AsmUtils.traceSerializedClassEnabled && plain.name.contains(AsmUtils.traceSerializedClassPattern)) {
+          def readClass(bytes: Array[Byte]): asm.tree.ClassNode = {
+            val node = new asm.tree.ClassNode()
+            new asm.ClassReader(bytes).accept(node, 0)
+            node
+          }
+          if (mirrorC != null) AsmUtils.traceClass(readClass(mirrorC.jclassBytes))
+          AsmUtils.traceClass(readClass(plainC.jclassBytes))
+          if (beanC != null) AsmUtils.traceClass(readClass(beanC.jclassBytes))
+        }
+
         q3 add Item3(arrivalPos, mirrorC, plainC, beanC, outFolder)
 
       }
diff --git a/test/files/jvm/beanInfo.check b/test/files/jvm/beanInfo.check
new file mode 100644
index 0000000000..d74e127711
--- /dev/null
+++ b/test/files/jvm/beanInfo.check
@@ -0,0 +1,6 @@
+property descriptors
+x -- int -- public int p.C.x() -- null
+y -- class java.lang.String -- public java.lang.String p.C.y() -- public void p.C.y_$eq(java.lang.String)
+z -- class scala.collection.immutable.List -- public scala.collection.immutable.List p.C.z() -- public void p.C.z_$eq(scala.collection.immutable.List)
+method descriptors
+f -- public p.C p.C.f()
diff --git a/test/files/jvm/beanInfo/C_1.scala b/test/files/jvm/beanInfo/C_1.scala
new file mode 100644
index 0000000000..a338abea1d
--- /dev/null
+++ b/test/files/jvm/beanInfo/C_1.scala
@@ -0,0 +1,9 @@
+package p
+
+@scala.beans.BeanInfo
+class C {
+  val x: Int = 0
+  var y: String = ""
+  var z: List[_] = Nil
+  def f: C = ???
+}
diff --git a/test/files/jvm/beanInfo/Test_2.scala b/test/files/jvm/beanInfo/Test_2.scala
new file mode 100644
index 0000000000..fa9b6e1391
--- /dev/null
+++ b/test/files/jvm/beanInfo/Test_2.scala
@@ -0,0 +1,17 @@
+object Test extends App {
+  val info = java.beans.Introspector.getBeanInfo(classOf[p.C])
+
+  println("property descriptors")
+
+  val pds = info.getPropertyDescriptors
+  for (pd <- pds) {
+    println(s"${pd.getName} -- ${pd.getPropertyType} -- ${pd.getReadMethod} -- ${pd.getWriteMethod}")
+  }
+
+  println("method descriptors")
+
+  val mds = info.getMethodDescriptors
+  for (md <- mds) {
+    println(s"${md.getName} -- ${md.getMethod}")
+  }
+}
diff --git a/test/files/jvm/t8582.check b/test/files/jvm/t8582.check
new file mode 100644
index 0000000000..564f482ff8
--- /dev/null
+++ b/test/files/jvm/t8582.check
@@ -0,0 +1,44 @@
+getClass on module gives module class
+ class p1.p2.Singleton$Singleton$
+
+Nested module classes are found through reflection
+ p1.p2.Singleton$Singleton$: List(class p1.p2.Singleton$Singleton$Singleton$)
+
+Reflection can find direct nested classes (A1-B1-C1)
+ A1: List(class A1$B1)
+ A1$B1: List(class A1$B1$C1)
+ A1$B1$C1: List()
+
+Reflection can find direct nested classes (A2-B2-C2)
+ A2: List(class A2$B2)
+ A2$B2: List(class A2$B2$C2)
+ A2$B2$C2: List()
+
+Mirror classes have the same InnerClass attributes as the corresponding module class:
+ className[p1/p2/Singleton$Singleton$] outerClassName[p1/p2/Singleton] innerName[Singleton$] access[9]
+Module class
+ className[p1/p2/Singleton$Singleton$] outerClassName[p1/p2/Singleton] innerName[Singleton$] access[9]
+
+An outer class has a InnerClass attribute for direct nested classes
+ className[A1$B1] outerClassName[A1] innerName[B1] access[1]
+A nested class has an InnerClass attribute for itself (and also for its nested classes)
+ className[A1$B1] outerClassName[A1] innerName[B1] access[1]
+ className[A1$B1$C1] outerClassName[A1$B1] innerName[C1] access[1]
+C1 is a nested class, so it has an InnerClass attribute for itself.
+Because that attribute leads to an entry for B1 in the constant pool, C1 needs an InnerClass attribute for B1.
+ className[A1$B1] outerClassName[A1] innerName[B1] access[1]
+ className[A1$B1$C1] outerClassName[A1$B1] innerName[C1] access[1]
+
+The BeanInfo class has the same InnerClass attributes as the corresponding bean
+ className[A1$B1] outerClassName[A1] innerName[B1] access[1]
+ className[A1$B1$C1] outerClassName[A1$B1] innerName[C1] access[1]
+
+Class A2 mentions class C2 in the constant pool (due to method  f), therefore it needs an InnerClass attribute for C1
+ className[A2$B2] outerClassName[A2] innerName[B2] access[1]
+ className[A2$B2$C2] outerClassName[A2$B2] innerName[C2] access[1]
+B2
+ className[A2$B2] outerClassName[A2] innerName[B2] access[1]
+ className[A2$B2$C2] outerClassName[A2$B2] innerName[C2] access[1]
+C2
+ className[A2$B2] outerClassName[A2] innerName[B2] access[1]
+ className[A2$B2$C2] outerClassName[A2$B2] innerName[C2] access[1]
diff --git a/test/files/jvm/t8582.scala b/test/files/jvm/t8582.scala
new file mode 100644
index 0000000000..8a57ef7952
--- /dev/null
+++ b/test/files/jvm/t8582.scala
@@ -0,0 +1,81 @@
+import scala.tools.partest.BytecodeTest
+import scala.collection.JavaConverters._
+
+package p1 {
+  package p2 {
+    object Singleton {
+      object Singleton {
+        object Singleton
+      }
+    }
+  }
+}
+
+class A1 {
+  class B1 {
+    @scala.beans.BeanInfo
+    class C1
+  }
+}
+
+class A2 {
+  class B2 {
+    class C2
+  }
+  def f: B2#C2 = null
+}
+
+
+object Test extends BytecodeTest {
+  import p1.p2._
+
+  def nested(c: Class[_]) = s" ${c.getName}: ${c.getDeclaredClasses.toList}"
+
+  def nprintln(s: String) = println("\n"+s)
+  def printInner(cname: String): Unit = {
+    val cnode = loadClassNode(cname)
+    println(cnode.innerClasses.asScala.toList.map(i => s"className[${i.name}] outerClassName[${i.outerName}] innerName[${i.innerName}] access[${i.access}]").mkString(" ", "\n ", ""))
+  }
+
+  def show() {
+
+    println("getClass on module gives module class")
+    println(" " + Singleton.Singleton.getClass)
+
+    nprintln("Nested module classes are found through reflection")
+    println(nested(Singleton.Singleton.getClass))
+
+    nprintln("Reflection can find direct nested classes (A1-B1-C1)")
+    println(nested(classOf[A1]))
+    println(nested(classOf[A1#B1]))
+    println(nested(classOf[A1#B1#C1]))
+
+    nprintln("Reflection can find direct nested classes (A2-B2-C2)")
+    println(nested(classOf[A2]))
+    println(nested(classOf[A2#B2]))
+    println(nested(classOf[A2#B2#C2]))
+
+    nprintln("Mirror classes have the same InnerClass attributes as the corresponding module class:")
+    printInner("p1.p2.Singleton") // mirror class
+    println("Module class")
+    printInner("p1.p2.Singleton$")
+
+    nprintln("An outer class has a InnerClass attribute for direct nested classes")
+    printInner("A1")
+    println("A nested class has an InnerClass attribute for itself (and also for its nested classes)")
+    printInner("A1$B1")
+    println("C1 is a nested class, so it has an InnerClass attribute for itself.\n"+
+            "Because that attribute leads to an entry for B1 in the constant pool, C1 needs an InnerClass attribute for B1.")
+    printInner("A1$B1$C1")
+
+    nprintln("The BeanInfo class has the same InnerClass attributes as the corresponding bean")
+    printInner("A1$B1$C1BeanInfo")
+
+    nprintln("Class A2 mentions class C2 in the constant pool (due to method  f), therefore it needs an InnerClass attribute for C1")
+    printInner("A2")
+    println("B2")
+    printInner("A2$B2")
+    println("C2")
+    printInner("A2$B2$C2")
+  }
+}
diff --git a/test/files/run/t8582.check b/test/files/run/t8582.check
deleted file mode 100644
index 7e96756986..0000000000
--- a/test/files/run/t8582.check
+++ /dev/null
@@ -1,2 +0,0 @@
-class p1.p2.Singleton$Singleton$
-List(class p1.p2.Singleton$Singleton$Singleton$)
diff --git a/test/files/run/t8582.scala b/test/files/run/t8582.scala
deleted file mode 100644
index 844ab8ac14..0000000000
--- a/test/files/run/t8582.scala
+++ /dev/null
@@ -1,16 +0,0 @@
-package p1 {
-  package p2 {
-    object Singleton {
-      object Singleton {
-        object Singleton
-      }
-    }
-  }
-}
-
-
-object Test extends App {
-  import p1.p2._
-  println(Singleton.Singleton.getClass)
-  println(Singleton.Singleton.getClass.getDeclaredClasses.toList)
-}
diff --git a/test/pending/run/t8582-bcode.check b/test/pending/run/t8582-bcode.check
deleted file mode 100644
index 7e96756986..0000000000
--- a/test/pending/run/t8582-bcode.check
+++ /dev/null
@@ -1,2 +0,0 @@
-class p1.p2.Singleton$Singleton$
-List(class p1.p2.Singleton$Singleton$Singleton$)
diff --git a/test/pending/run/t8582-bcode.flags b/test/pending/run/t8582-bcode.flags
deleted file mode 100644
index c30091d3de..0000000000
--- a/test/pending/run/t8582-bcode.flags
+++ /dev/null
@@ -1 +0,0 @@
--Ybackend:GenBCode
diff --git a/test/pending/run/t8582-bcode.scala b/test/pending/run/t8582-bcode.scala
deleted file mode 100644
index 844ab8ac14..0000000000
--- a/test/pending/run/t8582-bcode.scala
+++ /dev/null
@@ -1,16 +0,0 @@
-package p1 {
-  package p2 {
-    object Singleton {
-      object Singleton {
-        object Singleton
-      }
-    }
-  }
-}
-
-
-object Test extends App {
-  import p1.p2._
-  println(Singleton.Singleton.getClass)
-  println(Singleton.Singleton.getClass.getDeclaredClasses.toList)
-}