Wrong instruction for invoke private function

#KT-2202 Fixed
2012-11-02 11:49:10 +04:00
parent a9c567d19b
commit 91e83c2860
12 changed files with 191 additions and 50 deletions
@@ -120,8 +120,6 @@ public class SpecialFiles {

        excludedFiles.add("kt684.jet"); // StackOverflow with StringBuilder (escape())

-        excludedFiles.add("kt1199.kt"); // Bug KT-2202
-        excludedFiles.add("kt1779.kt"); // Bug KT-2202 - private fun tryToComputeNext() in AbstractIterator.kt
        excludedFiles.add("kt344.jet"); // Bug KT-2251
        excludedFiles.add("kt529.kt");  // Bug

@@ -22,6 +22,8 @@ import com.intellij.util.containers.Stack;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.jet.codegen.binding.CalculatedClosure;
+import org.jetbrains.jet.codegen.context.CodegenContext;
+import org.jetbrains.jet.codegen.context.NamespaceContext;
 import org.jetbrains.jet.codegen.signature.BothSignatureWriter;
 import org.jetbrains.jet.codegen.signature.JvmMethodParameterKind;
 import org.jetbrains.jet.codegen.signature.JvmMethodSignature;
@@ -252,4 +254,17 @@ public class CodegenUtil {
                .addAll(innerClassesScope.getObjectDescriptors())
                .build();
    }
+
+    public static boolean isCallInsideSameClassAsDeclared(CallableMemberDescriptor declarationDescriptor, CodegenContext context) {
+        boolean isFakeOverride = declarationDescriptor.getKind() == CallableMemberDescriptor.Kind.FAKE_OVERRIDE;
+        boolean isDelegate = declarationDescriptor.getKind() == CallableMemberDescriptor.Kind.DELEGATION;
+
+        DeclarationDescriptor containingDeclaration = declarationDescriptor.getContainingDeclaration();
+        containingDeclaration = containingDeclaration.getOriginal();
+
+        return !isFakeOverride && !isDelegate &&
+               (((context.hasThisDescriptor() && containingDeclaration == context.getThisDescriptor()) ||
+                 (context.getParentContext() instanceof NamespaceContext && context.getParentContext().getContextDescriptor() == containingDeclaration))
+                && context.getContextKind() != OwnerKind.TRAIT_IMPL);
+    }
 }
@@ -1521,16 +1521,9 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
        boolean isStatic = containingDeclaration instanceof NamespaceDescriptor;
        boolean overridesTrait = isOverrideForTrait(propertyDescriptor);
        boolean isFakeOverride = propertyDescriptor.getKind() == CallableMemberDescriptor.Kind.FAKE_OVERRIDE;
-        boolean isDelegate = propertyDescriptor.getKind() == CallableMemberDescriptor.Kind.DELEGATION;
        PropertyDescriptor initialDescriptor = propertyDescriptor;
        propertyDescriptor = initialDescriptor.getOriginal();
-        boolean isInsideClass = !isFakeOverride &&
-                                !isDelegate &&
-                                (((containingDeclaration == null && !context.hasThisDescriptor() ||
-                                   context.hasThisDescriptor() && containingDeclaration == context.getThisDescriptor()) ||
-                                  (context.getParentContext() instanceof NamespaceContext) &&
-                                  context.getParentContext().getContextDescriptor() == containingDeclaration)
-                                 && contextKind() != OwnerKind.TRAIT_IMPL);
+        boolean isInsideClass = isCallInsideSameClassAsDeclared(propertyDescriptor, context);
        Method getter = null;
        Method setter = null;
        if (!forceField) {
@@ -1594,32 +1587,60 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
            }
        }

-        int invokeOpcode;
+        int getterInvokeOpcode;
+        int setterInvokeOpcode;

        JvmClassName owner;
        JvmClassName ownerParam;
        boolean isInterface;
-        if (isInsideClass || isStatic || propertyDescriptor.getGetter() == null) {
-            owner = ownerParam = typeMapper.getOwner(propertyDescriptor, contextKind());
+        if (isStatic) {
            isInterface = overridesTrait;
-            invokeOpcode = isStatic ? INVOKESTATIC :
-                           overridesTrait ? INVOKEINTERFACE
-                                          : INVOKEVIRTUAL;
+            owner = ownerParam = typeMapper.getOwner(propertyDescriptor, contextKind());
+
+            getterInvokeOpcode = INVOKESTATIC;
+            setterInvokeOpcode = INVOKESTATIC;
        }
        else {
            isInterface = isInterface(containingDeclaration) || overridesTrait;
-            // TODO ugly
-            CallableMethod callableMethod = typeMapper.mapToCallableMethod(propertyDescriptor.getGetter(), isSuper, contextKind());
-            invokeOpcode = callableMethod.getInvokeOpcode();
-            owner = isFakeOverride && !overridesTrait && !isInterface(initialDescriptor.getContainingDeclaration())
-                    ? JvmClassName.byType(typeMapper.mapType(
-                    ((ClassDescriptor) initialDescriptor.getContainingDeclaration()).getDefaultType(), JetTypeMapperMode.IMPL))
-                    : callableMethod.getOwner();
-            ownerParam = callableMethod.getDefaultImplParam();
+            CallableMethod callableGetter = null;
+            CallableMethod callableSetter = null;
+            if (getter == null) {
+                getterInvokeOpcode = getOpcodeForPropertyDescriptorWithoutAccessor(propertyDescriptor);
+            }
+            else {
+                callableGetter = typeMapper.mapToCallableMethod(propertyDescriptor.getGetter(), isSuper, isInsideClass, contextKind());
+                getterInvokeOpcode = callableGetter.getInvokeOpcode();
+            }
+
+            if (setter == null) {
+                setterInvokeOpcode = getOpcodeForPropertyDescriptorWithoutAccessor(propertyDescriptor);
+            }
+            else {
+                callableSetter = typeMapper.mapToCallableMethod(propertyDescriptor.getSetter(), isSuper, isInsideClass, contextKind());
+                setterInvokeOpcode = callableSetter.getInvokeOpcode();
+            }
+
+            CallableMethod callableMethod = callableGetter != null ? callableGetter : callableSetter;
+            if (callableMethod == null) {
+                owner = ownerParam = typeMapper.getOwner(propertyDescriptor, contextKind());
+            }
+            else {
+                owner = isFakeOverride && !overridesTrait && !isInterface(initialDescriptor.getContainingDeclaration())
+                        ? JvmClassName.byType(typeMapper.mapType(
+                        ((ClassDescriptor) initialDescriptor.getContainingDeclaration()).getDefaultType(), JetTypeMapperMode.IMPL))
+                        : callableMethod.getOwner();
+                ownerParam = callableMethod.getDefaultImplParam();
+            }
        }

        return StackValue.property(propertyDescriptor, owner, ownerParam, asmType(propertyDescriptor.getType()),
-                                   isStatic, isInterface, isSuper, getter, setter, invokeOpcode, state);
+                                   isStatic, isInterface, isSuper, 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) {
@@ -1807,7 +1828,7 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
            callableMethod = typeMapper.asCallableMethod(invoke);
        }
        else {
-            callableMethod = typeMapper.mapToCallableMethod(fd, superCall, OwnerKind.IMPLEMENTATION);
+            callableMethod = typeMapper.mapToCallableMethod(fd, superCall, isCallInsideSameClassAsDeclared(fd, context), OwnerKind.IMPLEMENTATION);
        }
        return callableMethod;
    }
@@ -2899,7 +2920,11 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
        Type type = asmType(javaClass.getDefaultType());
        v.anew(type);
        v.dup();
-        final CallableMethod callableMethod = typeMapper.mapToCallableMethod(descriptor, false, OwnerKind.IMPLEMENTATION);
+        final CallableMethod callableMethod = typeMapper.mapToCallableMethod(
+                descriptor,
+                false,
+                isCallInsideSameClassAsDeclared(descriptor, context),
+                OwnerKind.IMPLEMENTATION);
        invokeMethodWithArguments(callableMethod, expression, StackValue.none());
        return type;
    }
@@ -2997,7 +3022,11 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
            }
        }
        else {
-            CallableMethod accessor = typeMapper.mapToCallableMethod(operationDescriptor, false, OwnerKind.IMPLEMENTATION);
+            CallableMethod accessor = typeMapper.mapToCallableMethod(
+                    operationDescriptor,
+                    false,
+                    isCallInsideSameClassAsDeclared(operationDescriptor, context),
+                    OwnerKind.IMPLEMENTATION);

            boolean isGetter = accessor.getSignature().getAsmMethod().getName().equals("get");

@@ -77,8 +77,11 @@ public class FunctionCodegen extends GenerationStateAware {
        final SimpleFunctionDescriptor functionDescriptor = bindingContext.get(BindingContext.FUNCTION, f);
        assert functionDescriptor != null;
        JvmMethodSignature method =
-                typeMapper.mapToCallableMethod(functionDescriptor, false, owner.getContextKind())
-                        .getSignature();
+                typeMapper.mapToCallableMethod(
+                        functionDescriptor,
+                        false,
+                        isCallInsideSameClassAsDeclared(functionDescriptor, owner),
+                        owner.getContextKind()).getSignature();
        generateMethod(f, method, true, null, functionDescriptor);
    }

@@ -391,7 +394,7 @@ public class FunctionCodegen extends GenerationStateAware {
            MethodVisitor mv
    ) {
        if (jvmSignature == null) {
-            jvmSignature = state.getTypeMapper().mapToCallableMethod(functionDescriptor, false, OwnerKind.IMPLEMENTATION).getSignature();
+            jvmSignature = state.getTypeMapper().mapToCallableMethod(functionDescriptor, false, false, OwnerKind.IMPLEMENTATION).getSignature();
        }

        List<ValueParameterDescriptor> paramDescrs = functionDescriptor.getValueParameters();
@@ -1318,8 +1318,11 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen {
                final MethodVisitor mv = v.newMethod(myClass, flags, function.getName(), function.getDescriptor(), null, null);
                AnnotationCodegen.forMethod(mv, state.getTypeMapper()).genAnnotations(fun);

-                JvmMethodSignature jvmSignature =
-                        typeMapper.mapToCallableMethod(inheritedFun, false, OwnerKind.IMPLEMENTATION).getSignature();
+                JvmMethodSignature jvmSignature = typeMapper.mapToCallableMethod(
+                        inheritedFun,
+                        false,
+                        isCallInsideSameClassAsDeclared(inheritedFun, context),
+                        OwnerKind.IMPLEMENTATION).getSignature();
                JetMethodAnnotationWriter aw = JetMethodAnnotationWriter.visitAnnotation(mv);
                int kotlinFlags = getFlagsForVisibility(fun.getVisibility());
                if (fun instanceof PropertyAccessorDescriptor) {
@@ -1556,7 +1559,7 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen {
                        JvmClassName owner = typeMapper.getOwner(propertyDescriptor, OwnerKind.IMPLEMENTATION);
                        Type propType = typeMapper.mapType(jetType);
                        StackValue.property(propertyDescriptor, owner, owner,
-                                            propType, false, false, false, null, null, 0, state).store(propType, iv);
+                                            propType, false, false, false, null, null, 0, 0, state).store(propType, iv);
                    }
                }
            }
@@ -161,10 +161,11 @@ public abstract class StackValue {
            boolean isSuper,
            @Nullable Method getter,
            @Nullable Method setter,
-            int invokeOpcode,
+            int getterInvokeOpcode,
+            int setterInvokeOpcode,
            GenerationState state
    ) {
-        return new Property(descriptor, methodOwner, methodOwnerParam, getter, setter, isStatic, isInterface, isSuper, type, invokeOpcode,
+        return new Property(descriptor, methodOwner, methodOwnerParam, getter, setter, isStatic, isInterface, isSuper, type, getterInvokeOpcode, setterInvokeOpcode,
                            state);
    }

@@ -940,7 +941,8 @@ public abstract class StackValue {
        private final boolean isStatic;
        private final boolean isInterface;
        private final boolean isSuper;
-        private final int invokeOpcode;
+        private final int getterInvokeOpcode;
+        private final int setterInvokeOpcode;
        @NotNull
        private final PropertyDescriptor descriptor;
        @NotNull
@@ -949,7 +951,7 @@ public abstract class StackValue {
        public Property(
                @NotNull PropertyDescriptor descriptor, @NotNull JvmClassName methodOwner, @NotNull JvmClassName methodOwnerParam,
                @Nullable Method getter, @Nullable Method setter, boolean isStatic, boolean isInterface, boolean isSuper,
-                @NotNull Type type, int invokeOpcode, @NotNull GenerationState state
+                @NotNull Type type, int getterInvokeOpcode, int setterInvokeOpcode, @NotNull GenerationState state
        ) {
            super(type);
            this.methodOwner = methodOwner;
@@ -959,13 +961,15 @@ public abstract class StackValue {
            this.isStatic = isStatic;
            this.isInterface = isInterface;
            this.isSuper = isSuper;
-            this.invokeOpcode = invokeOpcode;
+            this.getterInvokeOpcode = getterInvokeOpcode;
+            this.setterInvokeOpcode = setterInvokeOpcode;
            this.descriptor = descriptor;
            this.state = state;
-            if (invokeOpcode == 0) {
-                if (setter != null || getter != null) {
-                    throw new IllegalArgumentException();
-                }
+            if (getterInvokeOpcode == 0 && getter != null) {
+                throw new IllegalArgumentException();
+            }
+            if (setterInvokeOpcode == 0 && setter != null) {
+                throw new IllegalArgumentException();
            }
        }

@@ -983,7 +987,7 @@ public abstract class StackValue {
                    genNotNullAssertionForField(v, state, descriptor);
                }
                else {
-                    v.visitMethodInsn(invokeOpcode, methodOwner.getInternalName(), getter.getName(), getter.getDescriptor());
+                    v.visitMethodInsn(getterInvokeOpcode, methodOwner.getInternalName(), getter.getName(), getter.getDescriptor());
                }
            }
            coerceTo(type, v);
@@ -1002,7 +1006,7 @@ public abstract class StackValue {
                                 this.type.getDescriptor());
            }
            else {
-                v.visitMethodInsn(invokeOpcode, methodOwner.getInternalName(), setter.getName(), setter.getDescriptor());
+                v.visitMethodInsn(setterInvokeOpcode, methodOwner.getInternalName(), setter.getName(), setter.getDescriptor());
            }
        }

@@ -50,7 +50,7 @@ public class PsiMethodCall implements IntrinsicMethod {
            List<JetExpression> arguments, StackValue receiver, @NotNull GenerationState state
    ) {
        final CallableMethod callableMethod =
-                state.getTypeMapper().mapToCallableMethod(method, false, OwnerKind.IMPLEMENTATION);
+                state.getTypeMapper().mapToCallableMethod(method, false, false, OwnerKind.IMPLEMENTATION);
        if(element instanceof JetBinaryExpression) {
            codegen.invokeMethodWithArguments(callableMethod, receiver, ((JetBinaryExpression)element).getOperationReference());
        }  else {
@@ -385,7 +385,12 @@ public class JetTypeMapper extends BindingTraceAware {
        }
    }

-    public CallableMethod mapToCallableMethod(@NotNull FunctionDescriptor functionDescriptor, boolean superCall, OwnerKind kind) {
+    public CallableMethod mapToCallableMethod(
+            @NotNull FunctionDescriptor functionDescriptor,
+            boolean superCall,
+            boolean isInsideClass,
+            OwnerKind kind
+    ) {
        final DeclarationDescriptor functionParent = functionDescriptor.getOriginal().getContainingDeclaration();

        functionDescriptor = unwrapFakeOverride(functionDescriptor);
@@ -444,10 +449,19 @@ public class JetTypeMapper extends BindingTraceAware {
            ownerForDefaultParam = JvmClassName.byType(mapType(declarationOwner.getDefaultType(), JetTypeMapperMode.TYPE_PARAMETER));
            ownerForDefaultImpl = JvmClassName.byInternalName(
                    ownerForDefaultParam.getInternalName() + (originalIsInterface ? JvmAbi.TRAIT_IMPL_SUFFIX : ""));
+            if (isInterface) {
+                invokeOpcode = superCall ? INVOKESTATIC : INVOKEINTERFACE;
+            }
+            else {
+                if (isAccessor) {
+                    invokeOpcode = INVOKESTATIC;
+                }
+                else {
+                    boolean isPrivateFunInvocation = isInsideClass && functionDescriptor.getVisibility() == Visibilities.PRIVATE;
+                    invokeOpcode = superCall || isPrivateFunInvocation ? INVOKESPECIAL : INVOKEVIRTUAL;
+                }
+            }

-            invokeOpcode = isInterface
-                           ? (superCall ? INVOKESTATIC : INVOKEINTERFACE)
-                           : (isAccessor ? INVOKESTATIC : (superCall ? INVOKESPECIAL : INVOKEVIRTUAL));
            if (isInterface && superCall) {
                descriptor = mapSignature(functionDescriptor, false, OwnerKind.TRAIT_IMPL);
                owner = JvmClassName.byInternalName(owner.getInternalName() + JvmAbi.TRAIT_IMPL_SUFFIX);
@@ -0,0 +1,45 @@
+// KT-2202 Wrong instruction for invoke private setter
+
+class A {
+    private fun f1() { }
+    fun foo() {
+        f1()
+    }
+}
+
+class B {
+    private val foo = 1
+        get
+
+    fun foo() {
+        foo
+    }
+}
+
+class C {
+    private var foo = 1
+        get
+        set
+
+    fun foo() {
+        foo = 2
+        foo
+    }
+}
+
+class D {
+    var foo = 1
+        private set
+
+    fun foo() {
+        foo = 2
+    }
+}
+
+fun box(): String {
+   A().foo()
+   B().foo()
+   C().foo()
+   D().foo()
+   return "OK"
+}
@@ -0,0 +1,18 @@
+// KT-2202 Wrong instruction for invoke private setter
+
+class A {
+    private fun f1() {}
+
+    fun foo() {
+        f1()
+    }
+}
+
+class B {
+    var foo = 1
+        private set
+
+    fun foo() {
+       foo = 2
+    }
+}
@@ -549,4 +549,8 @@ public class NamespaceGenTest extends CodegenTestCase {
    public void testPrivateTopLevelPropAndVarInInner() {
        blackBoxFile("privateTopLevelPropAndVarInInner.kt");
    }
+
+    public void testInvokeSpecial() {
+        blackBoxFile("invokespecial.kt");
+    }
 }
@@ -376,4 +376,12 @@ public class PropertyGenTest extends CodegenTestCase {
        blackBoxFile("properties/accessToPrivateSetter.kt");
    }

+    public void testKt2202() throws Exception {
+        createEnvironmentWithMockJdkAndIdeaAnnotations(ConfigurationKind.JDK_ONLY);
+        loadFile("properties/kt2202.kt");
+        String text = generateToText();
+        assertFalse(text.contains("INVOKEVIRTUAL"));
+        assertTrue(text.contains("INVOKESPECIAL"));
+    }
+
 }