Skip to content

Simplify and correctify calculation of the InnerClass attribute #15153

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 18, 2022
Merged

Simplify and correctify calculation of the InnerClass attribute #15153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
97ec2cb
Clean up CoreBTypes, consistent names and remove unused entries
WojciechMazur May 5, 2022
0aa184d
Simplify and correctify calculation of the InnerClass attribute
WojciechMazur May 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 27 additions & 50 deletions compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala
View file
Open in desktop
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
Open in desktop
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
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