KT-9717, KT-9603:

pass getter/setter-related flags to AccessorForPropertyDescriptor
2015-10-26 13:48:55 +03:00
parent e83315f5bc
commit 882827bf04
14 changed files with 291 additions and 43 deletions
@@ -35,15 +35,20 @@ public class AccessorForPropertyDescriptor extends PropertyDescriptorImpl implem
    private final PropertyDescriptor calleeDescriptor;
    private final KtSuperExpression superCallExpression;
    @NotNull private final String nameSuffix;
+    private final boolean withSyntheticGetterAccessor;
+    private final boolean withSyntheticSetterAccessor;

    public AccessorForPropertyDescriptor(
            @NotNull PropertyDescriptor property,
            @NotNull DeclarationDescriptor containingDeclaration,
            @Nullable KtSuperExpression superCallExpression,
-            @NotNull String nameSuffix
+            @NotNull String nameSuffix,
+            boolean getterAccessorRequired,
+            boolean setterAccessorRequired
    ) {
        this(property, property.getType(), DescriptorUtils.getReceiverParameterType(property.getExtensionReceiverParameter()),
-             property.getDispatchReceiverParameter(), containingDeclaration, superCallExpression, nameSuffix);
+             property.getDispatchReceiverParameter(), containingDeclaration, superCallExpression, nameSuffix,
+             getterAccessorRequired, setterAccessorRequired);
    }

    protected AccessorForPropertyDescriptor(
@@ -54,6 +59,20 @@ public class AccessorForPropertyDescriptor extends PropertyDescriptorImpl implem
            @NotNull DeclarationDescriptor containingDeclaration,
            @Nullable KtSuperExpression superCallExpression,
            @NotNull String nameSuffix
+    ) {
+        this(original, propertyType, receiverType, dispatchReceiverParameter, containingDeclaration, superCallExpression, nameSuffix, true, true);
+    }
+
+    protected AccessorForPropertyDescriptor(
+            @NotNull PropertyDescriptor original,
+            @NotNull KotlinType propertyType,
+            @Nullable KotlinType receiverType,
+            @Nullable ReceiverParameterDescriptor dispatchReceiverParameter,
+            @NotNull DeclarationDescriptor containingDeclaration,
+            @Nullable KtSuperExpression superCallExpression,
+            @NotNull String nameSuffix,
+            boolean getterAccessorRequired,
+            boolean setterAccessorRequired
    ) {
        super(containingDeclaration, null, Annotations.Companion.getEMPTY(), Modality.FINAL, Visibilities.LOCAL,
              original.isVar(), Name.identifier("access$" + nameSuffix),
@@ -63,7 +82,15 @@ public class AccessorForPropertyDescriptor extends PropertyDescriptorImpl implem
        this.superCallExpression = superCallExpression;
        this.nameSuffix = nameSuffix;
        setType(propertyType, Collections.<TypeParameterDescriptorImpl>emptyList(), dispatchReceiverParameter, receiverType);
-        initialize(new Getter(this), new Setter(this));
+
+        this.withSyntheticGetterAccessor = getterAccessorRequired;
+        this.withSyntheticSetterAccessor = setterAccessorRequired;
+
+        PropertyGetterDescriptorImpl getterDescriptor =
+                getterAccessorRequired ? new Getter(this) : (PropertyGetterDescriptorImpl) original.getGetter();
+        PropertySetterDescriptor setterDescriptor =
+                setterAccessorRequired ? new Setter(this) : original.getSetter();
+        initialize(getterDescriptor, setterDescriptor);
    }

    public static class Getter extends PropertyGetterDescriptorImpl implements AccessorForCallableDescriptor<PropertyGetterDescriptor> {
@@ -128,4 +155,12 @@ public class AccessorForPropertyDescriptor extends PropertyDescriptorImpl implem
    public String getAccessorSuffix() {
        return nameSuffix;
    }
+
+    public boolean isWithSyntheticGetterAccessor() {
+        return withSyntheticGetterAccessor;
+    }
+
+    public boolean isWithSyntheticSetterAccessor() {
+        return withSyntheticSetterAccessor;
+    }
 }
@@ -665,13 +665,14 @@ public abstract class MemberCodegen<T extends KtElement/* TODO: & JetDeclaration
                }
            }

-            PropertyGetterDescriptor getter = accessor.getGetter();
-            assert getter != null;
-            functionCodegen.generateMethod(Synthetic(null, original.getGetter() != null ? original.getGetter() : original),
-                                           getter, new PropertyAccessorStrategy(getter));
+            if (accessor.isWithSyntheticGetterAccessor()) {
+                PropertyGetterDescriptor getter = accessor.getGetter();
+                assert getter != null;
+                functionCodegen.generateMethod(Synthetic(null, original.getGetter() != null ? original.getGetter() : original),
+                                               getter, new PropertyAccessorStrategy(getter));
+            }

-
-            if (accessor.isVar()) {
+            if (accessor.isVar() && accessor.isWithSyntheticSetterAccessor()) {
                PropertySetterDescriptor setter = accessor.getSetter();
                assert setter != null;

@@ -51,12 +51,16 @@ public abstract class CodegenContext<T extends DeclarationDescriptor> {

    private Map<DeclarationDescriptor, CodegenContext> childContexts;
    private Map<AccessorKey, AccessorForCallableDescriptor<?>> accessors;
+    private Map<AccessorKey, AccessorForPropertyDescriptorFactory> propertyAccessorFactories;

    private static class AccessorKey {
        public final DeclarationDescriptor descriptor;
        public final ClassDescriptor superCallLabelTarget;

-        public AccessorKey(@NotNull DeclarationDescriptor descriptor, @Nullable ClassDescriptor superCallLabelTarget) {
+        public AccessorKey(
+                @NotNull DeclarationDescriptor descriptor,
+                @Nullable ClassDescriptor superCallLabelTarget
+        ) {
            this.descriptor = descriptor;
            this.superCallLabelTarget = superCallLabelTarget;
        }
@@ -66,9 +70,8 @@ public abstract class CodegenContext<T extends DeclarationDescriptor> {
            if (!(obj instanceof AccessorKey)) return false;
            AccessorKey other = (AccessorKey) obj;
            return descriptor.equals(other.descriptor) &&
-                   (superCallLabelTarget == null
-                    ? other.superCallLabelTarget == null
-                    : superCallLabelTarget.equals(other.superCallLabelTarget));
+                   (superCallLabelTarget == null ? other.superCallLabelTarget == null
+                                                 : superCallLabelTarget.equals(other.superCallLabelTarget));
        }

        @Override
@@ -82,6 +85,65 @@ public abstract class CodegenContext<T extends DeclarationDescriptor> {
        }
    }

+    private static class AccessorForPropertyDescriptorFactory {
+        private final @NotNull PropertyDescriptor property;
+        private final @NotNull DeclarationDescriptor containingDeclaration;
+        private final @Nullable KtSuperExpression superCallExpression;
+        private final @NotNull String nameSuffix;
+
+        private AccessorForPropertyDescriptor withSyntheticGetterAndSetter = null;
+        private AccessorForPropertyDescriptor withSyntheticGetter = null;
+        private AccessorForPropertyDescriptor withSyntheticSetter = null;
+
+        public AccessorForPropertyDescriptorFactory(
+                @NotNull PropertyDescriptor property,
+                @NotNull DeclarationDescriptor containingDeclaration,
+                @Nullable KtSuperExpression superCallExpression,
+                @NotNull String nameSuffix
+        ) {
+            this.property = property;
+            this.containingDeclaration = containingDeclaration;
+            this.superCallExpression = superCallExpression;
+            this.nameSuffix = nameSuffix;
+        }
+
+        @SuppressWarnings("ConstantConditions")
+        public PropertyDescriptor getOrCreateAccessorIfNeeded(boolean getterAccessorRequired, boolean setterAccessorRequired) {
+            if (getterAccessorRequired && setterAccessorRequired) {
+                return getOrCreateAccessorWithSyntheticGetterAndSetter();
+            }
+            else if (getterAccessorRequired && !setterAccessorRequired) {
+                if (withSyntheticGetter == null) {
+                    withSyntheticGetter = new AccessorForPropertyDescriptor(
+                            property, containingDeclaration, superCallExpression, nameSuffix,
+                            true, false);
+                }
+                return withSyntheticGetter;
+            }
+            else if (!getterAccessorRequired && setterAccessorRequired) {
+                if (withSyntheticSetter == null) {
+                    withSyntheticSetter = new AccessorForPropertyDescriptor(
+                            property, containingDeclaration, superCallExpression, nameSuffix,
+                            false, true);
+                }
+                return withSyntheticSetter;
+            }
+            else {
+                return property;
+            }
+        }
+
+        @NotNull
+        public AccessorForPropertyDescriptor getOrCreateAccessorWithSyntheticGetterAndSetter() {
+            if (withSyntheticGetterAndSetter == null) {
+                withSyntheticGetterAndSetter = new AccessorForPropertyDescriptor(
+                        property, containingDeclaration, superCallExpression, nameSuffix,
+                        true, true);
+            }
+            return withSyntheticGetterAndSetter;
+        }
+    }
+
    public CodegenContext(
            @NotNull T contextDescriptor,
            @NotNull OwnerKind contextKind,
@@ -291,6 +353,16 @@ public abstract class CodegenContext<T extends DeclarationDescriptor> {
        return c;
    }

+    @NotNull
+    private PropertyDescriptor getPropertyAccessor(
+            @NotNull PropertyDescriptor propertyDescriptor,
+            @Nullable KtSuperExpression superCallExpression,
+            boolean getterAccessorRequired,
+            boolean setterAccessorRequired
+    ) {
+        return getAccessor(propertyDescriptor, FieldAccessorKind.NORMAL, null, superCallExpression, getterAccessorRequired, setterAccessorRequired);
+    }
+
    @NotNull
    public <D extends CallableMemberDescriptor> D getAccessor(@NotNull D descriptor, @Nullable KtSuperExpression superCallExpression) {
        return getAccessor(descriptor, FieldAccessorKind.NORMAL, null, superCallExpression);
@@ -303,16 +375,42 @@ public abstract class CodegenContext<T extends DeclarationDescriptor> {
            @NotNull FieldAccessorKind accessorKind,
            @Nullable KotlinType delegateType,
            @Nullable KtSuperExpression superCallExpression
+    ) {
+        // TODO this corresponds to default behavior for properties before fixing KT-9717. Is it Ok in general case?
+        // Does not matter for other descriptor kinds.
+        return getAccessor(possiblySubstitutedDescriptor, accessorKind, delegateType, superCallExpression,
+                           /* getterAccessorRequired */ true,
+                           /* setterAccessorRequired */ true);
+    }
+
+    @SuppressWarnings("unchecked")
+    @NotNull
+    private <D extends CallableMemberDescriptor> D getAccessor(
+            @NotNull D possiblySubstitutedDescriptor,
+            @NotNull FieldAccessorKind accessorKind,
+            @Nullable KotlinType delegateType,
+            @Nullable KtSuperExpression superCallExpression,
+            boolean getterAccessorRequired,
+            boolean setterAccessorRequired
    ) {
        if (accessors == null) {
            accessors = new LinkedHashMap<AccessorKey, AccessorForCallableDescriptor<?>>();
        }
+        if (propertyAccessorFactories == null) {
+            propertyAccessorFactories = new LinkedHashMap<AccessorKey, AccessorForPropertyDescriptorFactory>();
+        }

        D descriptor = (D) possiblySubstitutedDescriptor.getOriginal();
        AccessorKey key = new AccessorKey(
-                descriptor, superCallExpression == null ? null : ExpressionCodegen.getSuperCallLabelTarget(this, superCallExpression)
+                descriptor,
+                superCallExpression == null ? null : ExpressionCodegen.getSuperCallLabelTarget(this, superCallExpression)
        );

+        // NB should check for property accessor factory first (or change property accessor tracking under propertyAccessorFactory creation)
+        AccessorForPropertyDescriptorFactory propertyAccessorFactory = propertyAccessorFactories.get(key);
+        if (propertyAccessorFactory != null) {
+            return (D) propertyAccessorFactory.getOrCreateAccessorIfNeeded(getterAccessorRequired, setterAccessorRequired);
+        }
        AccessorForCallableDescriptor<?> accessor = accessors.get(key);
        if (accessor != null) {
            assert accessorKind == FieldAccessorKind.NORMAL ||
@@ -332,8 +430,18 @@ public abstract class CodegenContext<T extends DeclarationDescriptor> {
            PropertyDescriptor propertyDescriptor = (PropertyDescriptor) descriptor;
            switch (accessorKind) {
                case NORMAL:
-                    accessor = new AccessorForPropertyDescriptor(propertyDescriptor, contextDescriptor, superCallExpression, nameSuffix);
-                    break;
+                    propertyAccessorFactory = new AccessorForPropertyDescriptorFactory((PropertyDescriptor) descriptor, contextDescriptor,
+                                                                                       superCallExpression, nameSuffix);
+                    propertyAccessorFactories.put(key, propertyAccessorFactory);
+
+                    // Record worst case accessor for accessor methods generation.
+                    AccessorForPropertyDescriptor accessorWithGetterAndSetter =
+                            propertyAccessorFactory.getOrCreateAccessorWithSyntheticGetterAndSetter();
+                    accessors.put(key, accessorWithGetterAndSetter);
+
+                    PropertyDescriptor accessorDescriptor =
+                            propertyAccessorFactory.getOrCreateAccessorIfNeeded(getterAccessorRequired, setterAccessorRequired);
+                    return (D) accessorDescriptor;
                case IN_CLASS_COMPANION:
                    accessor = new AccessorForPropertyBackingFieldInClassCompanion(propertyDescriptor, contextDescriptor,
                                                                                   delegateType, nameSuffix);
@@ -423,20 +531,6 @@ public abstract class CodegenContext<T extends DeclarationDescriptor> {
        }
    }

-    private static int getAccessFlags(@NotNull CallableMemberDescriptor descriptor) {
-        int flag = getVisibilityAccessFlag(descriptor);
-        if (descriptor instanceof PropertyDescriptor) {
-            PropertyDescriptor propertyDescriptor = (PropertyDescriptor) descriptor;
-
-            PropertySetterDescriptor setter = propertyDescriptor.getSetter();
-            PropertyGetterDescriptor getter = propertyDescriptor.getGetter();
-
-            flag |= (getter == null ? 0 : getVisibilityAccessFlag(getter)) |
-                (setter == null ? 0 : getVisibilityAccessFlag(setter));
-        }
-        return flag;
-    }
-
    @SuppressWarnings("unchecked")
    @NotNull
    private <D extends CallableMemberDescriptor> D accessibleDescriptorIfNeeded(
@@ -444,10 +538,6 @@ public abstract class CodegenContext<T extends DeclarationDescriptor> {
            @Nullable KtSuperExpression superCallExpression
    ) {
        CallableMemberDescriptor unwrappedDescriptor = DescriptorUtils.unwrapFakeOverride(descriptor);
-        int flag = getAccessFlags(unwrappedDescriptor);
-        if ((flag & ACC_PRIVATE) == 0 && (flag & ACC_PROTECTED) == 0) {
-            return descriptor;
-        }

        DeclarationDescriptor enclosed = descriptor.getContainingDeclaration();
        CodegenContext descriptorContext = findParentContextWithDescriptor(enclosed);
@@ -462,20 +552,51 @@ public abstract class CodegenContext<T extends DeclarationDescriptor> {
            return descriptor;
        }

-        if ((flag & ACC_PROTECTED) != 0) {
-            PackageFragmentDescriptor unwrappedDescriptorPackage =
-                    DescriptorUtils.getParentOfType(unwrappedDescriptor, PackageFragmentDescriptor.class, false);
-            PackageFragmentDescriptor contextDescriptorPackage =
-                    DescriptorUtils.getParentOfType(descriptorContext.getContextDescriptor(), PackageFragmentDescriptor.class, false);
+        if (descriptor instanceof PropertyDescriptor) {
+            PropertyDescriptor propertyDescriptor = (PropertyDescriptor) descriptor;
+            int propertyAccessFlag = getVisibilityAccessFlag(descriptor);

-            boolean inSamePackage = contextDescriptorPackage != null && unwrappedDescriptorPackage != null &&
-                                    unwrappedDescriptorPackage.getFqName().equals(contextDescriptorPackage.getFqName());
-            if (inSamePackage) {
+            PropertyGetterDescriptor getter = propertyDescriptor.getGetter();
+            int getterAccessFlag = getter == null ? propertyAccessFlag
+                                                  : propertyAccessFlag | getVisibilityAccessFlag(getter);
+            boolean getterAccessorRequired = isAccessorRequired(getterAccessFlag, unwrappedDescriptor, descriptorContext);
+
+            PropertySetterDescriptor setter = propertyDescriptor.getSetter();
+            int setterAccessFlag = setter == null ? propertyAccessFlag
+                                                  : propertyAccessFlag | getVisibilityAccessFlag(setter);
+            boolean setterAccessorRequired = isAccessorRequired(setterAccessFlag, unwrappedDescriptor, descriptorContext);
+
+            if (!getterAccessorRequired && !setterAccessorRequired) {
                return descriptor;
            }
+            return (D) descriptorContext.getPropertyAccessor(propertyDescriptor, superCallExpression, getterAccessorRequired, setterAccessorRequired);
        }
+        else {
+            int flag = getVisibilityAccessFlag(unwrappedDescriptor);
+            if (!isAccessorRequired(flag, unwrappedDescriptor, descriptorContext)) {
+                return descriptor;
+            }
+            return (D) descriptorContext.getAccessor(descriptor, superCallExpression);
+        }
+    }

-        return (D) descriptorContext.getAccessor(descriptor, superCallExpression);
+    private static boolean isAccessorRequired(
+            int accessFlag,
+            @NotNull CallableMemberDescriptor unwrappedDescriptor,
+            @NotNull CodegenContext descriptorContext
+    ) {
+        return (accessFlag & ACC_PRIVATE) != 0 ||
+               ((accessFlag & ACC_PROTECTED) != 0 && !isInSamePackage(unwrappedDescriptor, descriptorContext.getContextDescriptor()));
+    }
+
+    private static boolean isInSamePackage(DeclarationDescriptor descriptor1, DeclarationDescriptor descriptor2) {
+        PackageFragmentDescriptor package1 =
+                DescriptorUtils.getParentOfType(descriptor1, PackageFragmentDescriptor.class, false);
+        PackageFragmentDescriptor package2 =
+                DescriptorUtils.getParentOfType(descriptor2, PackageFragmentDescriptor.class, false);
+
+        return package2 != null && package1 != null &&
+               package1.getFqName().equals(package2.getFqName());
    }

    private void addChild(@NotNull CodegenContext child) {
@@ -0,0 +1,11 @@
+class A {
+    public var prop = "OK"
+        private set
+
+
+    fun test(): String {
+        return { prop }()
+    }
+}
+
+fun box(): String = A().test()
@@ -0,0 +1,2 @@
+public var OK: String = "OK"
+    private set
@@ -0,0 +1,5 @@
+object Test {
+    val test: String = OK
+}
+
+fun box(): String = Test.test
@@ -0,0 +1,8 @@
+package a
+
+import b.*
+
+fun box(): String {
+    BB().ok()
+    return BB().OK
+}
@@ -0,0 +1,10 @@
+package b
+
+public open class B {
+    public var OK: String = "OK"
+        protected set
+}
+
+public class BB : B() {
+    public fun ok(): String = OK
+}
@@ -0,0 +1,14 @@
+import kotlin.properties.Delegates
+
+open class A<T : Any> {
+    protected var value: T by Delegates.notNull()
+        private set
+}
+
+class B : A<Int>()
+
+fun box(): String {
+    B()
+
+    return "OK"
+}
@@ -0,0 +1,11 @@
+class A {
+    public var prop = "O"
+        private set
+
+    fun test() {
+        { prop }()
+    }
+}
+
+// 0 INVOKESTATIC test\/A\.access\$getProp\$0
+// 1 INVOKEVIRTUAL A\.getProp
@@ -197,6 +197,12 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest {
        doTest(fileName);
    }

+    @TestMetadata("kt9603.kt")
+    public void testKt9603() throws Exception {
+        String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/kt9603.kt");
+        doTest(fileName);
+    }
+
    @TestMetadata("noFlagAnnotations.kt")
    public void testNoFlagAnnotations() throws Exception {
        String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/noFlagAnnotations.kt");
@@ -6667,6 +6667,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
            doTest(fileName);
        }

+        @TestMetadata("kt9603.kt")
+        public void testKt9603() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/box/properties/kt9603.kt");
+            doTest(fileName);
+        }
+
        @TestMetadata("primitiveOverrideDefaultAccessor.kt")
        public void testPrimitiveOverrideDefaultAccessor() throws Exception {
            String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/box/properties/primitiveOverrideDefaultAccessor.kt");
@@ -89,6 +89,18 @@ public class BlackBoxMultiFileCodegenTestGenerated extends AbstractBlackBoxCodeg
        doTestMultiFile(fileName);
    }

+    @TestMetadata("kt9717")
+    public void testKt9717() throws Exception {
+        String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/boxMultiFile/kt9717/");
+        doTestMultiFile(fileName);
+    }
+
+    @TestMetadata("kt9717DifferentPackages")
+    public void testKt9717DifferentPackages() throws Exception {
+        String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/boxMultiFile/kt9717DifferentPackages/");
+        doTestMultiFile(fileName);
+    }
+
    @TestMetadata("mainInFiles")
    public void testMainInFiles() throws Exception {
        String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/boxMultiFile/mainInFiles/");
@@ -1462,6 +1462,12 @@ public class BlackBoxWithStdlibCodegenTestGenerated extends AbstractBlackBoxCode
            doTestWithStdlib(fileName);
        }

+        @TestMetadata("protectedVarWithPrivateSet.kt")
+        public void testProtectedVarWithPrivateSet() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/boxWithStdlib/delegatedProperty/protectedVarWithPrivateSet.kt");
+            doTestWithStdlib(fileName);
+        }
+
        @TestMetadata("stackOverflowOnCallFromGetValue.kt")
        public void testStackOverflowOnCallFromGetValue() throws Exception {
            String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/boxWithStdlib/delegatedProperty/stackOverflowOnCallFromGetValue.kt");