KT 3492: Bug in bytecode generation for labeled super call from inner class & Property generation refactoring

2013-04-09 19:45:14 +04:00
parent 8a14087c91
commit 3da3f94b4e
13 changed files with 230 additions and 115 deletions
@@ -181,6 +181,7 @@ public class CodegenUtil {
        return KotlinBuiltIns.getInstance().getAnyType();
    }

+    @NotNull
    public static <T extends CallableMemberDescriptor> T unwrapFakeOverride(T member) {
        while (member.getKind() == CallableMemberDescriptor.Kind.FAKE_OVERRIDE) {
            //noinspection unchecked
@@ -69,6 +69,7 @@ import java.util.*;
 import static org.jetbrains.asm4.Opcodes.*;
 import static org.jetbrains.jet.codegen.AsmUtil.*;
 import static org.jetbrains.jet.codegen.CodegenUtil.*;
+import static org.jetbrains.jet.codegen.CodegenUtil.isInterface;
 import static org.jetbrains.jet.codegen.binding.CodegenBinding.*;
 import static org.jetbrains.jet.lang.resolve.BindingContext.*;
 import static org.jetbrains.jet.lang.resolve.BindingContextUtils.descriptorToDeclaration;
@@ -1694,6 +1695,7 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
        return myFrameMap.getIndex(descriptor);
    }

+    @NotNull
    public StackValue.Property intermediateValueForProperty(
            PropertyDescriptor propertyDescriptor,
            boolean forceField,
@@ -1702,21 +1704,15 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem

        DeclarationDescriptor containingDeclaration = propertyDescriptor.getContainingDeclaration();
        assert containingDeclaration != null;
-        containingDeclaration = containingDeclaration.getOriginal();

        boolean isStatic = containingDeclaration instanceof NamespaceDescriptor;
        boolean isSuper = superExpression != null;
-        boolean overridesTrait = isOverrideForTrait(propertyDescriptor);
-        boolean isFakeOverride = propertyDescriptor.getKind() == CallableMemberDescriptor.Kind.FAKE_OVERRIDE;
-
-        PropertyDescriptor initialDescriptor = propertyDescriptor;
-        propertyDescriptor = initialDescriptor.getOriginal();
        boolean isInsideClass = isCallInsideSameClassAsDeclared(propertyDescriptor, context);
        boolean isInsideModule = isCallInsideSameModuleAsDeclared(propertyDescriptor, context);
        boolean isExtensionProperty = propertyDescriptor.getReceiverParameter() != null;

-        Method getter = null;
-        Method setter = null;
+        CallableMethod callableGetter = null;
+        CallableMethod callableSetter = null;

        if (!forceField) {
            //noinspection ConstantConditions
@@ -1725,76 +1721,40 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
                (propertyDescriptor.getGetter() == null ||
                 !DescriptorUtils.isExternallyAccessible(propertyDescriptor) ||
                 propertyDescriptor.getGetter().isDefault() && propertyDescriptor.getGetter().getModality() == Modality.FINAL)) {
-                getter = null;
+                callableGetter = null;
            }
            else {
-                if (isSuper) {
-                    PsiElement enclosingElement = bindingContext.get(BindingContext.LABEL_TARGET, superExpression.getTargetLabel());
-                    ClassDescriptor enclosed =
-                            (ClassDescriptor) bindingContext.get(BindingContext.DECLARATION_TO_DESCRIPTOR, enclosingElement);
-                    if (!isInterface(containingDeclaration)) {
-                        if (enclosed != null && enclosed != context.getThisDescriptor()) {
-                            CodegenContext c = context;
-                            while (c.getContextDescriptor() != enclosed) {
-                                c = c.getParentContext();
-                            }
-                            propertyDescriptor = (PropertyDescriptor) c.getAccessor(propertyDescriptor);
-                            isSuper = false;
-                        }
+                if (isSuper && !isInterface(containingDeclaration)) {
+                    ClassDescriptor owner = getSuperCallLabelTarget(superExpression);
+                    CodegenContext c = context.findParentContextWithDescriptor(owner);
+                    assert c != null : "Couldn't find a context for a super-call: " + propertyDescriptor;
+                    if (c != context.getParentContext()) {
+                        propertyDescriptor = (PropertyDescriptor) c.getAccessor(propertyDescriptor);
                    }
                }
-                else {
-                    propertyDescriptor = accessablePropertyDescriptor(propertyDescriptor);
-                }
+
+                propertyDescriptor = accessablePropertyDescriptor(propertyDescriptor);

                if (propertyDescriptor.getGetter() != null) {
-                    getter = typeMapper.
-                            mapGetterSignature(propertyDescriptor, OwnerKind.IMPLEMENTATION).
-                            getJvmMethodSignature().getAsmMethod();
-                }
-
-                if (getter == null && isExtensionProperty) {
-                    throw new IllegalStateException();
+                    callableGetter = typeMapper.mapToCallableMethod(propertyDescriptor.getGetter(), isSuper, isInsideClass, isInsideModule, OwnerKind.IMPLEMENTATION);
                }
            }

            if (propertyDescriptor.isVar()) {
                if (propertyDescriptor.getSetter() != null) {
                    if (isInsideClass && !isExtensionProperty &&
-                               (!DescriptorUtils.isExternallyAccessible(propertyDescriptor) ||
-                                propertyDescriptor.getSetter().isDefault() &&
-                                propertyDescriptor.getSetter().getModality() == Modality.FINAL)) {
-                        setter = null;
+                        (!DescriptorUtils.isExternallyAccessible(propertyDescriptor) ||
+                         propertyDescriptor.getSetter().isDefault() &&
+                         propertyDescriptor.getSetter().getModality() == Modality.FINAL)) {
+                        callableSetter = null;
                    }
                    else {
-                        setter = typeMapper.
-                                mapSetterSignature(propertyDescriptor, OwnerKind.IMPLEMENTATION).
-                                getJvmMethodSignature().getAsmMethod();
+                        callableSetter = typeMapper.mapToCallableMethod(propertyDescriptor.getSetter(), isSuper, isInsideClass, isInsideModule, OwnerKind.IMPLEMENTATION);
                    }
                }
-
-                if (setter == null && isExtensionProperty) {
-                    throw new IllegalStateException();
-                }
            }
        }

-        int getterInvokeOpcode = getOpcodeForPropertyDescriptorWithoutAccessor(propertyDescriptor);
-        int setterInvokeOpcode = getterInvokeOpcode;
-
-        CallableMethod callableGetter = null;
-        CallableMethod callableSetter = null;
-
-        if (getter != null) {
-            callableGetter = typeMapper.mapToCallableMethod(propertyDescriptor.getGetter(), isSuper, isInsideClass, isInsideModule, contextKind());
-            getterInvokeOpcode = callableGetter.getInvokeOpcode();
-        }
-
-        if (setter != null) {
-            callableSetter = typeMapper.mapToCallableMethod(propertyDescriptor.getSetter(), isSuper, isInsideClass, isInsideModule, contextKind());
-            setterInvokeOpcode = callableSetter.getInvokeOpcode();
-        }
-
        JvmClassName owner;
        CallableMethod callableMethod = callableGetter != null ? callableGetter : callableSetter;

@@ -1802,32 +1762,11 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
            owner = typeMapper.getOwner(propertyDescriptor, contextKind(), isInsideModule);
        }
        else {
-            owner = isFakeOverride && !overridesTrait && !isInterface(initialDescriptor.getContainingDeclaration())
-                    ? JvmClassName.byType(typeMapper.mapType(
-                    ((ClassDescriptor) initialDescriptor.getContainingDeclaration()).getDefaultType(), JetTypeMapperMode.IMPL))
-                    : callableMethod.getOwner();
+            owner = callableMethod.getOwner();
        }

-        return StackValue.property(propertyDescriptor, owner, asmType(propertyDescriptor.getType()),
-                                   isStatic, getter, setter, getterInvokeOpcode, setterInvokeOpcode, state);
-    }
-
-    private static int getOpcodeForPropertyDescriptorWithoutAccessor(PropertyDescriptor descriptor) {
-        return isOverrideForTrait(descriptor)
-            ? INVOKEINTERFACE
-            : descriptor.getVisibility() == Visibilities.PRIVATE ? INVOKESPECIAL : INVOKEVIRTUAL;
-    }
-
-    private static boolean isOverrideForTrait(CallableMemberDescriptor propertyDescriptor) {
-        if (propertyDescriptor.getKind() == CallableMemberDescriptor.Kind.FAKE_OVERRIDE) {
-            Set<? extends CallableMemberDescriptor> overriddenDescriptors = propertyDescriptor.getOverriddenDescriptors();
-            for (CallableMemberDescriptor descriptor : overriddenDescriptors) {
-                if (isInterface(descriptor.getContainingDeclaration())) {
-                    return true;
-                }
-            }
-        }
-        return false;
+        return StackValue.property(propertyDescriptor, owner, asmType(unwrapFakeOverride(propertyDescriptor).getOriginal().getType()),
+                                   isStatic, callableGetter, callableSetter, state);
    }

    @Override
@@ -1962,7 +1901,7 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
            CodegenContext c = context.findParentContextWithDescriptor(owner);
            assert c != null : "Couldn't find a context for a super-call: " + fd;
            if (c != context.getParentContext()) {
-                fd = (FunctionDescriptor) c.getAccessor(unwrapFakeOverride(fd));
+                fd = (FunctionDescriptor) c.getAccessor(fd);
            }
        }

@@ -1621,7 +1621,7 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen {
                        JvmClassName owner = typeMapper.getOwner(propertyDescriptor, OwnerKind.IMPLEMENTATION, isCallInsideSameModuleAsDeclared(propertyDescriptor, codegen.context));
                        Type propType = typeMapper.mapType(jetType);
                        StackValue.property(propertyDescriptor, owner,
-                                            propType, false, null, null, 0, 0, state).store(propType, iv);
+                                            propType, false, null, null, state).store(propType, iv);
                    }
                }
            }
@@ -139,13 +139,11 @@ public abstract class StackValue {
            JvmClassName methodOwner,
            Type type,
            boolean isStatic,
-            @Nullable Method getter,
-            @Nullable Method setter,
-            int getterInvokeOpcode,
-            int setterInvokeOpcode,
+            @Nullable CallableMethod getter,
+            @Nullable CallableMethod setter,
            GenerationState state
    ) {
-        return new Property(descriptor, methodOwner, getter, setter, isStatic, type, getterInvokeOpcode, setterInvokeOpcode,
+        return new Property(descriptor, methodOwner, getter, setter, isStatic, type,
                            state);
    }

@@ -858,14 +856,12 @@ public abstract class StackValue {

    static class Property extends StackValueWithSimpleReceiver {
        @Nullable
-        private final Method getter;
+        private final CallableMethod getter;
        @Nullable
-        private final Method setter;
+        private final CallableMethod setter;
        @NotNull
        public final JvmClassName methodOwner;

-        private final int getterInvokeOpcode;
-        private final int setterInvokeOpcode;
        @NotNull
        private final PropertyDescriptor descriptor;
        @NotNull
@@ -873,23 +869,15 @@ public abstract class StackValue {

        public Property(
                @NotNull PropertyDescriptor descriptor, @NotNull JvmClassName methodOwner,
-                @Nullable Method getter, @Nullable Method setter, boolean isStatic,
-                @NotNull Type type, int getterInvokeOpcode, int setterInvokeOpcode, @NotNull GenerationState state
+                @Nullable CallableMethod getter, @Nullable CallableMethod setter, boolean isStatic,
+                @NotNull Type type, @NotNull GenerationState state
        ) {
            super(type, isStatic);
            this.methodOwner = methodOwner;
            this.getter = getter;
            this.setter = setter;
-            this.getterInvokeOpcode = getterInvokeOpcode;
-            this.setterInvokeOpcode = setterInvokeOpcode;
            this.descriptor = descriptor;
            this.state = state;
-            if (getterInvokeOpcode == 0 && getter != null) {
-                throw new IllegalArgumentException();
-            }
-            if (setterInvokeOpcode == 0 && setter != null) {
-                throw new IllegalArgumentException();
-            }
        }

        @Override
@@ -900,7 +888,8 @@ public abstract class StackValue {
                genNotNullAssertionForField(v, state, descriptor);
            }
            else {
-                v.visitMethodInsn(getterInvokeOpcode, methodOwner.getInternalName(), getter.getName(), getter.getDescriptor());
+                Method method = getter.getSignature().getAsmMethod();
+                v.visitMethodInsn(getter.getInvokeOpcode(), methodOwner.getInternalName(), method.getName(), method.getDescriptor());
            }
            coerceTo(type, v);
        }
@@ -912,7 +901,8 @@ public abstract class StackValue {
                v.visitFieldInsn(isStatic ? PUTSTATIC : PUTFIELD, methodOwner.getInternalName(), descriptor.getName().getName(),
                                 this.type.getDescriptor()); }
            else {
-                v.visitMethodInsn(setterInvokeOpcode, methodOwner.getInternalName(), setter.getName(), setter.getDescriptor());
+                Method method = setter.getSignature().getAsmMethod();
+                v.visitMethodInsn(setter.getInvokeOpcode(), methodOwner.getInternalName(), method.getName(), method.getDescriptor());
            }
        }
    }
@@ -1244,7 +1234,7 @@ public abstract class StackValue {
        }
    }

-    abstract static class StackValueWithSimpleReceiver extends StackValue {
+    private abstract static class StackValueWithSimpleReceiver extends StackValue {

        protected final boolean isStatic;

@@ -446,6 +446,7 @@ public class JetTypeMapper extends BindingTraceAware {
        }
    }

+    @NotNull
    public CallableMethod mapToCallableMethod(
            @NotNull FunctionDescriptor functionDescriptor,
            boolean superCall,
@@ -596,7 +597,13 @@ public class JetTypeMapper extends BindingTraceAware {
            mapReturnType(returnType, signatureVisitor);
            signatureVisitor.writeReturnTypeEnd();
        }
-        return signatureVisitor.makeJvmMethodSignature(f.getName().getName());
+
+        String name = f.getName().getName();
+        if (f instanceof PropertyAccessorDescriptor) {
+            boolean isGetter = f instanceof PropertyGetterDescriptor;
+            name = getPropertyAccessorName(((PropertyAccessorDescriptor)f).getCorrespondingProperty(), isGetter);
+        }
+        return signatureVisitor.makeJvmMethodSignature(name);
    }

    private void writeThisIfNeeded(
@@ -716,12 +723,18 @@ public class JetTypeMapper extends BindingTraceAware {
        }
    }

-
-    public JvmPropertyAccessorSignature mapGetterSignature(PropertyDescriptor descriptor, OwnerKind kind) {
+    @NotNull
+    public static String getPropertyAccessorName(@NotNull PropertyDescriptor descriptor, boolean isGetter) {
        DeclarationDescriptor parentDescriptor = descriptor.getContainingDeclaration();
        boolean isAnnotation = parentDescriptor instanceof ClassDescriptor &&
                               ((ClassDescriptor) parentDescriptor).getKind() == ClassKind.ANNOTATION_CLASS;
-        String name = isAnnotation ? descriptor.getName().getName() : PropertyCodegen.getterName(descriptor.getName());
+        return isAnnotation ? descriptor.getName().getName() :
+               isGetter ? PropertyCodegen.getterName(descriptor.getName()) : PropertyCodegen.setterName(descriptor.getName());
+    }
+
+    @NotNull
+    public JvmPropertyAccessorSignature mapGetterSignature(PropertyDescriptor descriptor, OwnerKind kind) {
+        String name = getPropertyAccessorName(descriptor, true);

        // TODO: do not genClassOrObject generics if not needed
        BothSignatureWriter signatureWriter = new BothSignatureWriter(BothSignatureWriter.Mode.METHOD, true);
@@ -742,6 +755,7 @@ public class JetTypeMapper extends BindingTraceAware {
        return new JvmPropertyAccessorSignature(jvmMethodSignature, jvmMethodSignature.getKotlinReturnType());
    }

+
    @NotNull
    public JvmPropertyAccessorSignature mapSetterSignature(PropertyDescriptor descriptor, OwnerKind kind) {
        assert descriptor.isVar();
@@ -759,7 +773,7 @@ public class JetTypeMapper extends BindingTraceAware {

        signatureWriter.writeVoidReturn();

-        String name = PropertyCodegen.setterName(descriptor.getName());
+        String name = getPropertyAccessorName(descriptor, false);
        JvmMethodSignature jvmMethodSignature = signatureWriter.makeJvmMethodSignature(name);
        return new JvmPropertyAccessorSignature(jvmMethodSignature,
                                                jvmMethodSignature.getKotlinParameterType(jvmMethodSignature.getParameterCount() - 1));
@@ -64,7 +64,7 @@ public class PropertyDescriptorImpl extends VariableDescriptorImpl implements Pr
        this.isVar = isVar;
        this.modality = modality;
        this.visibility = visibility;
-        this.original = original == null ? this : original.getOriginal();
+        this.original = original == null ? this : original;
        this.kind = kind;
    }

@@ -205,7 +205,7 @@ public class PropertyDescriptorImpl extends VariableDescriptorImpl implements Pr

    private PropertyDescriptor doSubstitute(TypeSubstitutor originalSubstitutor,
            DeclarationDescriptor newOwner, Modality newModality, Visibility newVisibility, boolean preserveOriginal, boolean copyOverrides, Kind kind) {
-        PropertyDescriptorImpl substitutedDescriptor = new PropertyDescriptorImpl(preserveOriginal ? getOriginal() : this, newOwner,
+        PropertyDescriptorImpl substitutedDescriptor = new PropertyDescriptorImpl(preserveOriginal ? getOriginal() : null, newOwner,
                getAnnotations(), newModality, newVisibility, isVar(), getName(), kind);

        List<TypeParameterDescriptor> substitutedTypeParameters = Lists.newArrayList();
@@ -290,7 +290,7 @@ public class PropertyDescriptorImpl extends VariableDescriptorImpl implements Pr
    @NotNull
    @Override
    public PropertyDescriptor getOriginal() {
-        return original;
+        return original == this ? this : original.getOriginal();
    }

    @Override
@@ -0,0 +1,44 @@
+//inspired by kt3492
+trait BK {
+    fun foo(): String
+    fun bar(): String
+}
+
+trait KTrait: BK {
+    override fun foo() = bar()
+}
+
+open abstract class K : KTrait {
+
+}
+
+class A : K() {
+    override fun foo() = "A.foo"
+    override fun bar() = "A.bar"
+
+    inner class B : K() {
+        override fun foo() = "B.foo"
+        override fun bar() = "B.bar"
+
+        fun test1() = super<K>@A.foo()
+        fun test2() = super<K>@B.foo()
+        fun test3() = super<K>.foo()
+        fun test4() = super@A.foo()
+        fun test5() = super@B.foo()
+        fun test6() = super.foo()
+    }
+}
+
+
+fun box(): String {
+    val b = A().B()
+    if (b.test1() != "A.bar") return "test1 ${b.test1()}"
+    if (b.test2() != "B.bar") return "test2 ${b.test2()}"
+    if (b.test3() != "B.bar") return "test3 ${b.test3()}"
+    if (b.test4() != "A.bar") return "test4 ${b.test4()}"
+    if (b.test5() != "B.bar") return "test5 ${b.test5()}"
+    if (b.test6() != "B.bar") return "test6 ${b.test6()}"
+
+    return "OK"
+}
+
@@ -0,0 +1,43 @@
+//inspired by kt3492
+trait Base {
+    val foo: String
+    fun bar(): String
+}
+
+abstract class KWithOverride : Base {
+    override val foo = bar()
+}
+
+abstract class K : KWithOverride() {
+
+}
+
+class A : K() {
+    override val foo = "A.foo"
+    override fun bar() = "A.bar"
+
+    inner class B : K() {
+        override val foo = "B.foo"
+        override fun bar() = "B.bar"
+
+        fun test1() = super<K>@A.foo
+        fun test2() = super<K>@B.foo
+        fun test3() = super<K>.foo
+        fun test4() = super@A.foo
+        fun test5() = super@B.foo
+        fun test6() = super.foo
+    }
+}
+
+
+fun box(): String {
+    val b = A().B()
+    if (b.test1() != "A.bar") return "test1 ${b.test1()}"
+    if (b.test2() != "B.bar") return "test2 ${b.test2()}"
+    if (b.test3() != "B.bar") return "test3 ${b.test3()}"
+    if (b.test4() != "A.bar") return "test4 ${b.test4()}"
+    if (b.test5() != "B.bar") return "test5 ${b.test5()}"
+    if (b.test6() != "B.bar") return "test6 ${b.test6()}"
+
+    return "OK"
+}
@@ -0,0 +1,19 @@
+open class A {
+    open fun foo2(): String = "OK"
+}
+
+open class B : A() {
+
+}
+
+class C : B() {
+    inner class D {
+        val foo: String = super<B>@C.foo2()
+    }
+}
+
+fun box() : String {
+    val obj = C().D();
+    return obj.foo
+}
+
@@ -0,0 +1,15 @@
+open class A {
+    open val foo: String = "OK"
+}
+
+open class B : A() {
+
+}
+
+class C : B() {
+    inner class D {
+        val foo: String = super<B>@C.foo
+    }
+}
+
+fun box() = C().D().foo
@@ -0,0 +1,19 @@
+trait ATrait {
+    open fun foo2(): String = "OK"
+}
+
+open class B : ATrait {
+
+}
+
+class C : B() {
+    inner class D {
+        val foo: String = super<B>@C.foo2()
+    }
+}
+
+fun box() : String {
+    val obj = C().D();
+    return obj.foo
+}
+
@@ -0,0 +1,16 @@
+trait A {
+    open val foo: String
+      get() = "OK"
+}
+
+open class B : A {
+
+}
+
+class C : B() {
+    inner class D {
+        val foo: String = super<B>@C.foo
+    }
+}
+
+fun box() = C().D().foo
@@ -3300,6 +3300,21 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
            doTest("compiler/testData/codegen/box/super/innerClassLabeledSuperProperty.kt");
        }
        
+        @TestMetadata("kt3492ClassProperty.kt")
+        public void testKt3492ClassProperty() throws Exception {
+            doTest("compiler/testData/codegen/box/super/kt3492ClassProperty.kt");
+        }
+        
+        @TestMetadata("kt3492TraitFun.kt")
+        public void testKt3492TraitFun() throws Exception {
+            doTest("compiler/testData/codegen/box/super/kt3492TraitFun.kt");
+        }
+        
+        @TestMetadata("kt3492TraitProperty.kt")
+        public void testKt3492TraitProperty() throws Exception {
+            doTest("compiler/testData/codegen/box/super/kt3492TraitProperty.kt");
+        }
+        
        @TestMetadata("multipleSuperTraits.kt")
        public void testMultipleSuperTraits() throws Exception {
            doTest("compiler/testData/codegen/box/super/multipleSuperTraits.kt");