Fix for KT-7490: Bad bytecode generated by delegates and lambdas

#KT-7490 Fixed
2015-05-09 14:36:22 +03:00
parent aed6ed0f9b
commit d6964525b2
9 changed files with 148 additions and 22 deletions
@@ -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<String, List<String>> fieldNames = new HashMap<String, List<String>>();

+    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<CapturedParamInfo> additionalFakeParams =
-                extractParametersMappingAndPatchConstructor(constructor, allCapturedParamBuilder, constructorParamBuilder, anonymousObjectGen);
+                extractParametersMappingAndPatchConstructor(constructor, allCapturedParamBuilder, constructorParamBuilder,
+                                                            anonymousObjectGen);
+        List<MethodVisitor> 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<MethodVisitor>() {
+                    @Override
+                    public MethodVisitor invoke() {
+                        return builder.newMethod(
+                                NO_ORIGIN,
+                                original.access,
+                                original.name,
+                                original.desc,
+                                original.signature,
+                                ArrayUtil.toStringArray(original.exceptions));
+                    }
+                });
    }

    private List<CapturedParamInfo> extractParametersMappingAndPatchConstructor(
@@ -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)
+    }
+}
@@ -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<String> classesToRemove = new HashSet<String>();
+    private final Map<String, String> changedTypes = new HashMap<String, String>();
    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<String> getClassesToRemove() {
        return classesToRemove;
    }

+    public Map<String, String> getChangedTypes() {
+        return changedTypes;
+    }
+
    @NotNull
    public ReifiedTypeParametersUsages getReifiedTypeParametersUsages() {
        return reifiedTypeParametersUsages;
@@ -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()) {
@@ -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<String, String> typeMapping;
+    private Map<String, String> additionalMappings;

    //typeMapping could be changed outside through method processing
    public TypeRemapper(@NotNull Map<String, String> 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<String, String>();
+        additionalMappings.put(oldName, newName);
+    }
 }
@@ -0,0 +1,7 @@
+//NO_CHECK_LAMBDA_INLINING
+//KT-7490
+import test.*
+
+fun box(): String {
+    return Entity("OK").directed().calc().value
+}
@@ -0,0 +1,18 @@
+package test
+
+open class Entity(val value: String)
+
+public abstract class Task<T>() {
+    abstract fun calc(): T
+}
+
+fun <Self : Entity> nullableTask(factory: () -> Task<Self>): Task<Self> {
+    return factory()
+}
+
+inline fun<reified Self : Entity> Self.directed(): Task<Self> =
+        nullableTask {
+            object : Task<Self>() {
+                override fun calc(): Self = this@directed
+            }
+        }
@@ -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")
@@ -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")