Generate type-safe barrier in method body

In cases when signature of special bridge is the same as current method, but type is not 'Any?'. Also there is tiny optimization: only null check needed if value parameter type is mapped to Object, but it's not nullable. #KT-9973 Fixed
2015-11-16 13:02:45 +03:00
parent 1f704e0c4d
commit fa99ea1e98
14 changed files with 226 additions and 29 deletions
@@ -27,6 +27,7 @@ import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.kotlin.backend.common.bridges.Bridge;
 import org.jetbrains.kotlin.backend.common.bridges.ImplKt;
+import org.jetbrains.kotlin.builtins.KotlinBuiltIns;
 import org.jetbrains.kotlin.codegen.annotation.AnnotatedWithOnlyTargetedAnnotations;
 import org.jetbrains.kotlin.codegen.context.*;
 import org.jetbrains.kotlin.codegen.intrinsics.TypeIntrinsics;
@@ -59,6 +60,7 @@ import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodParameterKind;
 import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodParameterSignature;
 import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodSignature;
 import org.jetbrains.kotlin.types.KotlinType;
+import org.jetbrains.kotlin.types.TypeUtils;
 import org.jetbrains.org.objectweb.asm.AnnotationVisitor;
 import org.jetbrains.org.objectweb.asm.Label;
 import org.jetbrains.org.objectweb.asm.MethodVisitor;
@@ -355,6 +357,10 @@ public class FunctionCodegen {
        mv.visitLabel(methodBegin);

        JetTypeMapper typeMapper = parentCodegen.typeMapper;
+        if (BuiltinSpecialBridgesUtil.shouldHaveTypeSafeBarrier(functionDescriptor, getSignatureMapper(typeMapper))) {
+            generateTypeCheckBarrierIfNeeded(
+                    new InstructionAdapter(mv), functionDescriptor, signature.getReturnType(), /* delegateParameterType = */null);
+        }

        Label methodEnd;

@@ -556,12 +562,7 @@ public class FunctionCodegen {
        if (!isSpecial) {
            bridgesToGenerate = ImplKt.generateBridgesForFunctionDescriptor(
                    descriptor,
-                    new Function1<FunctionDescriptor, Method>() {
-                        @Override
-                        public Method invoke(FunctionDescriptor descriptor) {
-                            return typeMapper.mapSignature(descriptor).getAsmMethod();
-                        }
-                    }
+                    getSignatureMapper(typeMapper)
            );
            if (!bridgesToGenerate.isEmpty()) {
                PsiElement origin = descriptor.getKind() == DECLARATION ? getSourceFromDescriptor(descriptor) : null;
@@ -576,12 +577,7 @@ public class FunctionCodegen {
        else {
            Set<BridgeForBuiltinSpecial<Method>> specials = BuiltinSpecialBridgesUtil.generateBridgesForBuiltinSpecial(
                    descriptor,
-                    new Function1<FunctionDescriptor, Method>() {
-                        @Override
-                        public Method invoke(FunctionDescriptor descriptor) {
-                            return typeMapper.mapSignature(descriptor).getAsmMethod();
-                        }
-                    }
+                    getSignatureMapper(typeMapper)
            );

            if (!specials.isEmpty()) {
@@ -604,6 +600,16 @@ public class FunctionCodegen {
        }
    }

+    @NotNull
+    private static Function1<FunctionDescriptor, Method> getSignatureMapper(final @NotNull  JetTypeMapper typeMapper) {
+        return new Function1<FunctionDescriptor, Method>() {
+            @Override
+            public Method invoke(FunctionDescriptor descriptor) {
+                return typeMapper.mapSignature(descriptor).getAsmMethod();
+            }
+        };
+    }
+
    private static boolean isMethodOfAny(@NotNull FunctionDescriptor descriptor) {
        String name = descriptor.getName().asString();
        List<ValueParameterDescriptor> parameters = descriptor.getValueParameters();
@@ -852,7 +858,9 @@ public class FunctionCodegen {
        InstructionAdapter iv = new InstructionAdapter(mv);
        MemberCodegen.markLineNumberForSyntheticFunction(owner.getThisDescriptor(), iv);

-        generateInstanceOfBarrierIfNeeded(iv, descriptor, bridge, delegateTo);
+        if (delegateTo.getArgumentTypes().length == 1 && isSpecialBridge) {
+            generateTypeCheckBarrierIfNeeded(iv, descriptor, bridge.getReturnType(), delegateTo.getArgumentTypes()[0]);
+        }

        iv.load(0, OBJECT_TYPE);
        for (int i = 0, reg = 1; i < argTypes.length; i++) {
@@ -877,11 +885,11 @@ public class FunctionCodegen {
        endVisit(mv, "bridge method", origin);
    }

-    private static void generateInstanceOfBarrierIfNeeded(
+    private static void generateTypeCheckBarrierIfNeeded(
            @NotNull InstructionAdapter iv,
            @NotNull FunctionDescriptor descriptor,
-            @NotNull Method bridge,
-            @NotNull final Method delegateTo
+            @NotNull Type returnType,
+            @Nullable final Type delegateParameterType
    ) {
        BuiltinMethodsWithSpecialGenericSignature.DefaultValue defaultValue =
                BuiltinMethodsWithSpecialGenericSignature.getDefaultValueForOverriddenBuiltinFunction(descriptor);
@@ -889,24 +897,30 @@ public class FunctionCodegen {

        assert descriptor.getValueParameters().size() == 1 : "Should be descriptor with one value parameter, but found: " + descriptor;

-        if (bridge.getArgumentTypes()[0].getSort() != Type.OBJECT) return;
+        boolean isCheckForAny = delegateParameterType == null || OBJECT_TYPE.equals(delegateParameterType);
+
+        final KotlinType kotlinType = descriptor.getValueParameters().get(0).getType();
+
+        if (isCheckForAny && TypeUtils.isNullableType(kotlinType)) return;

        iv.load(1, OBJECT_TYPE);

-        final KotlinType kotlinType = descriptor.getValueParameters().get(0).getType();
-        CodegenUtilKt.generateIsCheck(iv, kotlinType, new Function1<InstructionAdapter, Unit>() {
-            @Override
-            public Unit invoke(InstructionAdapter adapter) {
-                TypeIntrinsics.instanceOf(adapter, kotlinType, boxType(delegateTo.getArgumentTypes()[0]));
-                return Unit.INSTANCE;
-            }
-        });
-
        Label afterBarrier = new Label();

-        iv.ifne(afterBarrier);
-
-        Type returnType = bridge.getReturnType();
+        if (isCheckForAny) {
+            assert !TypeUtils.isNullableType(kotlinType) : "Only bridges for not-nullable types are necessary";
+            iv.ifnonnull(afterBarrier);
+        }
+        else {
+            CodegenUtilKt.generateIsCheck(iv, kotlinType, new Function1<InstructionAdapter, Unit>() {
+                @Override
+                public Unit invoke(InstructionAdapter adapter) {
+                    TypeIntrinsics.instanceOf(adapter, kotlinType, boxType(delegateParameterType));
+                    return Unit.INSTANCE;
+                }
+            });
+            iv.ifne(afterBarrier);
+        }

        StackValue.constant(defaultValue.getValue(), returnType).put(returnType, iv);
        iv.areturn(returnType);
@@ -88,6 +88,16 @@ object BuiltinSpecialBridgesUtil {

        return bridges
    }
+
+    @JvmStatic
+    public fun <Signature> FunctionDescriptor.shouldHaveTypeSafeBarrier(
+            signatureByDescriptor: (FunctionDescriptor) -> Signature
+    ): Boolean {
+        if (BuiltinMethodsWithSpecialGenericSignature.getDefaultValueForOverriddenBuiltinFunction(this) == null) return false
+
+        val builtin = getOverriddenBuiltinWithDifferentJvmDescriptor()!!
+        return signatureByDescriptor(this) == signatureByDescriptor(builtin)
+    }
 }


@@ -0,0 +1,34 @@
+private object NotEmptyMap : MutableMap<Any, Int> {
+    override fun containsKey(key: Any): Boolean = true
+    override fun containsValue(value: Int): Boolean = true
+
+    // non-special bridges get(Object)Integer -> get(Object)I
+    override fun get(key: Any): Int = 1
+    override fun remove(key: Any): Int = 1
+
+    override val size: Int get() = 0
+    override fun isEmpty(): Boolean = true
+    override fun put(key: Any, value: Int): Int? = throw UnsupportedOperationException()
+    override fun putAll(from: Map<out Any, Int>): Unit = throw UnsupportedOperationException()
+    override fun clear(): Unit = throw UnsupportedOperationException()
+    override val entries: MutableSet<MutableMap.MutableEntry<Any, Int>> get() = null!!
+    override val keys: MutableSet<Any> get() = null!!
+    override val values: MutableCollection<Int> get() = null!!
+}
+
+
+fun box(): String {
+    val n = NotEmptyMap as MutableMap<Any?, Any?>
+
+    if (n.get(null) != null) return "fail 1"
+    if (n.containsKey(null)) return "fail 2"
+    if (n.containsValue(null)) return "fail 3"
+    if (n.remove(null) != null) return "fail 4"
+
+    if (n.get(1) == null) return "fail 5"
+    if (!n.containsKey("")) return "fail 6"
+    if (!n.containsValue(3)) return "fail 7"
+    if (n.remove("") == null) return "fail 8"
+
+    return "OK"
+}
@@ -0,0 +1,43 @@
+private object NotEmptyList : MutableList<Any> {
+    override fun contains(element: Any): Boolean = true
+    override fun indexOf(element: Any): Int = 0
+    override fun lastIndexOf(element: Any): Int = 0
+    override fun remove(element: Any): Boolean = true
+
+    override val size: Int
+        get() = throw UnsupportedOperationException()
+
+    override fun containsAll(elements: Collection<Any>): Boolean = elements.isEmpty()
+    override fun isEmpty(): Boolean = throw UnsupportedOperationException()
+    override fun get(index: Int): Any = throw UnsupportedOperationException()
+    override fun add(element: Any): Boolean = throw UnsupportedOperationException()
+    override fun addAll(elements: Collection<Any>): Boolean = throw UnsupportedOperationException()
+    override fun addAll(index: Int, elements: Collection<Any>): Boolean = throw UnsupportedOperationException()
+    override fun removeAll(elements: Collection<Any>): Boolean = throw UnsupportedOperationException()
+    override fun retainAll(elements: Collection<Any>): Boolean = throw UnsupportedOperationException()
+    override fun clear(): Unit = throw UnsupportedOperationException()
+    override fun set(index: Int, element: Any): Any = throw UnsupportedOperationException()
+    override fun add(index: Int, element: Any): Unit = throw UnsupportedOperationException()
+    override fun removeAt(index: Int): Any = throw UnsupportedOperationException()
+    override fun listIterator(): MutableListIterator<Any> = throw UnsupportedOperationException()
+    override fun listIterator(index: Int): MutableListIterator<Any> = throw UnsupportedOperationException()
+    override fun subList(fromIndex: Int, toIndex: Int): MutableList<Any> = throw UnsupportedOperationException()
+    override fun iterator(): MutableIterator<Any> = throw UnsupportedOperationException()
+}
+
+fun box(): String {
+    val n = NotEmptyList as MutableList<Any?>
+
+    if (n.contains(null)) return "fail 1"
+    if (n.indexOf(null) != -1) return "fail 2"
+    if (n.lastIndexOf(null) != -1) return "fail 3"
+
+    if (!n.contains("")) return "fail 3"
+    if (n.indexOf("") != 0) return "fail 4"
+    if (n.lastIndexOf("") != 0) return "fail 5"
+
+    if (n.remove(null)) return "fail 6"
+    if (!n.remove("")) return "fail 7"
+
+    return "OK"
+}
@@ -0,0 +1,32 @@
+private object NotEmptyMap : MutableMap<Any, Any> {
+    override fun containsKey(key: Any): Boolean = true
+    override fun containsValue(value: Any): Boolean = true
+    override fun get(key: Any): Any? = Any()
+    override fun remove(key: Any): Any? = Any()
+
+    override val size: Int get() = 0
+    override fun isEmpty(): Boolean = true
+    override fun put(key: Any, value: Any): Any? = throw UnsupportedOperationException()
+    override fun putAll(from: Map<out Any, Any>): Unit = throw UnsupportedOperationException()
+    override fun clear(): Unit = throw UnsupportedOperationException()
+    override val entries: MutableSet<MutableMap.MutableEntry<Any, Any>> get() = null!!
+    override val keys: MutableSet<Any> get() = null!!
+    override val values: MutableCollection<Any> get() = null!!
+}
+
+
+fun box(): String {
+    val n = NotEmptyMap as MutableMap<Any?, Any?>
+
+    if (n.get(null) != null) return "fail 1"
+    if (n.containsKey(null)) return "fail 2"
+    if (n.containsValue(null)) return "fail 3"
+    if (n.remove(null) != null) return "fail 4"
+
+    if (n.get("") == null) return "fail 5"
+    if (!n.containsKey("")) return "fail 6"
+    if (!n.containsValue("")) return "fail 7"
+    if (n.remove("") == null) return "fail 8"
+
+    return "OK"
+}
@@ -6,3 +6,5 @@ abstract class A7 : MutableCollection<Int> {

 // 1 public final bridge contains\(Ljava/lang/Object;\)Z
 // 1 public final bridge remove\(Ljava/lang/Object;\)Z
+/* 2 INSTANCEOF: one for 'remove', one for 'contains' type-safe bridges */
+// 2 INSTANCEOF java/lang/Integer
@@ -26,3 +26,5 @@ abstract class A6 : java.util.AbstractList<String>() {
 // 2 public final bridge remove\(Ljava/lang/Object;\)Z
 // 2 public final bridge indexOf\(Ljava/lang/Object;\)I
 // 2 public final bridge lastIndexOf\(Ljava/lang/Object;\)I
+/* 2 INSTANCEOF for each class: one for 'remove', one for 'contains' type-safe bridges */
+// 8 INSTANCEOF java/lang/String
@@ -23,3 +23,5 @@ abstract class A2 : MutableCollection<String> {
 // 1 public final bridge contains\(Ljava/lang/Object;\)Z
 // 1 public final bridge remove\(Ljava/lang/Object;\)Z
 // 1 INVOKEVIRTUAL A[0-9]\.contains \(Ljava/lang/String;\)Z
+/* 2 INSTANCEOF: one for 'remove', one for 'contains' type-safe bridges */
+// 2 INSTANCEOF
@@ -22,3 +22,7 @@ abstract class A : I2

 // 1 public final bridge contains\(Ljava/lang/Object;\)Z
 // 1 public final bridge remove\(Ljava/lang/Object;\)Z
+/* 2 INSTANCEOF: one for 'remove', one for 'contains' type-safe bridges of A
+   There should be no bridges in interfaces
+*/
+// 2 INSTANCEOF java/lang/String
@@ -0,0 +1,11 @@
+abstract class A8 : MutableCollection<Any> {
+    override fun contains(o: Any): Boolean {
+        throw UnsupportedOperationException()
+    }
+}
+
+// 1 bridge
+// 1 public final bridge size
+// 0 INSTANCEOF
+/* Only 1 IFNONNULL should be within contains method */
+// 1 IFNONNULL
@@ -0,0 +1,11 @@
+abstract class A<T : Any> : MutableCollection<T> {
+    override fun contains(o: T): Boolean {
+        throw UnsupportedOperationException()
+    }
+}
+
+// 1 bridge
+// 1 public final bridge size
+// 0 INSTANCEOF
+/* Only 1 IFNONNULL should be within contains method (because T is not nullable) */
+// 1 IFNONNULL
@@ -6,3 +6,5 @@ abstract class A8 : MutableCollection<Any?> {

 // 1 bridge
 // 1 public final bridge size
+// 0 INSTANCEOF
+// 0 NULL
@@ -435,6 +435,18 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest {
                doTest(fileName);
            }

+            @TestMetadata("notNullAnyMC.kt")
+            public void testNotNullAnyMC() throws Exception {
+                String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/builtinFunctions/genericParameterBridge/notNullAnyMC.kt");
+                doTest(fileName);
+            }
+
+            @TestMetadata("notNullParamMC.kt")
+            public void testNotNullParamMC() throws Exception {
+                String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/builtinFunctions/genericParameterBridge/notNullParamMC.kt");
+                doTest(fileName);
+            }
+
            @TestMetadata("nullableAnyMC.kt")
            public void testNullableAnyMC() throws Exception {
                String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/builtinFunctions/genericParameterBridge/nullableAnyMC.kt");
@@ -7315,6 +7315,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/specialBuiltins"), Pattern.compile("^(.+)\\.kt$"), true);
        }

+        @TestMetadata("bridgeNotEmptyMap.kt")
+        public void testBridgeNotEmptyMap() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/specialBuiltins/bridgeNotEmptyMap.kt");
+            doTest(fileName);
+        }
+
        @TestMetadata("bridges.kt")
        public void testBridges() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/specialBuiltins/bridges.kt");
@@ -7375,6 +7381,18 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
            doTest(fileName);
        }

+        @TestMetadata("notEmptyListAny.kt")
+        public void testNotEmptyListAny() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/specialBuiltins/notEmptyListAny.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("notEmptyMap.kt")
+        public void testNotEmptyMap() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/specialBuiltins/notEmptyMap.kt");
+            doTest(fileName);
+        }
+
        @TestMetadata("throwable.kt")
        public void testThrowable() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/specialBuiltins/throwable.kt");