From 7c3cc1d5c1d501ab5f9d7aadf462aaa68781fb89 Mon Sep 17 00:00:00 2001 From: Mads Sig Ager Date: Mon, 30 Jan 2023 14:11:07 +0100 Subject: [PATCH] [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. --- .../kotlin/types/AbstractTypeMapper.kt | 44 +++++++++++-------- .../codegen/FirBytecodeTextTestGenerated.java | 6 +++ .../backend/jvm/codegen/ClassCodegen.kt | 9 ++-- .../jvm/codegen/JvmSignatureClashDetector.kt | 2 +- .../backend/jvm/mapping/IrTypeMapper.kt | 5 ++- .../jvm/mapping/MethodSignatureMapper.kt | 43 +++++++++++------- .../bytecodeText/innerClasses/kt56104.kt | 13 ++++++ .../codegen/IrBytecodeTextTestGenerated.java | 6 +++ 8 files changed, 86 insertions(+), 42 deletions(-) create mode 100644 compiler/testData/codegen/bytecodeText/innerClasses/kt56104.kt diff --git a/compiler/backend.common.jvm/src/org/jetbrains/kotlin/types/AbstractTypeMapper.kt b/compiler/backend.common.jvm/src/org/jetbrains/kotlin/types/AbstractTypeMapper.kt index 6826ab25f58..f51bfda654d 100644 --- a/compiler/backend.common.jvm/src/org/jetbrains/kotlin/types/AbstractTypeMapper.kt +++ b/compiler/backend.common.jvm/src/org/jetbrains/kotlin/types/AbstractTypeMapper.kt @@ -28,15 +28,16 @@ interface TypeMappingContext> { object AbstractTypeMapper { fun > mapClass( context: TypeMappingContext, - typeConstructor: TypeConstructorMarker + typeConstructor: TypeConstructorMarker, + materialized: Boolean ): Type { return with(context.typeContext) { when { typeConstructor.isClassTypeConstructor() -> { - mapType(context, typeConstructor.defaultType(), TypeMappingMode.CLASS_DECLARATION) + mapType(context, typeConstructor.defaultType(), TypeMappingMode.CLASS_DECLARATION, materialized = materialized) } typeConstructor.isTypeParameter() -> { - mapType(context, typeConstructor.defaultType()) + mapType(context, typeConstructor.defaultType(), materialized = materialized) } else -> error("Unknown type constructor: $typeConstructor") } @@ -47,15 +48,17 @@ object AbstractTypeMapper { context: TypeMappingContext, type: KotlinTypeMarker, mode: TypeMappingMode = TypeMappingMode.DEFAULT, - sw: Writer? = null - ): Type = context.typeContext.mapType(context, type, mode, sw) + sw: Writer? = null, + materialized: Boolean = true, + ): Type = context.typeContext.mapType(context, type, mode, sw, materialized) // NB: The counterpart, [descriptorBasedTypeSignatureMapping#mapType] doesn't have restriction on [type]. private fun > TypeSystemCommonBackendContextForTypeMapping.mapType( context: TypeMappingContext, type: KotlinTypeMarker, - mode: TypeMappingMode = TypeMappingMode.DEFAULT, - sw: Writer? = null + mode: TypeMappingMode, + sw: Writer?, + materialized: Boolean, ): Type { if (type.isError()) { val name = type.getNameForErrorType() ?: NON_EXISTENT_CLASS_NAME @@ -75,15 +78,15 @@ object AbstractTypeMapper { } if (type.isSuspendFunction()) { - return mapSuspendFunctionType(type, context, mode, sw) + return mapSuspendFunctionType(type, context, mode, sw, materialized) } if (type.isArrayOrNullableArray()) { - return mapArrayType(type, sw, context, mode) + return mapArrayType(type, sw, context, mode, materialized) } 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() else upperBound - val asmType = mapType(context, newType, mode, null) + val asmType = mapType(context, newType, mode, null, materialized) sw?.writeTypeVariable(typeParameter.getName(), asmType) asmType } type.isFlexible() -> { - mapType(context, type.upperBoundIfFlexible(), mode, sw) + mapType(context, type.upperBoundIfFlexible(), mode, sw, materialized) } type is DefinitelyNotNullTypeMarker -> - mapType(context, type.original(), mode, sw) + mapType(context, type.original(), mode, sw, materialized) typeConstructor.isScript() -> Type.getObjectType(context.getScriptInternalName(typeConstructor)).let { @@ -124,7 +127,8 @@ object AbstractTypeMapper { type: SimpleTypeMarker, context: TypeMappingContext, mode: TypeMappingMode, - sw: Writer? + sw: Writer?, + materialized: Boolean, ): Type { val argumentsCount = type.argumentsCount() val argumentsList = type.asArgumentList() @@ -136,14 +140,15 @@ object AbstractTypeMapper { this += nullableAnyType() } val runtimeFunctionType = functionNTypeConstructor(arguments.size - 1).typeWithArguments(arguments) - return mapType(context, runtimeFunctionType, mode, sw) + return mapType(context, runtimeFunctionType, mode, sw, materialized) } private fun > TypeSystemCommonBackendContextForTypeMapping.mapArrayType( type: SimpleTypeMarker, sw: Writer?, context: TypeMappingContext, - mode: TypeMappingMode + mode: TypeMappingMode, + materialized: Boolean ): Type { val typeArgument = type.asArgumentList()[0] val (variance, memberType) = when { @@ -158,7 +163,7 @@ object AbstractTypeMapper { arrayElementType = AsmTypes.OBJECT_TYPE sw?.writeClass(arrayElementType) } else { - arrayElementType = mapType(context, memberType, mode.toGenericArgumentMode(variance, ofArray = true), sw) + arrayElementType = mapType(context, memberType, mode.toGenericArgumentMode(variance, ofArray = true), sw, materialized) } sw?.writeArrayEnd() return AsmUtil.getArrayType(arrayElementType) @@ -169,13 +174,14 @@ object AbstractTypeMapper { mode: TypeMappingMode, type: SimpleTypeMarker, context: TypeMappingContext, - sw: Writer? + sw: Writer?, + materialized: Boolean ): Type { if (typeConstructor.isInlineClass() && !mode.needInlineClassWrapping) { val expandedType = computeExpandedTypeForInlineClass(type) require(expandedType is SimpleTypeMarker?) if (expandedType != null) { - return mapType(context, expandedType, mode.wrapInlineClassesMode(), sw) + return mapType(context, expandedType, mode.wrapInlineClassesMode(), sw, materialized) } } diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBytecodeTextTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBytecodeTextTestGenerated.java index 73ad04d55b0..c8cdbe442eb 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBytecodeTextTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBytecodeTextTestGenerated.java @@ -4136,6 +4136,12 @@ public class FirBytecodeTextTestGenerated extends AbstractFirBytecodeTextTest { 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 @TestMetadata("nestedClassInAnnotationArgument.kt") public void testNestedClassInAnnotationArgument() throws Exception { diff --git a/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt b/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt index 0edc0ffc8ce..0db54b26f73 100644 --- a/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt +++ b/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt @@ -86,13 +86,16 @@ class ClassCodegen private constructor( if (context.state.oldInnerClassesLogic) context.defaultTypeMapper 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 while (t.isArray()) { t = t.getArrayElementType(context.irBuiltIns) } - t.classOrNull?.owner?.let(::addInnerClassInfo) - return super.mapType(type, mode, sw) + // Only record inner class info for types that are materialized in the class file. + if (materialized) { + t.classOrNull?.owner?.let(::addInnerClassInfo) + } + return super.mapType(type, mode, sw, materialized) } } diff --git a/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/JvmSignatureClashDetector.kt b/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/JvmSignatureClashDetector.kt index fdf9c21cdad..ad9e35ec93a 100644 --- a/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/JvmSignatureClashDetector.kt +++ b/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/JvmSignatureClashDetector.kt @@ -42,7 +42,7 @@ class JvmSignatureClashDetector( } 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) } diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/mapping/IrTypeMapper.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/mapping/IrTypeMapper.kt index d230a04e070..e05a90ccb86 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/mapping/IrTypeMapper.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/mapping/IrTypeMapper.kt @@ -128,8 +128,9 @@ open class IrTypeMapper(private val context: JvmBackendContext) : KotlinTypeMapp open fun mapType( type: IrType, mode: TypeMappingMode = TypeMappingMode.DEFAULT, - sw: JvmSignatureWriter? = null - ): Type = AbstractTypeMapper.mapType(this, type, mode, sw) + sw: JvmSignatureWriter? = null, + materialized: Boolean = true + ): Type = AbstractTypeMapper.mapType(this, type, mode, sw, materialized) override fun JvmSignatureWriter.writeGenericType(type: KotlinTypeMarker, asmType: Type, mode: TypeMappingMode) { if (type is IrErrorType) { diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/mapping/MethodSignatureMapper.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/mapping/MethodSignatureMapper.kt index 9b89e66489c..09d5ff10977 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/mapping/MethodSignatureMapper.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/mapping/MethodSignatureMapper.kt @@ -151,10 +151,10 @@ class MethodSignatureMapper(private val context: JvmBackendContext, private val private fun IrSimpleFunction.isPublishedApi(): Boolean = 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) { require(declaration is IrField) { "Unsupported declaration: $declaration" } - return mapReturnType(declaration, declaration.type, sw) + return mapReturnType(declaration, declaration.type, sw, materialized) } return when { @@ -163,16 +163,16 @@ class MethodSignatureMapper(private val context: JvmBackendContext, private val Type.VOID_TYPE } 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 } if (sw == null || sw.skipGenericSignature()) { - return typeMapper.mapType(returnType, TypeMappingMode.getModeForReturnTypeNoGeneric(isAnnotationMethod), sw) + return typeMapper.mapType(returnType, TypeMappingMode.getModeForReturnTypeNoGeneric(isAnnotationMethod), sw, materialized) } val typeMappingModeFromAnnotation = @@ -180,12 +180,12 @@ class MethodSignatureMapper(private val context: JvmBackendContext, private val declaration.suppressWildcardsMode(), returnType, isAnnotationMethod, mapTypeAliases = false ) if (typeMappingModeFromAnnotation != null) { - return typeMapper.mapType(returnType, typeMappingModeFromAnnotation, sw) + return typeMapper.mapType(returnType, typeMappingModeFromAnnotation, sw, materialized) } 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 = @@ -214,13 +214,21 @@ class MethodSignatureMapper(private val context: JvmBackendContext, private val function.origin == JvmLoweredDeclarationOrigin.SYNTHETIC_INLINE_CLASS_MEMBER && function.name.asString() == "box-impl" + fun mapFakeOverrideSignatureSkipGeneric(function: IrFunction): JvmMethodSignature = + mapSignature(function, skipGenericSignature = true, materialized = false) + fun mapSignatureSkipGeneric(function: IrFunction): JvmMethodSignature = mapSignature(function, true) fun mapSignatureWithGeneric(function: IrFunction): JvmMethodGenericSignature = 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 && (!function.isFakeOverride || function.parentAsClass.isFromJava()) && function.initialSignatureFunction != null @@ -261,11 +269,11 @@ class MethodSignatureMapper(private val context: JvmBackendContext, private val if (shouldBoxSingleValueParameterForSpecialCaseOfRemove(function)) parameter.type.makeNullable() else parameter.type - writeParameter(sw, kind, type, function) + writeParameter(sw, kind, type, function, materialized) } sw.writeReturnType() - mapReturnType(function, sw) + mapReturnType(function, sw, materialized) sw.writeReturnTypeEnd() val signature = sw.makeJvmMethodSignature(mapFunctionName(function, skipSpecial)) @@ -327,19 +335,20 @@ class MethodSignatureMapper(private val context: JvmBackendContext, private val sw: JvmSignatureWriter, kind: JvmMethodParameterKind, type: IrType, - function: IrFunction + function: IrFunction, + materialized: Boolean = true ) { sw.writeParameterType(kind) - writeParameterType(sw, type, function) + writeParameterType(sw, type, function, materialized) 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 (type.isInlineClassType() && declaration.isFromJava()) { - typeMapper.mapType(type, TypeMappingMode.GENERIC_ARGUMENT, sw) + typeMapper.mapType(type, TypeMappingMode.GENERIC_ARGUMENT, sw, materialized) } else { - typeMapper.mapType(type, TypeMappingMode.DEFAULT, sw) + typeMapper.mapType(type, TypeMappingMode.DEFAULT, sw, materialized) } 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 diff --git a/compiler/testData/codegen/bytecodeText/innerClasses/kt56104.kt b/compiler/testData/codegen/bytecodeText/innerClasses/kt56104.kt new file mode 100644 index 00000000000..2e36cc684c5 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/innerClasses/kt56104.kt @@ -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 diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBytecodeTextTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBytecodeTextTestGenerated.java index 6e782b0dab5..82261908407 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBytecodeTextTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBytecodeTextTestGenerated.java @@ -4136,6 +4136,12 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest { 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 @TestMetadata("nestedClassInAnnotationArgument.kt") public void testNestedClassInAnnotationArgument() throws Exception {