From c46c80822c6a14ed18be423c77677125b50ea568 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 3 Sep 2020 18:31:06 +0200 Subject: [PATCH] JVM IR: fix enclosing constructor for lambdas in inner classes Before this change, we stored the enclosing constructor in a map in JvmBackendContext before moving lambdas and local classes out of initializer blocks. However, in case the lambda was declared in an inner class, we stored a reference to the unlowered constructor of the inner class, whose JVM signature is "()V" instead of the correct "(LOuter;)V". Java reflection then threw exception if we tried to call `getEnclosingConstructor()` on such class at runtime. Proguard finished with errors for the same reason. It turns out that we can just store the fact that the class has been moved, and load the matching constructor in codegen, where everything is already lowered and guaranteed to match with the signatures of the actual generated declarations. #KT-41668 Fixed --- .../kotlin/backend/jvm/JvmBackendContext.kt | 2 +- .../backend/jvm/codegen/ClassCodegen.kt | 9 +++++- .../backend/jvm/codegen/irCodegenUtils.kt | 2 +- .../jvm/lower/JvmLocalClassPopupLowering.kt | 22 +++++--------- .../lambdaInInnerClassConstructor.kt | 6 ++++ .../lambdaInInnerClassConstructor.txt | 30 +++++++++++++++++++ .../codegen/BytecodeListingTestGenerated.java | 5 ++++ .../ir/IrBytecodeListingTestGenerated.java | 5 ++++ 8 files changed, 63 insertions(+), 18 deletions(-) create mode 100644 compiler/testData/codegen/bytecodeListing/inline/enclosingInfo/lambdaInInnerClassConstructor.kt create mode 100644 compiler/testData/codegen/bytecodeListing/inline/enclosingInfo/lambdaInInnerClassConstructor.txt diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmBackendContext.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmBackendContext.kt index ffafe153c13..2544cbc7317 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmBackendContext.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmBackendContext.kt @@ -91,7 +91,7 @@ class JvmBackendContext( localClassType[container.attributeOwnerId] = value } - internal val customEnclosingFunction = mutableMapOf() + internal val isEnclosedInConstructor = mutableSetOf() internal val classCodegens = mutableMapOf() diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt index 997a9187112..420d3863fed 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt @@ -36,6 +36,7 @@ import org.jetbrains.kotlin.resolve.jvm.annotations.VOLATILE_ANNOTATION_FQ_NAME import org.jetbrains.kotlin.resolve.jvm.checkers.JvmSimpleNameBacktickChecker import org.jetbrains.kotlin.resolve.jvm.diagnostics.* import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmClassSignature +import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull import org.jetbrains.kotlin.utils.addToStdlib.safeAs import org.jetbrains.org.objectweb.asm.* import org.jetbrains.org.objectweb.asm.commons.Method @@ -360,7 +361,13 @@ abstract class ClassCodegen protected constructor( // the current class. If the current class is immediately enclosed by a method // or constructor, the name and type of the function is recorded as well. if (parentClassCodegen != null) { - val enclosingFunction = context.customEnclosingFunction[irClass.attributeOwnerId] ?: parentFunction + // In case there's no primary constructor, it's unclear which constructor should be the enclosing one, so we select the first. + val enclosingFunction = if (irClass.attributeOwnerId in context.isEnclosedInConstructor) { + val containerClass = parentClassCodegen.irClass + containerClass.primaryConstructor + ?: containerClass.declarations.firstIsInstanceOrNull() + ?: error("Class in a non-static initializer found, but container has no constructors: ${containerClass.render()}") + } else parentFunction if (enclosingFunction != null || irClass.isAnonymousObject) { val method = enclosingFunction?.let(context.methodSignatureMapper::mapAsmMethod) visitor.visitOuterClass(parentClassCodegen.type.internalName, method?.name, method?.descriptor) diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/irCodegenUtils.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/irCodegenUtils.kt index a699f9f5e04..945a542486f 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/irCodegenUtils.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/irCodegenUtils.kt @@ -113,7 +113,7 @@ val IrType.isExtensionFunctionType: Boolean fun writeInnerClass(innerClass: IrClass, typeMapper: IrTypeMapper, context: JvmBackendContext, v: ClassBuilder) { val outerClassInternalName = - if (context.customEnclosingFunction[innerClass.attributeOwnerId] != null) null + if (innerClass.attributeOwnerId in context.isEnclosedInConstructor) null else innerClass.parent.safeAs()?.let(typeMapper::classInternalName) val innerName = innerClass.name.takeUnless { it.isSpecial }?.asString() val innerClassInternalName = typeMapper.classInternalName(innerClass) diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmLocalClassPopupLowering.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmLocalClassPopupLowering.kt index 2dde70473ac..7e0150a300f 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmLocalClassPopupLowering.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmLocalClassPopupLowering.kt @@ -12,10 +12,6 @@ import org.jetbrains.kotlin.backend.jvm.JvmLoweredDeclarationOrigin import org.jetbrains.kotlin.backend.jvm.ir.IrInlineReferenceLocator import org.jetbrains.kotlin.ir.declarations.* import org.jetbrains.kotlin.ir.expressions.IrFunctionReference -import org.jetbrains.kotlin.ir.util.parentAsClass -import org.jetbrains.kotlin.ir.util.primaryConstructor -import org.jetbrains.kotlin.ir.util.render -import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull class JvmLocalClassPopupLowering(context: JvmBackendContext) : LocalClassPopupLowering(context) { private val inlineLambdaToScope = mutableMapOf() @@ -49,17 +45,13 @@ class JvmLocalClassPopupLowering(context: JvmBackendContext) : LocalClassPopupLo while (parent is IrFunction) { parent = inlineLambdaToScope[parent] ?: break } - val container = when (parent) { - is IrAnonymousInitializer -> parent.parentAsClass.takeUnless { parent.isStatic } - is IrField -> parent.parentAsClass.takeUnless { parent.isStatic } - else -> null - } ?: return false - // In case there's no primary constructor, it's unclear which constructor should be the enclosing one, so we select the first. - (context as JvmBackendContext).customEnclosingFunction[klass.attributeOwnerId] = - container.primaryConstructor ?: container.declarations.firstIsInstanceOrNull() - ?: error("Class in a non-static initializer found, but container has no constructors: ${container.render()}") - - return true + if (parent is IrAnonymousInitializer && !parent.isStatic || + parent is IrField && !parent.isStatic + ) { + (context as JvmBackendContext).isEnclosedInConstructor.add(klass.attributeOwnerId) + return true + } + return false } } diff --git a/compiler/testData/codegen/bytecodeListing/inline/enclosingInfo/lambdaInInnerClassConstructor.kt b/compiler/testData/codegen/bytecodeListing/inline/enclosingInfo/lambdaInInnerClassConstructor.kt new file mode 100644 index 00000000000..08c06dd27c9 --- /dev/null +++ b/compiler/testData/codegen/bytecodeListing/inline/enclosingInfo/lambdaInInnerClassConstructor.kt @@ -0,0 +1,6 @@ +class Outer { + inner class Inner { + // This test checks the EnclosingMethod attribute of this lambda class. + val lambda = {} + } +} diff --git a/compiler/testData/codegen/bytecodeListing/inline/enclosingInfo/lambdaInInnerClassConstructor.txt b/compiler/testData/codegen/bytecodeListing/inline/enclosingInfo/lambdaInInnerClassConstructor.txt new file mode 100644 index 00000000000..0606ad49601 --- /dev/null +++ b/compiler/testData/codegen/bytecodeListing/inline/enclosingInfo/lambdaInInnerClassConstructor.txt @@ -0,0 +1,30 @@ +@kotlin.Metadata +final class Outer$Inner$lambda$1 { + // source: 'lambdaInInnerClassConstructor.kt' + enclosing method Outer$Inner.(LOuter;)V + public final static field INSTANCE: Outer$Inner$lambda$1 + inner (anonymous) class Outer$Inner$lambda$1 + static method (): void + method (): void + public synthetic bridge method invoke(): java.lang.Object + public final method invoke(): void + public final inner class Outer$Inner +} + +@kotlin.Metadata +public final class Outer$Inner { + // source: 'lambdaInInnerClassConstructor.kt' + private final @org.jetbrains.annotations.NotNull field lambda: kotlin.jvm.functions.Function0 + synthetic final field this$0: Outer + inner (anonymous) class Outer$Inner$lambda$1 + public method (p0: Outer): void + public final @org.jetbrains.annotations.NotNull method getLambda(): kotlin.jvm.functions.Function0 + public final inner class Outer$Inner +} + +@kotlin.Metadata +public final class Outer { + // source: 'lambdaInInnerClassConstructor.kt' + public method (): void + public final inner class Outer$Inner +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeListingTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeListingTestGenerated.java index 23b8c3f7c19..ab4387d8978 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeListingTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeListingTestGenerated.java @@ -474,6 +474,11 @@ public class BytecodeListingTestGenerated extends AbstractBytecodeListingTest { runTest("compiler/testData/codegen/bytecodeListing/inline/enclosingInfo/lambdaInInitBlockNoPrimaryConstructor.kt"); } + @TestMetadata("lambdaInInnerClassConstructor.kt") + public void testLambdaInInnerClassConstructor() throws Exception { + runTest("compiler/testData/codegen/bytecodeListing/inline/enclosingInfo/lambdaInInnerClassConstructor.kt"); + } + @TestMetadata("transformedConstructor.kt") public void testTransformedConstructor() throws Exception { runTest("compiler/testData/codegen/bytecodeListing/inline/enclosingInfo/transformedConstructor.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeListingTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeListingTestGenerated.java index 0cc5ec43a0c..856a241bc12 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeListingTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeListingTestGenerated.java @@ -444,6 +444,11 @@ public class IrBytecodeListingTestGenerated extends AbstractIrBytecodeListingTes runTest("compiler/testData/codegen/bytecodeListing/inline/enclosingInfo/lambdaInInitBlockNoPrimaryConstructor.kt"); } + @TestMetadata("lambdaInInnerClassConstructor.kt") + public void testLambdaInInnerClassConstructor() throws Exception { + runTest("compiler/testData/codegen/bytecodeListing/inline/enclosingInfo/lambdaInInnerClassConstructor.kt"); + } + @TestMetadata("transformedConstructor.kt") public void testTransformedConstructor() throws Exception { runTest("compiler/testData/codegen/bytecodeListing/inline/enclosingInfo/transformedConstructor.kt");