[JVM_IR] Limit inner class attributes to types in class file (#5081)

* [JVM_IR] Limit inner class attributes to types in class file

Inner class attributes should only be recorded for types that
are materialized in the result class file. In particular, we
should not emit inner classes attributes for types that appear
only in fake overrides. We do map these types to track the
fake overrides for JVM signature clashes but they are not
materialized in the class file.

^KT-56104 Fixed

* [JVM_IR] Consistently pass around materialized boolean in mapType.
This commit is contained in:
Mads Sig Ager
2023-01-30 14:11:07 +01:00
committed by GitHub
parent c9aeadd31f
commit 7c3cc1d5c1
8 changed files with 86 additions and 42 deletions
@@ -28,15 +28,16 @@ interface TypeMappingContext<Writer : JvmDescriptorTypeWriter<Type>> {
object AbstractTypeMapper { object AbstractTypeMapper {
fun <Writer : JvmDescriptorTypeWriter<Type>> mapClass( fun <Writer : JvmDescriptorTypeWriter<Type>> mapClass(
context: TypeMappingContext<Writer>, context: TypeMappingContext<Writer>,
typeConstructor: TypeConstructorMarker typeConstructor: TypeConstructorMarker,
materialized: Boolean
): Type { ): Type {
return with(context.typeContext) { return with(context.typeContext) {
when { when {
typeConstructor.isClassTypeConstructor() -> { typeConstructor.isClassTypeConstructor() -> {
mapType(context, typeConstructor.defaultType(), TypeMappingMode.CLASS_DECLARATION) mapType(context, typeConstructor.defaultType(), TypeMappingMode.CLASS_DECLARATION, materialized = materialized)
} }
typeConstructor.isTypeParameter() -> { typeConstructor.isTypeParameter() -> {
mapType(context, typeConstructor.defaultType()) mapType(context, typeConstructor.defaultType(), materialized = materialized)
} }
else -> error("Unknown type constructor: $typeConstructor") else -> error("Unknown type constructor: $typeConstructor")
} }
@@ -47,15 +48,17 @@ object AbstractTypeMapper {
context: TypeMappingContext<Writer>, context: TypeMappingContext<Writer>,
type: KotlinTypeMarker, type: KotlinTypeMarker,
mode: TypeMappingMode = TypeMappingMode.DEFAULT, mode: TypeMappingMode = TypeMappingMode.DEFAULT,
sw: Writer? = null sw: Writer? = null,
): Type = context.typeContext.mapType(context, type, mode, sw) materialized: Boolean = true,
): Type = context.typeContext.mapType(context, type, mode, sw, materialized)
// NB: The counterpart, [descriptorBasedTypeSignatureMapping#mapType] doesn't have restriction on [type]. // NB: The counterpart, [descriptorBasedTypeSignatureMapping#mapType] doesn't have restriction on [type].
private fun <Writer : JvmDescriptorTypeWriter<Type>> TypeSystemCommonBackendContextForTypeMapping.mapType( private fun <Writer : JvmDescriptorTypeWriter<Type>> TypeSystemCommonBackendContextForTypeMapping.mapType(
context: TypeMappingContext<Writer>, context: TypeMappingContext<Writer>,
type: KotlinTypeMarker, type: KotlinTypeMarker,
mode: TypeMappingMode = TypeMappingMode.DEFAULT, mode: TypeMappingMode,
sw: Writer? = null sw: Writer?,
materialized: Boolean,
): Type { ): Type {
if (type.isError()) { if (type.isError()) {
val name = type.getNameForErrorType() ?: NON_EXISTENT_CLASS_NAME val name = type.getNameForErrorType() ?: NON_EXISTENT_CLASS_NAME
@@ -75,15 +78,15 @@ object AbstractTypeMapper {
} }
if (type.isSuspendFunction()) { if (type.isSuspendFunction()) {
return mapSuspendFunctionType(type, context, mode, sw) return mapSuspendFunctionType(type, context, mode, sw, materialized)
} }
if (type.isArrayOrNullableArray()) { if (type.isArrayOrNullableArray()) {
return mapArrayType(type, sw, context, mode) return mapArrayType(type, sw, context, mode, materialized)
} }
if (typeConstructor.isClassTypeConstructor()) { if (typeConstructor.isClassTypeConstructor()) {
return mapClassType(typeConstructor, mode, type, context, sw) return mapClassType(typeConstructor, mode, type, context, sw, materialized)
} }
} }
@@ -97,17 +100,17 @@ object AbstractTypeMapper {
upperBound.makeNullable() upperBound.makeNullable()
else upperBound else upperBound
val asmType = mapType(context, newType, mode, null) val asmType = mapType(context, newType, mode, null, materialized)
sw?.writeTypeVariable(typeParameter.getName(), asmType) sw?.writeTypeVariable(typeParameter.getName(), asmType)
asmType asmType
} }
type.isFlexible() -> { type.isFlexible() -> {
mapType(context, type.upperBoundIfFlexible(), mode, sw) mapType(context, type.upperBoundIfFlexible(), mode, sw, materialized)
} }
type is DefinitelyNotNullTypeMarker -> type is DefinitelyNotNullTypeMarker ->
mapType(context, type.original(), mode, sw) mapType(context, type.original(), mode, sw, materialized)
typeConstructor.isScript() -> typeConstructor.isScript() ->
Type.getObjectType(context.getScriptInternalName(typeConstructor)).let { Type.getObjectType(context.getScriptInternalName(typeConstructor)).let {
@@ -124,7 +127,8 @@ object AbstractTypeMapper {
type: SimpleTypeMarker, type: SimpleTypeMarker,
context: TypeMappingContext<Writer>, context: TypeMappingContext<Writer>,
mode: TypeMappingMode, mode: TypeMappingMode,
sw: Writer? sw: Writer?,
materialized: Boolean,
): Type { ): Type {
val argumentsCount = type.argumentsCount() val argumentsCount = type.argumentsCount()
val argumentsList = type.asArgumentList() val argumentsList = type.asArgumentList()
@@ -136,14 +140,15 @@ object AbstractTypeMapper {
this += nullableAnyType() this += nullableAnyType()
} }
val runtimeFunctionType = functionNTypeConstructor(arguments.size - 1).typeWithArguments(arguments) val runtimeFunctionType = functionNTypeConstructor(arguments.size - 1).typeWithArguments(arguments)
return mapType(context, runtimeFunctionType, mode, sw) return mapType(context, runtimeFunctionType, mode, sw, materialized)
} }
private fun <Writer : JvmDescriptorTypeWriter<Type>> TypeSystemCommonBackendContextForTypeMapping.mapArrayType( private fun <Writer : JvmDescriptorTypeWriter<Type>> TypeSystemCommonBackendContextForTypeMapping.mapArrayType(
type: SimpleTypeMarker, type: SimpleTypeMarker,
sw: Writer?, sw: Writer?,
context: TypeMappingContext<Writer>, context: TypeMappingContext<Writer>,
mode: TypeMappingMode mode: TypeMappingMode,
materialized: Boolean
): Type { ): Type {
val typeArgument = type.asArgumentList()[0] val typeArgument = type.asArgumentList()[0]
val (variance, memberType) = when { val (variance, memberType) = when {
@@ -158,7 +163,7 @@ object AbstractTypeMapper {
arrayElementType = AsmTypes.OBJECT_TYPE arrayElementType = AsmTypes.OBJECT_TYPE
sw?.writeClass(arrayElementType) sw?.writeClass(arrayElementType)
} else { } else {
arrayElementType = mapType(context, memberType, mode.toGenericArgumentMode(variance, ofArray = true), sw) arrayElementType = mapType(context, memberType, mode.toGenericArgumentMode(variance, ofArray = true), sw, materialized)
} }
sw?.writeArrayEnd() sw?.writeArrayEnd()
return AsmUtil.getArrayType(arrayElementType) return AsmUtil.getArrayType(arrayElementType)
@@ -169,13 +174,14 @@ object AbstractTypeMapper {
mode: TypeMappingMode, mode: TypeMappingMode,
type: SimpleTypeMarker, type: SimpleTypeMarker,
context: TypeMappingContext<Writer>, context: TypeMappingContext<Writer>,
sw: Writer? sw: Writer?,
materialized: Boolean
): Type { ): Type {
if (typeConstructor.isInlineClass() && !mode.needInlineClassWrapping) { if (typeConstructor.isInlineClass() && !mode.needInlineClassWrapping) {
val expandedType = computeExpandedTypeForInlineClass(type) val expandedType = computeExpandedTypeForInlineClass(type)
require(expandedType is SimpleTypeMarker?) require(expandedType is SimpleTypeMarker?)
if (expandedType != null) { if (expandedType != null) {
return mapType(context, expandedType, mode.wrapInlineClassesMode(), sw) return mapType(context, expandedType, mode.wrapInlineClassesMode(), sw, materialized)
} }
} }
@@ -4136,6 +4136,12 @@ public class FirBytecodeTextTestGenerated extends AbstractFirBytecodeTextTest {
runTest("compiler/testData/codegen/bytecodeText/innerClasses/kt53804.kt"); runTest("compiler/testData/codegen/bytecodeText/innerClasses/kt53804.kt");
} }
@Test
@TestMetadata("kt56104.kt")
public void testKt56104() throws Exception {
runTest("compiler/testData/codegen/bytecodeText/innerClasses/kt56104.kt");
}
@Test @Test
@TestMetadata("nestedClassInAnnotationArgument.kt") @TestMetadata("nestedClassInAnnotationArgument.kt")
public void testNestedClassInAnnotationArgument() throws Exception { public void testNestedClassInAnnotationArgument() throws Exception {
@@ -86,13 +86,16 @@ class ClassCodegen private constructor(
if (context.state.oldInnerClassesLogic) if (context.state.oldInnerClassesLogic)
context.defaultTypeMapper context.defaultTypeMapper
else object : IrTypeMapper(context) { else object : IrTypeMapper(context) {
override fun mapType(type: IrType, mode: TypeMappingMode, sw: JvmSignatureWriter?): Type { override fun mapType(type: IrType, mode: TypeMappingMode, sw: JvmSignatureWriter?, materialized: Boolean): Type {
var t = type var t = type
while (t.isArray()) { while (t.isArray()) {
t = t.getArrayElementType(context.irBuiltIns) t = t.getArrayElementType(context.irBuiltIns)
} }
t.classOrNull?.owner?.let(::addInnerClassInfo) // Only record inner class info for types that are materialized in the class file.
return super.mapType(type, mode, sw) if (materialized) {
t.classOrNull?.owner?.let(::addInnerClassInfo)
}
return super.mapType(type, mode, sw, materialized)
} }
} }
@@ -42,7 +42,7 @@ class JvmSignatureClashDetector(
} }
private fun mapRawSignature(irFunction: IrFunction): RawSignature { private fun mapRawSignature(irFunction: IrFunction): RawSignature {
val jvmSignature = classCodegen.methodSignatureMapper.mapSignatureSkipGeneric(irFunction) val jvmSignature = classCodegen.methodSignatureMapper.mapFakeOverrideSignatureSkipGeneric(irFunction)
return RawSignature(jvmSignature.asmMethod.name, jvmSignature.asmMethod.descriptor, MemberKind.METHOD) return RawSignature(jvmSignature.asmMethod.name, jvmSignature.asmMethod.descriptor, MemberKind.METHOD)
} }
@@ -128,8 +128,9 @@ open class IrTypeMapper(private val context: JvmBackendContext) : KotlinTypeMapp
open fun mapType( open fun mapType(
type: IrType, type: IrType,
mode: TypeMappingMode = TypeMappingMode.DEFAULT, mode: TypeMappingMode = TypeMappingMode.DEFAULT,
sw: JvmSignatureWriter? = null sw: JvmSignatureWriter? = null,
): Type = AbstractTypeMapper.mapType(this, type, mode, sw) materialized: Boolean = true
): Type = AbstractTypeMapper.mapType(this, type, mode, sw, materialized)
override fun JvmSignatureWriter.writeGenericType(type: KotlinTypeMarker, asmType: Type, mode: TypeMappingMode) { override fun JvmSignatureWriter.writeGenericType(type: KotlinTypeMarker, asmType: Type, mode: TypeMappingMode) {
if (type is IrErrorType) { if (type is IrErrorType) {
@@ -151,10 +151,10 @@ class MethodSignatureMapper(private val context: JvmBackendContext, private val
private fun IrSimpleFunction.isPublishedApi(): Boolean = private fun IrSimpleFunction.isPublishedApi(): Boolean =
propertyIfAccessor.annotations.hasAnnotation(StandardNames.FqNames.publishedApi) propertyIfAccessor.annotations.hasAnnotation(StandardNames.FqNames.publishedApi)
fun mapReturnType(declaration: IrDeclaration, sw: JvmSignatureWriter? = null): Type { fun mapReturnType(declaration: IrDeclaration, sw: JvmSignatureWriter? = null, materialized: Boolean = true): Type {
if (declaration !is IrFunction) { if (declaration !is IrFunction) {
require(declaration is IrField) { "Unsupported declaration: $declaration" } require(declaration is IrField) { "Unsupported declaration: $declaration" }
return mapReturnType(declaration, declaration.type, sw) return mapReturnType(declaration, declaration.type, sw, materialized)
} }
return when { return when {
@@ -163,16 +163,16 @@ class MethodSignatureMapper(private val context: JvmBackendContext, private val
Type.VOID_TYPE Type.VOID_TYPE
} }
forceBoxedReturnType(declaration) -> { forceBoxedReturnType(declaration) -> {
typeMapper.mapType(declaration.returnType, TypeMappingMode.RETURN_TYPE_BOXED, sw) typeMapper.mapType(declaration.returnType, TypeMappingMode.RETURN_TYPE_BOXED, sw, materialized)
} }
else -> mapReturnType(declaration, declaration.returnType, sw) else -> mapReturnType(declaration, declaration.returnType, sw, materialized)
} }
} }
private fun mapReturnType(declaration: IrDeclaration, returnType: IrType, sw: JvmSignatureWriter?): Type { private fun mapReturnType(declaration: IrDeclaration, returnType: IrType, sw: JvmSignatureWriter?, materialized: Boolean = true): Type {
val isAnnotationMethod = declaration.parent.let { it is IrClass && it.isAnnotationClass } val isAnnotationMethod = declaration.parent.let { it is IrClass && it.isAnnotationClass }
if (sw == null || sw.skipGenericSignature()) { if (sw == null || sw.skipGenericSignature()) {
return typeMapper.mapType(returnType, TypeMappingMode.getModeForReturnTypeNoGeneric(isAnnotationMethod), sw) return typeMapper.mapType(returnType, TypeMappingMode.getModeForReturnTypeNoGeneric(isAnnotationMethod), sw, materialized)
} }
val typeMappingModeFromAnnotation = val typeMappingModeFromAnnotation =
@@ -180,12 +180,12 @@ class MethodSignatureMapper(private val context: JvmBackendContext, private val
declaration.suppressWildcardsMode(), returnType, isAnnotationMethod, mapTypeAliases = false declaration.suppressWildcardsMode(), returnType, isAnnotationMethod, mapTypeAliases = false
) )
if (typeMappingModeFromAnnotation != null) { if (typeMappingModeFromAnnotation != null) {
return typeMapper.mapType(returnType, typeMappingModeFromAnnotation, sw) return typeMapper.mapType(returnType, typeMappingModeFromAnnotation, sw, materialized)
} }
val mappingMode = typeSystem.getOptimalModeForReturnType(returnType, isAnnotationMethod) val mappingMode = typeSystem.getOptimalModeForReturnType(returnType, isAnnotationMethod)
return typeMapper.mapType(returnType, mappingMode, sw) return typeMapper.mapType(returnType, mappingMode, sw, materialized)
} }
private fun hasVoidReturnType(function: IrFunction): Boolean = private fun hasVoidReturnType(function: IrFunction): Boolean =
@@ -214,13 +214,21 @@ class MethodSignatureMapper(private val context: JvmBackendContext, private val
function.origin == JvmLoweredDeclarationOrigin.SYNTHETIC_INLINE_CLASS_MEMBER && function.origin == JvmLoweredDeclarationOrigin.SYNTHETIC_INLINE_CLASS_MEMBER &&
function.name.asString() == "box-impl" function.name.asString() == "box-impl"
fun mapFakeOverrideSignatureSkipGeneric(function: IrFunction): JvmMethodSignature =
mapSignature(function, skipGenericSignature = true, materialized = false)
fun mapSignatureSkipGeneric(function: IrFunction): JvmMethodSignature = fun mapSignatureSkipGeneric(function: IrFunction): JvmMethodSignature =
mapSignature(function, true) mapSignature(function, true)
fun mapSignatureWithGeneric(function: IrFunction): JvmMethodGenericSignature = fun mapSignatureWithGeneric(function: IrFunction): JvmMethodGenericSignature =
mapSignature(function, false) mapSignature(function, false)
private fun mapSignature(function: IrFunction, skipGenericSignature: Boolean, skipSpecial: Boolean = false): JvmMethodGenericSignature { private fun mapSignature(
function: IrFunction,
skipGenericSignature: Boolean,
skipSpecial: Boolean = false,
materialized: Boolean = true
): JvmMethodGenericSignature {
if (function is IrLazyFunctionBase && if (function is IrLazyFunctionBase &&
(!function.isFakeOverride || function.parentAsClass.isFromJava()) && (!function.isFakeOverride || function.parentAsClass.isFromJava()) &&
function.initialSignatureFunction != null function.initialSignatureFunction != null
@@ -261,11 +269,11 @@ class MethodSignatureMapper(private val context: JvmBackendContext, private val
if (shouldBoxSingleValueParameterForSpecialCaseOfRemove(function)) if (shouldBoxSingleValueParameterForSpecialCaseOfRemove(function))
parameter.type.makeNullable() parameter.type.makeNullable()
else parameter.type else parameter.type
writeParameter(sw, kind, type, function) writeParameter(sw, kind, type, function, materialized)
} }
sw.writeReturnType() sw.writeReturnType()
mapReturnType(function, sw) mapReturnType(function, sw, materialized)
sw.writeReturnTypeEnd() sw.writeReturnTypeEnd()
val signature = sw.makeJvmMethodSignature(mapFunctionName(function, skipSpecial)) val signature = sw.makeJvmMethodSignature(mapFunctionName(function, skipSpecial))
@@ -327,19 +335,20 @@ class MethodSignatureMapper(private val context: JvmBackendContext, private val
sw: JvmSignatureWriter, sw: JvmSignatureWriter,
kind: JvmMethodParameterKind, kind: JvmMethodParameterKind,
type: IrType, type: IrType,
function: IrFunction function: IrFunction,
materialized: Boolean = true
) { ) {
sw.writeParameterType(kind) sw.writeParameterType(kind)
writeParameterType(sw, type, function) writeParameterType(sw, type, function, materialized)
sw.writeParameterTypeEnd() sw.writeParameterTypeEnd()
} }
private fun writeParameterType(sw: JvmSignatureWriter, type: IrType, declaration: IrDeclaration) { private fun writeParameterType(sw: JvmSignatureWriter, type: IrType, declaration: IrDeclaration, materialized: Boolean = true) {
if (sw.skipGenericSignature()) { if (sw.skipGenericSignature()) {
if (type.isInlineClassType() && declaration.isFromJava()) { if (type.isInlineClassType() && declaration.isFromJava()) {
typeMapper.mapType(type, TypeMappingMode.GENERIC_ARGUMENT, sw) typeMapper.mapType(type, TypeMappingMode.GENERIC_ARGUMENT, sw, materialized)
} else { } else {
typeMapper.mapType(type, TypeMappingMode.DEFAULT, sw) typeMapper.mapType(type, TypeMappingMode.DEFAULT, sw, materialized)
} }
return return
} }
@@ -355,7 +364,7 @@ class MethodSignatureMapper(private val context: JvmBackendContext, private val
} }
} }
typeMapper.mapType(type, mode, sw) typeMapper.mapType(type, mode, sw, materialized)
} }
private val IrDeclaration.isMethodWithDeclarationSiteWildcards: Boolean private val IrDeclaration.isMethodWithDeclarationSiteWildcards: Boolean
@@ -0,0 +1,13 @@
// TARGET_BACKEND: JVM_IR
// FILE: classes.kt
open class A {
class Inner
fun foo(i: Inner): Inner = Inner()
}
class B: A()
// A and A$Inner both need an inner class attribute for the relationship. B does not.
// 2 INNERCLASS A\$Inner A Inner
@@ -4136,6 +4136,12 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest {
runTest("compiler/testData/codegen/bytecodeText/innerClasses/kt53804.kt"); runTest("compiler/testData/codegen/bytecodeText/innerClasses/kt53804.kt");
} }
@Test
@TestMetadata("kt56104.kt")
public void testKt56104() throws Exception {
runTest("compiler/testData/codegen/bytecodeText/innerClasses/kt56104.kt");
}
@Test @Test
@TestMetadata("nestedClassInAnnotationArgument.kt") @TestMetadata("nestedClassInAnnotationArgument.kt")
public void testNestedClassInAnnotationArgument() throws Exception { public void testNestedClassInAnnotationArgument() throws Exception {