Skip to content

Commit

Permalink
Merge pull request scala#15153 from WojciechMazur/backport/scala-pr-4807
Browse files Browse the repository at this point in the history 
Simplify and correctify calculation of the InnerClass attribute
  • Loading branch information
@Kordyjan
Kordyjan authored May 18, 2022
2 parents 11e4b30 + 0aa184d commit 72352dc
Show file tree
Hide file tree
Showing 10 changed files with 557 additions and 235 deletions.
77 changes: 27 additions & 50 deletions compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
import DottyBackendInterface.symExtensions
import bTypes._
import coreBTypes._
import BCodeBodyBuilder._

protected val primitives: DottyPrimitives

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 =>
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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: _*)

Expand All @@ -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)
}
88 changes: 32 additions & 56 deletions compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 = {
Expand All @@ -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
Expand All @@ -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)
}

/*
Expand All @@ -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)

Expand Down Expand Up @@ -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
)

Expand All @@ -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.
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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]
}

/**
Expand All @@ -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 {
Expand Down Expand Up @@ -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))
Expand Down
Loading

0 comments on commit 72352dc

Please sign in to comment.