From d6964525b23c95df1373c1a7fe6d9a226e66c60f Mon Sep 17 00:00:00 2001 From: Michael Bogdanov Date: Sat, 9 May 2015 14:36:22 +0300 Subject: [PATCH] Fix for KT-7490: Bad bytecode generated by delegates and lambdas #KT-7490 Fixed --- .../inline/AnonymousObjectTransformer.java | 64 ++++++++++++++----- .../codegen/inline/DeferredMethodVisitor.kt | 32 ++++++++++ .../kotlin/codegen/inline/InlineResult.java | 11 ++++ .../kotlin/codegen/inline/MethodInliner.java | 12 ++-- .../kotlin/codegen/inline/TypeRemapper.java | 14 ++++ .../anonymousObject/changingReturnType.1.kt | 7 ++ .../anonymousObject/changingReturnType.2.kt | 18 ++++++ .../BlackBoxInlineCodegenTestGenerated.java | 6 ++ ...otlinAgainstInlineKotlinTestGenerated.java | 6 ++ 9 files changed, 148 insertions(+), 22 deletions(-) create mode 100644 compiler/backend/src/org/jetbrains/kotlin/codegen/inline/DeferredMethodVisitor.kt create mode 100644 compiler/testData/codegen/boxInline/anonymousObject/changingReturnType.1.kt create mode 100644 compiler/testData/codegen/boxInline/anonymousObject/changingReturnType.2.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/AnonymousObjectTransformer.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/AnonymousObjectTransformer.java index 77c9e02456d..b81ec3c3e4f 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/AnonymousObjectTransformer.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/AnonymousObjectTransformer.java @@ -18,6 +18,7 @@ package org.jetbrains.kotlin.codegen.inline; import com.intellij.openapi.util.Pair; import com.intellij.util.ArrayUtil; +import kotlin.Function0; import org.jetbrains.annotations.NotNull; import org.jetbrains.kotlin.codegen.AsmUtil; import org.jetbrains.kotlin.codegen.ClassBuilder; @@ -62,6 +63,8 @@ public class AnonymousObjectTransformer { private final Map> fieldNames = new HashMap>(); + private final TypeRemapper typeRemapper; + public AnonymousObjectTransformer( @NotNull String objectInternalName, @NotNull InliningContext inliningContext, @@ -76,6 +79,7 @@ public class AnonymousObjectTransformer { this.newLambdaType = newLambdaType; reader = InlineCodegenUtil.buildClassReaderByInternalName(state, objectInternalName); + typeRemapper = new TypeRemapper(inliningContext.typeMapping); } private void buildInvokeParamsFor(@NotNull ParametersBuilder builder, @NotNull MethodNode node) { @@ -181,13 +185,29 @@ public class AnonymousObjectTransformer { ParametersBuilder allCapturedParamBuilder = ParametersBuilder.newBuilder(); ParametersBuilder constructorParamBuilder = ParametersBuilder.newBuilder(); List additionalFakeParams = - extractParametersMappingAndPatchConstructor(constructor, allCapturedParamBuilder, constructorParamBuilder, anonymousObjectGen); + extractParametersMappingAndPatchConstructor(constructor, allCapturedParamBuilder, constructorParamBuilder, + anonymousObjectGen); + List deferringMethods = new ArrayList(); for (MethodNode next : methodsToTransform) { - MethodVisitor visitor = newMethod(classBuilder, next); - InlineResult funResult = inlineMethod(anonymousObjectGen, parentRemapper, visitor, next, allCapturedParamBuilder); + MethodVisitor deferringVisitor = newMethod(classBuilder, next); + InlineResult funResult = inlineMethod(anonymousObjectGen, parentRemapper, deferringVisitor, next, allCapturedParamBuilder); result.addAllClassesToRemove(funResult); result.getReifiedTypeParametersUsages().mergeAll(funResult.getReifiedTypeParametersUsages()); + + Type returnType = Type.getReturnType(next.desc); + if (!AsmUtil.isPrimitive(returnType)) { + String oldFunReturnType = returnType.getInternalName(); + String newFunReturnType = funResult.getChangedTypes().get(oldFunReturnType); + if (newFunReturnType != null) { + typeRemapper.addAdditionalMappings(oldFunReturnType, newFunReturnType); + } + } + deferringMethods.add(deferringVisitor); + } + + for (MethodVisitor method : deferringMethods) { + method.visitEnd(); } InlineResult constructorResult = @@ -207,7 +227,7 @@ public class AnonymousObjectTransformer { private InlineResult inlineMethod( @NotNull AnonymousObjectGeneration anonymousObjectGen, @NotNull FieldRemapper parentRemapper, - @NotNull MethodVisitor resultVisitor, + @NotNull MethodVisitor deferringVisitor, @NotNull MethodNode sourceNode, @NotNull ParametersBuilder capturedBuilder ) { @@ -223,10 +243,9 @@ public class AnonymousObjectTransformer { remapper, isSameModule, "Transformer for " + anonymousObjectGen.getOwnerInternalName(), sourceMapper); - InlineResult result = inliner.doInline(resultVisitor, new LocalVarRemapper(parameters, 0), false, LabelOwner.NOT_APPLICABLE); + InlineResult result = inliner.doInline(deferringVisitor, new LocalVarRemapper(parameters, 0), false, LabelOwner.NOT_APPLICABLE); result.getReifiedTypeParametersUsages().mergeAll(typeParametersToReify); - resultVisitor.visitMaxs(-1, -1); - resultVisitor.visitEnd(); + deferringVisitor.visitMaxs(-1, -1); return result; } @@ -338,19 +357,30 @@ public class AnonymousObjectTransformer { @NotNull private ClassBuilder createClassBuilder() { ClassBuilder classBuilder = state.getFactory().newVisitor(NO_ORIGIN, newLambdaType, inliningContext.getRoot().callElement.getContainingFile()); - return new RemappingClassBuilder(classBuilder, new TypeRemapper(inliningContext.typeMapping)); + return new RemappingClassBuilder(classBuilder, typeRemapper); } @NotNull - private static MethodVisitor newMethod(@NotNull ClassBuilder builder, @NotNull MethodNode original) { - return builder.newMethod( - NO_ORIGIN, - original.access, - original.name, - original.desc, - original.signature, - ArrayUtil.toStringArray(original.exceptions) - ); + private static DeferredMethodVisitor newMethod(@NotNull final ClassBuilder builder, @NotNull final MethodNode original) { + return new DeferredMethodVisitor( + new MethodNode(original.access, + original.name, + original.desc, + original.signature, + ArrayUtil.toStringArray(original.exceptions)), + + new Function0() { + @Override + public MethodVisitor invoke() { + return builder.newMethod( + NO_ORIGIN, + original.access, + original.name, + original.desc, + original.signature, + ArrayUtil.toStringArray(original.exceptions)); + } + }); } private List extractParametersMappingAndPatchConstructor( diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/DeferredMethodVisitor.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/DeferredMethodVisitor.kt new file mode 100644 index 00000000000..4cd6db29ebe --- /dev/null +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/DeferredMethodVisitor.kt @@ -0,0 +1,32 @@ +/* + * Copyright 2010-2015 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.kotlin.codegen.inline + +import org.jetbrains.org.objectweb.asm.MethodVisitor +import org.jetbrains.org.objectweb.asm.tree.MethodNode + +public class DeferredMethodVisitor( + val intermediate: MethodNode, + val resultNode: () -> MethodVisitor +) : MethodVisitor(InlineCodegenUtil.API, intermediate) { + + override fun visitEnd() { + super.visitEnd() + val resultVisitor = resultNode() + intermediate.accept(resultVisitor) + } +} \ No newline at end of file diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InlineResult.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InlineResult.java index 7890ab45f03..071d325bc0a 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InlineResult.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InlineResult.java @@ -18,12 +18,15 @@ package org.jetbrains.kotlin.codegen.inline; import org.jetbrains.annotations.NotNull; +import java.util.HashMap; import java.util.HashSet; +import java.util.Map; import java.util.Set; public class InlineResult { private final Set classesToRemove = new HashSet(); + private final Map changedTypes = new HashMap(); private final ReifiedTypeParametersUsages reifiedTypeParametersUsages = new ReifiedTypeParametersUsages(); private InlineResult() { @@ -44,11 +47,19 @@ public class InlineResult { classesToRemove.add(classInternalName); } + public void addChangedType(@NotNull String oldClassInternalName, @NotNull String newClassInternalName) { + changedTypes.put(oldClassInternalName, newClassInternalName); + } + @NotNull public Set getClassesToRemove() { return classesToRemove; } + public Map getChangedTypes() { + return changedTypes; + } + @NotNull public ReifiedTypeParametersUsages getReifiedTypeParametersUsages() { return reifiedTypeParametersUsages; diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.java index a697e0caa16..58df5648524 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.java @@ -158,26 +158,28 @@ public class MethodInliner { if (anonymousObjectGen.shouldRegenerate()) { //TODO: need poping of type but what to do with local funs??? - Type newLambdaType = Type.getObjectType(inliningContext.nameGenerator.genLambdaClassName()); - currentTypeMapping.put(anonymousObjectGen.getOwnerInternalName(), newLambdaType.getInternalName()); + String oldClassName = anonymousObjectGen.getOwnerInternalName(); + String newClassName = inliningContext.nameGenerator.genLambdaClassName(); + currentTypeMapping.put(oldClassName, newClassName); AnonymousObjectTransformer transformer = - new AnonymousObjectTransformer(anonymousObjectGen.getOwnerInternalName(), + new AnonymousObjectTransformer(oldClassName, inliningContext .subInlineWithClassRegeneration( inliningContext.nameGenerator, currentTypeMapping, anonymousObjectGen), - isSameModule, newLambdaType + isSameModule, Type.getObjectType(newClassName) ); InlineResult transformResult = transformer.doTransform(anonymousObjectGen, nodeRemapper); result.addAllClassesToRemove(transformResult); + result.addChangedType(oldClassName, newClassName); if (inliningContext.isInliningLambda && !anonymousObjectGen.isStaticOrigin()) { // this class is transformed and original not used so we should remove original one after inlining // Note: It is unsafe to remove anonymous class that is referenced by GETSTATIC within lambda // because it can be local function from outer scope - result.addClassToRemove(anonymousObjectGen.getOwnerInternalName()); + result.addClassToRemove(oldClassName); } if (transformResult.getReifiedTypeParametersUsages().wereUsedReifiedParameters()) { diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/TypeRemapper.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/TypeRemapper.java index ceed92f5352..6ff1757821e 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/TypeRemapper.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/TypeRemapper.java @@ -19,10 +19,12 @@ package org.jetbrains.kotlin.codegen.inline; import org.jetbrains.annotations.NotNull; import org.jetbrains.org.objectweb.asm.commons.Remapper; +import java.util.HashMap; import java.util.Map; public class TypeRemapper extends Remapper { private final Map typeMapping; + private Map additionalMappings; //typeMapping could be changed outside through method processing public TypeRemapper(@NotNull Map typeMapping) { @@ -36,6 +38,18 @@ public class TypeRemapper extends Remapper { return newType; } + if (additionalMappings != null) { + newType = additionalMappings.get(type); + if (newType != null) { + return newType; + } + } + return type; } + + public void addAdditionalMappings(String oldName, String newName) { + if (additionalMappings == null) additionalMappings = new HashMap(); + additionalMappings.put(oldName, newName); + } } diff --git a/compiler/testData/codegen/boxInline/anonymousObject/changingReturnType.1.kt b/compiler/testData/codegen/boxInline/anonymousObject/changingReturnType.1.kt new file mode 100644 index 00000000000..4862fd09175 --- /dev/null +++ b/compiler/testData/codegen/boxInline/anonymousObject/changingReturnType.1.kt @@ -0,0 +1,7 @@ +//NO_CHECK_LAMBDA_INLINING +//KT-7490 +import test.* + +fun box(): String { + return Entity("OK").directed().calc().value +} \ No newline at end of file diff --git a/compiler/testData/codegen/boxInline/anonymousObject/changingReturnType.2.kt b/compiler/testData/codegen/boxInline/anonymousObject/changingReturnType.2.kt new file mode 100644 index 00000000000..00db7bcbf97 --- /dev/null +++ b/compiler/testData/codegen/boxInline/anonymousObject/changingReturnType.2.kt @@ -0,0 +1,18 @@ +package test + +open class Entity(val value: String) + +public abstract class Task() { + abstract fun calc(): T +} + +fun nullableTask(factory: () -> Task): Task { + return factory() +} + +inline fun Self.directed(): Task = + nullableTask { + object : Task() { + override fun calc(): Self = this@directed + } + } \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxInlineCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxInlineCodegenTestGenerated.java index 57f60fc61d8..1665a29366c 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxInlineCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxInlineCodegenTestGenerated.java @@ -90,6 +90,12 @@ public class BlackBoxInlineCodegenTestGenerated extends AbstractBlackBoxInlineCo String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/anonymousObject/capturedLambdaInInlineObject.1.kt"); doTestMultiFileWithInlineCheck(fileName); } + + @TestMetadata("changingReturnType.1.kt") + public void testChangingReturnType() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/anonymousObject/changingReturnType.1.kt"); + doTestMultiFileWithInlineCheck(fileName); + } } @TestMetadata("compiler/testData/codegen/boxInline/builders") diff --git a/compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstInlineKotlinTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstInlineKotlinTestGenerated.java index 987762e8758..bfdffa36208 100644 --- a/compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstInlineKotlinTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstInlineKotlinTestGenerated.java @@ -90,6 +90,12 @@ public class CompileKotlinAgainstInlineKotlinTestGenerated extends AbstractCompi String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/anonymousObject/capturedLambdaInInlineObject.1.kt"); doBoxTestWithInlineCheck(fileName); } + + @TestMetadata("changingReturnType.1.kt") + public void testChangingReturnType() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/anonymousObject/changingReturnType.1.kt"); + doBoxTestWithInlineCheck(fileName); + } } @TestMetadata("compiler/testData/codegen/boxInline/builders")