Merge pull request #15153 from WojciechMazur/backport/scala-pr-4807

Simplify and correctify calculation of the InnerClass attribute

Author: Kordyjan

compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala

@@ -39,7 +39,6 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
   import DottyBackendInterface.symExtensions
   import bTypes._
   import coreBTypes._
-  import BCodeBodyBuilder._
 
   protected val primitives: DottyPrimitives
 
@@ -431,7 +430,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
           if (value.tag != UnitTag) (value.tag, expectedType) match {
             case (IntTag,   LONG  ) => bc.lconst(value.longValue);       generatedType = LONG
             case (FloatTag, DOUBLE) => bc.dconst(value.doubleValue);     generatedType = DOUBLE
-            case (NullTag,  _     ) => bc.emit(asm.Opcodes.ACONST_NULL); generatedType = RT_NULL
+            case (NullTag,  _     ) => bc.emit(asm.Opcodes.ACONST_NULL); generatedType = srNullRef
             case _                  => genConstant(value);               generatedType = tpeTK(tree)
           }
 
@@ -490,7 +489,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
           val thrownType = expectedType
           // `throw null` is valid although scala.Null (as defined in src/libray-aux) isn't a subtype of Throwable.
           // Similarly for scala.Nothing (again, as defined in src/libray-aux).
-          assert(thrownType.isNullType || thrownType.isNothingType || thrownType.asClassBType.isSubtypeOf(ThrowableReference))
+          assert(thrownType.isNullType || thrownType.isNothingType || thrownType.asClassBType.isSubtypeOf(jlThrowableRef))
           emit(asm.Opcodes.ATHROW)
     end genAdaptAndSendToDest
 
@@ -674,8 +673,8 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
         else if (l.isPrimitive) {
           bc drop l
           if (cast) {
-            mnode.visitTypeInsn(asm.Opcodes.NEW, classCastExceptionReference.internalName)
-            bc dup ObjectReference
+            mnode.visitTypeInsn(asm.Opcodes.NEW, jlClassCastExceptionRef.internalName)
+            bc dup ObjectRef
             emit(asm.Opcodes.ATHROW)
           } else {
             bc boolconst false
@@ -777,15 +776,15 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
           val nativeKind = tpeTK(expr)
           genLoad(expr, nativeKind)
           val MethodNameAndType(mname, methodType) = asmBoxTo(nativeKind)
-          bc.invokestatic(BoxesRunTime.internalName, mname, methodType.descriptor, itf = false)
+          bc.invokestatic(srBoxesRuntimeRef.internalName, mname, methodType.descriptor, itf = false)
           generatedType = boxResultType(fun.symbol) // was toTypeKind(fun.symbol.tpe.resultType)
 
         case Apply(fun, List(expr)) if Erasure.Boxing.isUnbox(fun.symbol) && fun.symbol.denot.owner != defn.UnitModuleClass =>
           genLoad(expr)
           val boxType = unboxResultType(fun.symbol) // was toTypeKind(fun.symbol.owner.linkedClassOfClass.tpe)
           generatedType = boxType
           val MethodNameAndType(mname, methodType) = asmUnboxTo(boxType)
-          bc.invokestatic(BoxesRunTime.internalName, mname, methodType.descriptor, itf = false)
+          bc.invokestatic(srBoxesRuntimeRef.internalName, mname, methodType.descriptor, itf = false)
 
         case app @ Apply(fun, args) =>
           val sym = fun.symbol
@@ -1237,7 +1236,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
       liftStringConcat(tree) match {
         // Optimization for expressions of the form "" + x
         case List(Literal(Constant("")), arg) =>
-          genLoad(arg, ObjectReference)
+          genLoad(arg, ObjectRef)
           genCallMethod(defn.String_valueOf_Object, InvokeStyle.Static)
 
         case concatenations =>
@@ -1409,7 +1408,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
 
     /* Generate the scala ## method. */
     def genScalaHash(tree: Tree): BType = {
-      genLoad(tree, ObjectReference)
+      genLoad(tree, ObjectRef)
       genCallMethod(NoSymbol, InvokeStyle.Static) // used to dispatch ## on primitives to ScalaRuntime.hash. Should be implemented by a miniphase
     }
 
@@ -1508,8 +1507,8 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
         val nonNullSide = if (ScalaPrimitivesOps.isReferenceEqualityOp(code)) ifOneIsNull(l, r) else null
         if (nonNullSide != null) {
           // special-case reference (in)equality test for null (null eq x, x eq null)
-          genLoad(nonNullSide, ObjectReference)
-          genCZJUMP(success, failure, op, ObjectReference, targetIfNoJump)
+          genLoad(nonNullSide, ObjectRef)
+          genCZJUMP(success, failure, op, ObjectRef, targetIfNoJump)
         } else {
           val tk = tpeTK(l).maxType(tpeTK(r))
           genLoad(l, tk)
@@ -1627,42 +1626,42 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
           } else defn.BoxesRunTimeModule_externalEquals
         }
 
-        genLoad(l, ObjectReference)
-        genLoad(r, ObjectReference)
+        genLoad(l, ObjectRef)
+        genLoad(r, ObjectRef)
         genCallMethod(equalsMethod, InvokeStyle.Static)
         genCZJUMP(success, failure, Primitives.NE, BOOL, targetIfNoJump)
       }
       else {
         if (isNull(l)) {
           // null == expr -> expr eq null
-          genLoad(r, ObjectReference)
-          genCZJUMP(success, failure, Primitives.EQ, ObjectReference, targetIfNoJump)
+          genLoad(r, ObjectRef)
+          genCZJUMP(success, failure, Primitives.EQ, ObjectRef, targetIfNoJump)
         } else if (isNull(r)) {
           // expr == null -> expr eq null
-          genLoad(l, ObjectReference)
-          genCZJUMP(success, failure, Primitives.EQ, ObjectReference, targetIfNoJump)
+          genLoad(l, ObjectRef)
+          genCZJUMP(success, failure, Primitives.EQ, ObjectRef, targetIfNoJump)
         } else if (isNonNullExpr(l)) {
           // SI-7852 Avoid null check if L is statically non-null.
-          genLoad(l, ObjectReference)
-          genLoad(r, ObjectReference)
+          genLoad(l, ObjectRef)
+          genLoad(r, ObjectRef)
           genCallMethod(defn.Any_equals, InvokeStyle.Virtual)
           genCZJUMP(success, failure, Primitives.NE, BOOL, targetIfNoJump)
         } else {
           // l == r -> if (l eq null) r eq null else l.equals(r)
-          val eqEqTempLocal = locals.makeLocal(ObjectReference, nme.EQEQ_LOCAL_VAR.mangledString, defn.ObjectType, r.span)
+          val eqEqTempLocal = locals.makeLocal(ObjectRef, nme.EQEQ_LOCAL_VAR.mangledString, defn.ObjectType, r.span)
           val lNull    = new asm.Label
           val lNonNull = new asm.Label
 
-          genLoad(l, ObjectReference)
-          genLoad(r, ObjectReference)
+          genLoad(l, ObjectRef)
+          genLoad(r, ObjectRef)
           locals.store(eqEqTempLocal)
-          bc dup ObjectReference
-          genCZJUMP(lNull, lNonNull, Primitives.EQ, ObjectReference, targetIfNoJump = lNull)
+          bc dup ObjectRef
+          genCZJUMP(lNull, lNonNull, Primitives.EQ, ObjectRef, targetIfNoJump = lNull)
 
           markProgramPoint(lNull)
-          bc drop ObjectReference
+          bc drop ObjectRef
           locals.load(eqEqTempLocal)
-          genCZJUMP(success, failure, Primitives.EQ, ObjectReference, targetIfNoJump = lNonNull)
+          genCZJUMP(success, failure, Primitives.EQ, ObjectRef, targetIfNoJump = lNonNull)
 
           markProgramPoint(lNonNull)
           locals.load(eqEqTempLocal)
@@ -1753,9 +1752,9 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
 
       val metafactory =
         if (flags != 0)
-          lambdaMetaFactoryAltMetafactoryHandle // altMetafactory required to be able to pass the flags and additional arguments if needed
+          jliLambdaMetaFactoryAltMetafactoryHandle // altMetafactory required to be able to pass the flags and additional arguments if needed
         else
-          lambdaMetaFactoryMetafactoryHandle
+          jliLambdaMetaFactoryMetafactoryHandle
 
       bc.jmethod.visitInvokeDynamicInsn(methodName, desc, metafactory, bsmArgs: _*)
 
@@ -1772,27 +1771,5 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
   private def isEmittedInterface(sym: Symbol): Boolean = sym.isInterface ||
     sym.is(JavaDefined) && (toDenot(sym).isAnnotation || sym.is(ModuleClass) && (sym.companionClass.is(PureInterface)) || sym.companionClass.is(Trait))
 
-}
 
-object BCodeBodyBuilder {
-  val lambdaMetaFactoryMetafactoryHandle = new Handle(
-    Opcodes.H_INVOKESTATIC,
-    "java/lang/invoke/LambdaMetafactory",
-    "metafactory",
-    "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;",
-    /* itf = */ false)
-
-  val lambdaMetaFactoryAltMetafactoryHandle = new Handle(
-    Opcodes.H_INVOKESTATIC,
-    "java/lang/invoke/LambdaMetafactory",
-    "altMetafactory",
-    "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite;",
-    /* itf = */ false)
-
-  val lambdaDeserializeBootstrapHandle = new Handle(
-    Opcodes.H_INVOKESTATIC,
-    "scala/runtime/LambdaDeserialize",
-    "bootstrap",
-    "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;[Ljava/lang/invoke/MethodHandle;)Ljava/lang/invoke/CallSite;",
-    /* itf = */ false)
 }

compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala

@@ -132,24 +132,24 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
   }
 
   /*
-   * Populates the InnerClasses JVM attribute with `refedInnerClasses`.
-   * In addition to inner classes mentioned somewhere in `jclass` (where `jclass` is a class file being emitted)
-   * `refedInnerClasses` should contain those inner classes defined as direct member classes of `jclass`
-   * but otherwise not mentioned in `jclass`.
+   * Populates the InnerClasses JVM attribute with `refedInnerClasses`. See also the doc on inner
+   * classes in BTypes.scala.
    *
-   * `refedInnerClasses` may contain duplicates,
-   * need not contain the enclosing inner classes of each inner class it lists (those are looked up for consistency).
+   * `refedInnerClasses` may contain duplicates, need not contain the enclosing inner classes of
+   * each inner class it lists (those are looked up and included).
    *
-   * This method serializes in the InnerClasses JVM attribute in an appropriate order,
+   * This method serializes in the InnerClasses JVM attribute in an appropriate order, 
    * not necessarily that given by `refedInnerClasses`.
    *
    * can-multi-thread
    */
-  final def addInnerClassesASM(jclass: asm.ClassVisitor, refedInnerClasses: List[ClassBType]): Unit = {
-    val allNestedClasses = refedInnerClasses.flatMap(_.enclosingNestedClassesChain).distinct
-
+  final def addInnerClasses(jclass: asm.ClassVisitor, declaredInnerClasses: List[ClassBType], refedInnerClasses: List[ClassBType]): Unit = {
     // sorting ensures nested classes are listed after their enclosing class thus satisfying the Eclipse Java compiler
-    for (nestedClass <- allNestedClasses.sortBy(_.internalName.toString)) {
+    val allNestedClasses = new mutable.TreeSet[ClassBType]()(Ordering.by(_.internalName))
+    allNestedClasses ++= declaredInnerClasses
+    refedInnerClasses.foreach(allNestedClasses ++= _.enclosingNestedClassesChain)
+    for nestedClass <- allNestedClasses
+    do {
       // Extract the innerClassEntry - we know it exists, enclosingNestedClassesChain only returns nested classes.
       val Some(e) = nestedClass.innerClassAttributeEntry: @unchecked
       jclass.visitInnerClass(e.name, e.outerName, e.innerName, e.flags)
@@ -218,26 +218,16 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
     final val emitLines  = debugLevel >= 2
     final val emitVars   = debugLevel >= 3
 
-    /*
-     *  Contains class-symbols that:
-     *    (a) are known to denote inner classes
-     *    (b) are mentioned somewhere in the class being generated.
-     *
-     *  In other words, the lifetime of `innerClassBufferASM` is associated to "the class being generated".
-     */
-    final val innerClassBufferASM = mutable.Set.empty[ClassBType]
-
     /**
-     * The class internal name for a given class symbol. If the symbol describes a nested class, the
-     * ClassBType is added to the innerClassBufferASM.
+     * The class internal name for a given class symbol.
      */
     final def internalName(sym: Symbol): String = {
       // For each java class, the scala compiler creates a class and a module (thus a module class).
-      // If the `sym` is a java module class, we use the java class instead. This ensures that we
-      // register the class (instead of the module class) in innerClassBufferASM.
+      // If the `sym` is a java module class, we use the java class instead. This ensures that the
+      // ClassBType is created from the main class (instead of the module class).
       // The two symbols have the same name, so the resulting internalName is the same.
       val classSym = if (sym.is(JavaDefined) && sym.is(ModuleClass)) sym.linkedClass else sym
-      getClassBTypeAndRegisterInnerClass(classSym).internalName
+      getClassBType(classSym).internalName
     }
 
     private def assertClassNotArray(sym: Symbol): Unit = {
@@ -251,8 +241,7 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
     }
 
     /**
-     * The ClassBType for a class symbol. If the class is nested, the ClassBType is added to the
-     * innerClassBufferASM.
+     * The ClassBType for a class symbol.
      *
      * The class symbol scala.Nothing is mapped to the class scala.runtime.Nothing$. Similarly,
      * scala.Null is mapped to scala.runtime.Null$. This is because there exist no class files
@@ -264,16 +253,12 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
      * the class descriptor of the receiver (the implementation class) is obtained by creating the
      * ClassBType.
      */
-    final def getClassBTypeAndRegisterInnerClass(sym: Symbol): ClassBType = {
+    final def getClassBType(sym: Symbol): ClassBType = {
       assertClassNotArrayNotPrimitive(sym)
 
-      if (sym == defn.NothingClass) RT_NOTHING
-      else if (sym == defn.NullClass) RT_NULL
-      else {
-        val r = classBTypeFromSymbol(sym)
-        if (r.isNestedClass) innerClassBufferASM += r
-        r
-      }
+      if (sym == defn.NothingClass) srNothingRef
+      else if (sym == defn.NullClass) srNullRef
+      else classBTypeFromSymbol(sym)
     }
 
     /*
@@ -288,16 +273,14 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
     }
 
     /**
-     * The jvm descriptor of a type. If `t` references a nested class, its ClassBType is added to
-     * the innerClassBufferASM.
+     * The jvm descriptor of a type.
      */
     final def typeDescriptor(t: Type): String = { toTypeKind(t).descriptor   }
 
     /**
-     * The jvm descriptor for a symbol. If `sym` represents a nested class, its ClassBType is added
-     * to the innerClassBufferASM.
+     * The jvm descriptor for a symbol.
      */
-    final def symDescriptor(sym: Symbol): String = { getClassBTypeAndRegisterInnerClass(sym).descriptor }
+    final def symDescriptor(sym: Symbol): String = getClassBType(sym).descriptor
 
     final def toTypeKind(tp: Type): BType = typeToTypeKind(tp)(BCodeHelpers.this)(this)
 
@@ -691,19 +674,18 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
     def genMirrorClass(moduleClass: Symbol, cunit: CompilationUnit): asm.tree.ClassNode = {
       assert(moduleClass.is(ModuleClass))
       assert(moduleClass.companionClass == NoSymbol, moduleClass)
-      innerClassBufferASM.clear()
       this.cunit = cunit
+      val bType      = mirrorClassBTypeFromSymbol(moduleClass)
       val moduleName = internalName(moduleClass) // + "$"
-      val mirrorName = moduleName.substring(0, moduleName.length() - 1)
+      val mirrorName = bType.internalName
 
-      val flags = (asm.Opcodes.ACC_SUPER | asm.Opcodes.ACC_PUBLIC | asm.Opcodes.ACC_FINAL)
       val mirrorClass = new asm.tree.ClassNode
       mirrorClass.visit(
         classfileVersion,
-        flags,
+        bType.info.flags,
         mirrorName,
         null /* no java-generic-signature */,
-        ObjectReference.internalName,
+        ObjectRef.internalName,
         EMPTY_STRING_ARRAY
       )
 
@@ -717,10 +699,6 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
       emitAnnotations(mirrorClass, moduleClass.annotations ++ ssa)
 
       addForwarders(mirrorClass, mirrorName, moduleClass)
-
-      innerClassBufferASM ++= classBTypeFromSymbol(moduleClass).info.memberClasses
-      addInnerClassesASM(mirrorClass, innerClassBufferASM.toList)
-
       mirrorClass.visitEnd()
 
       moduleClass.name // this side-effect is necessary, really.
@@ -754,8 +732,7 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
      * must-single-thread
      */
     def legacyAddCreatorCode(clinit: asm.MethodVisitor, cnode: asm.tree.ClassNode, thisName: String): Unit = {
-      // this tracks the inner class in innerClassBufferASM, if needed.
-      val androidCreatorType = getClassBTypeAndRegisterInnerClass(AndroidCreatorClass)
+      val androidCreatorType = getClassBType(AndroidCreatorClass)
       val tdesc_creator = androidCreatorType.descriptor
 
       cnode.visitField(
@@ -818,8 +795,7 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
     def primitiveOrClassToBType(sym: Symbol): BType = {
       assert(sym.isClass, sym)
       assert(sym != defn.ArrayClass || compilingArray, sym)
-      primitiveTypeMap.getOrElse(sym,
-        storage.getClassBTypeAndRegisterInnerClass(sym)).asInstanceOf[BType]
+      primitiveTypeMap.getOrElse(sym, storage.getClassBType(sym)).asInstanceOf[BType]
     }
 
     /**
@@ -828,7 +804,7 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
       */
     def nonClassTypeRefToBType(sym: Symbol): ClassBType = {
       assert(sym.isType && compilingArray, sym)
-      ObjectReference.asInstanceOf[ct.bTypes.ClassBType]
+      ObjectRef.asInstanceOf[ct.bTypes.ClassBType]
     }
 
     tp.widenDealias match {
@@ -861,8 +837,8 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
             "If possible, please file a bug on https://github.com/lampepfl/dotty/issues")
 
         tp match {
-          case tp: ThisType if tp.cls == defn.ArrayClass => ObjectReference.asInstanceOf[ct.bTypes.ClassBType] // was introduced in 9b17332f11 to fix SI-999, but this code is not reached in its test, or any other test
-          case tp: ThisType                         => storage.getClassBTypeAndRegisterInnerClass(tp.cls)
+          case tp: ThisType if tp.cls == defn.ArrayClass => ObjectRef.asInstanceOf[ct.bTypes.ClassBType] // was introduced in 9b17332f11 to fix SI-999, but this code is not reached in its test, or any other test
+          case tp: ThisType                         => storage.getClassBType(tp.cls)
           // case t: SingletonType                   => primitiveOrClassToBType(t.classSymbol)
           case t: SingletonType                     => typeToTypeKind(t.underlying)(ct)(storage)
           case t: RefinedType                       => typeToTypeKind(t.parent)(ct)(storage) //parents.map(_.toTypeKind(ct)(storage).asClassBType).reduceLeft((a, b) => a.jvmWiseLUB(b))