From 7bdc09301d4d0eb39cb38465ff0596f4e88cf75f Mon Sep 17 00:00:00 2001 From: pyos Date: Thu, 28 Apr 2022 16:41:07 +0200 Subject: [PATCH] JVM: add inline lambdas' captured parameters to constructor first This makes the code a bit cleaner, but has no effect other than that. --- .../inline/AnonymousObjectTransformer.kt | 38 +++++++------------ .../common/lower/LocalDeclarationsLowering.kt | 10 ++--- 2 files changed, 16 insertions(+), 32 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/AnonymousObjectTransformer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/AnonymousObjectTransformer.kt index 50d4f768c8e..ae6f91ef5f8 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/AnonymousObjectTransformer.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/AnonymousObjectTransformer.kt @@ -511,30 +511,20 @@ class AnonymousObjectTransformer( val capturedOuterThisTypes = mutableSetOf() for (info in capturedLambdas) { for (desc in info.capturedVars) { - // Merge all outer `this` of the same type captured by inlined lambdas, since they have to be the same - // object. Outer `this` captured by the original object itself should have been renamed above, - // and can have a different value even if the same type is captured by a lambda. - val recapturedParamInfo = if (isThis0(desc.fieldName)) - capturedParamBuilder.addCapturedParam(desc, desc.fieldName, !capturedOuterThisTypes.add(desc.type.className)) - else - capturedParamBuilder.addCapturedParam(desc, addUniqueField(desc.fieldName + INLINE_TRANSFORMATION_SUFFIX), false) + val recapturedParamInfo = constructorParamBuilder.addCapturedParam( + desc, + // Merge all outer `this` of the same type captured by inlined lambdas, since they have to be the same + // object. Outer `this` captured by the original object itself should have been renamed above, + // and can have a different value even if the same type is captured by a lambda. + if (isThis0(desc.fieldName)) desc.fieldName else addUniqueField(desc.fieldName + INLINE_TRANSFORMATION_SUFFIX), + (isThis0(desc.fieldName) && !capturedOuterThisTypes.add(desc.type.className)) + ) if (desc.isSuspend) { recapturedParamInfo.functionalArgument = NonInlineArgumentForInlineSuspendParameter.INLINE_LAMBDA_AS_VARIABLE } - val composed = StackValue.field( - desc.type, - oldObjectType, /*TODO owner type*/ - recapturedParamInfo.newFieldName, - false, - StackValue.LOCAL_0 - ) - recapturedParamInfo.remapValue = composed + capturedParamBuilder.addCapturedParam(recapturedParamInfo, recapturedParamInfo.newFieldName).remapValue = + StackValue.field(desc.type, oldObjectType, recapturedParamInfo.newFieldName, false, StackValue.LOCAL_0) allRecapturedParameters.add(desc) - - // DO NOT assign a remap value to the constructor parameter - this would cause the inliner to miscount - // the arguments, eventually generating code that overwrites some of these captures with other stuff. - // Plus we don't really want to remap local loads to field reads anyway, the latter are faster. - constructorParamBuilder.addCapturedParam(recapturedParamInfo, recapturedParamInfo.newFieldName).remapValue = null } } } else if (capturedLambdas.isNotEmpty()) { @@ -545,12 +535,10 @@ class AnonymousObjectTransformer( ?: throw AssertionError("Expecting RegeneratedLambdaFieldRemapper, but ${parentFieldRemapper.parent}") val ownerType = Type.getObjectType(parent.originalLambdaInternalName) val desc = CapturedParamDesc(ownerType, AsmUtil.THIS, ownerType) - val recapturedParamInfo = capturedParamBuilder.addCapturedParam(desc, AsmUtil.CAPTURED_THIS_FIELD/*outer lambda/object*/, false) - val composed = StackValue.LOCAL_0 - recapturedParamInfo.remapValue = composed + val recapturedParamInfo = + constructorParamBuilder.addCapturedParam(desc, AsmUtil.CAPTURED_THIS_FIELD/*outer lambda/object*/, false) + capturedParamBuilder.addCapturedParam(recapturedParamInfo, recapturedParamInfo.newFieldName).remapValue = StackValue.LOCAL_0 allRecapturedParameters.add(desc) - - constructorParamBuilder.addCapturedParam(recapturedParamInfo, recapturedParamInfo.newFieldName).remapValue = null } transformationInfo.allRecapturedParameters = allRecapturedParameters diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/LocalDeclarationsLowering.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/LocalDeclarationsLowering.kt index f7276412272..e5d04d30854 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/LocalDeclarationsLowering.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/LocalDeclarationsLowering.kt @@ -172,13 +172,9 @@ class LocalDeclarationsLowering( val capturedValueToField: MutableMap = mutableMapOf() override fun irGet(startOffset: Int, endOffset: Int, valueDeclaration: IrValueDeclaration): IrExpression? { - // On the JVM backend, `AnonymousObjectTransformer` in the bytecode inliner uses field assignment - // instructions to find captured parameters. Most of the time this is unimportant, as captured - // parameters are effectively indistinguishable from normal ones; the exception is ones that can - // take an inline lambda value, as the parameter needs to be replaced with the lambda's capture. - // Thus we cannot erase the field - the inliner won't handle the inline lambda without it. - // (Most of the time this is inconsequential - if the object really is instantiated with an inline - // lambda, the field will be erased completely regardless of whether it's used.) + // TODO: this used to be a hack for the JVM bytecode inliner (which misbehaved when inline lambdas had no fields), + // but it's no longer necessary. It is only here for backwards compatibility with old kotlinc versions + // and can be removed, probably in 1.9. if (!forceFieldsForInlineCaptures || !valueDeclaration.isInlineDeclaration()) { // We're in the initializer scope, which will be moved to a primary constructor later. // Thus we can directly use that constructor's context and read from a parameter instead of a field.