diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/AccessorForPropertyDescriptor.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/AccessorForPropertyDescriptor.java index d7df0fdc654..032c15d3342 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/AccessorForPropertyDescriptor.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/AccessorForPropertyDescriptor.java @@ -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.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 { @@ -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; + } } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/MemberCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/MemberCodegen.java index b8ac8c66642..e3cd2d07d84 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/MemberCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/MemberCodegen.java @@ -665,13 +665,14 @@ public abstract class MemberCodegen { private Map childContexts; private Map> accessors; + private Map 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 { 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 { } } + 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 { 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 getAccessor(@NotNull D descriptor, @Nullable KtSuperExpression superCallExpression) { return getAccessor(descriptor, FieldAccessorKind.NORMAL, null, superCallExpression); @@ -303,16 +375,42 @@ public abstract class CodegenContext { @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 getAccessor( + @NotNull D possiblySubstitutedDescriptor, + @NotNull FieldAccessorKind accessorKind, + @Nullable KotlinType delegateType, + @Nullable KtSuperExpression superCallExpression, + boolean getterAccessorRequired, + boolean setterAccessorRequired ) { if (accessors == null) { accessors = new LinkedHashMap>(); } + if (propertyAccessorFactories == null) { + propertyAccessorFactories = new LinkedHashMap(); + } 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 { 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 { } } - 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 accessibleDescriptorIfNeeded( @@ -444,10 +538,6 @@ public abstract class CodegenContext { @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 { 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) { diff --git a/compiler/testData/codegen/box/properties/kt9603.kt b/compiler/testData/codegen/box/properties/kt9603.kt new file mode 100644 index 00000000000..ddbabfcc99b --- /dev/null +++ b/compiler/testData/codegen/box/properties/kt9603.kt @@ -0,0 +1,11 @@ +class A { + public var prop = "OK" + private set + + + fun test(): String { + return { prop }() + } +} + +fun box(): String = A().test() \ No newline at end of file diff --git a/compiler/testData/codegen/boxMultiFile/kt9717/Vars.kt b/compiler/testData/codegen/boxMultiFile/kt9717/Vars.kt new file mode 100644 index 00000000000..76e2c8e1b78 --- /dev/null +++ b/compiler/testData/codegen/boxMultiFile/kt9717/Vars.kt @@ -0,0 +1,2 @@ +public var OK: String = "OK" + private set \ No newline at end of file diff --git a/compiler/testData/codegen/boxMultiFile/kt9717/box.kt b/compiler/testData/codegen/boxMultiFile/kt9717/box.kt new file mode 100644 index 00000000000..a71af04152b --- /dev/null +++ b/compiler/testData/codegen/boxMultiFile/kt9717/box.kt @@ -0,0 +1,5 @@ +object Test { + val test: String = OK +} + +fun box(): String = Test.test \ No newline at end of file diff --git a/compiler/testData/codegen/boxMultiFile/kt9717DifferentPackages/a.kt b/compiler/testData/codegen/boxMultiFile/kt9717DifferentPackages/a.kt new file mode 100644 index 00000000000..b281370407e --- /dev/null +++ b/compiler/testData/codegen/boxMultiFile/kt9717DifferentPackages/a.kt @@ -0,0 +1,8 @@ +package a + +import b.* + +fun box(): String { + BB().ok() + return BB().OK +} \ No newline at end of file diff --git a/compiler/testData/codegen/boxMultiFile/kt9717DifferentPackages/b.kt b/compiler/testData/codegen/boxMultiFile/kt9717DifferentPackages/b.kt new file mode 100644 index 00000000000..12f8e3aad0f --- /dev/null +++ b/compiler/testData/codegen/boxMultiFile/kt9717DifferentPackages/b.kt @@ -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 +} \ No newline at end of file diff --git a/compiler/testData/codegen/boxWithStdlib/delegatedProperty/protectedVarWithPrivateSet.kt b/compiler/testData/codegen/boxWithStdlib/delegatedProperty/protectedVarWithPrivateSet.kt new file mode 100644 index 00000000000..99cf9d50f03 --- /dev/null +++ b/compiler/testData/codegen/boxWithStdlib/delegatedProperty/protectedVarWithPrivateSet.kt @@ -0,0 +1,14 @@ +import kotlin.properties.Delegates + +open class A { + protected var value: T by Delegates.notNull() + private set +} + +class B : A() + +fun box(): String { + B() + + return "OK" +} \ No newline at end of file diff --git a/compiler/testData/codegen/bytecodeText/kt9603.kt b/compiler/testData/codegen/bytecodeText/kt9603.kt new file mode 100644 index 00000000000..51891241737 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/kt9603.kt @@ -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 \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index 04e54c44ffe..1640608a930 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -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"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java index 3a185bfdf97..90494d428f5 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java @@ -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"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxMultiFileCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxMultiFileCodegenTestGenerated.java index dba5251b4dd..e3dc6dca471 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxMultiFileCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxMultiFileCodegenTestGenerated.java @@ -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/"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxWithStdlibCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxWithStdlibCodegenTestGenerated.java index a9bd2922546..89b24b42b57 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxWithStdlibCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxWithStdlibCodegenTestGenerated.java @@ -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");