diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/PropertyCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/PropertyCodegen.java index 124933ce773..1b9549e09b9 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/PropertyCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/PropertyCodegen.java @@ -22,7 +22,6 @@ import com.intellij.psi.PsiElement; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.codegen.context.*; -import org.jetbrains.jet.lang.resolve.java.jvmSignature.JvmMethodSignature; import org.jetbrains.jet.codegen.state.GenerationState; import org.jetbrains.jet.codegen.state.JetTypeMapper; import org.jetbrains.jet.descriptors.serialization.descriptors.DeserializedPropertyDescriptor; @@ -33,6 +32,7 @@ import org.jetbrains.jet.lang.resolve.DescriptorFactory; import org.jetbrains.jet.lang.resolve.calls.model.ResolvedCall; import org.jetbrains.jet.lang.resolve.constants.CompileTimeConstant; import org.jetbrains.jet.lang.resolve.java.JvmAbi; +import org.jetbrains.jet.lang.resolve.java.jvmSignature.JvmMethodSignature; import org.jetbrains.jet.lang.resolve.name.Name; import org.jetbrains.jet.lang.types.ErrorUtils; import org.jetbrains.jet.lang.types.JetType; @@ -44,13 +44,14 @@ import org.jetbrains.org.objectweb.asm.commons.InstructionAdapter; import org.jetbrains.org.objectweb.asm.commons.Method; import static org.jetbrains.jet.codegen.AsmUtil.*; -import static org.jetbrains.jet.lang.resolve.java.diagnostics.DiagnosticsPackage.OtherOrigin; import static org.jetbrains.jet.codegen.JvmCodegenUtil.getParentBodyCodegen; import static org.jetbrains.jet.codegen.JvmCodegenUtil.isInterface; import static org.jetbrains.jet.codegen.JvmSerializationBindings.*; +import static org.jetbrains.jet.lang.resolve.DescriptorUtils.isClassObject; import static org.jetbrains.jet.lang.resolve.DescriptorUtils.isTrait; import static org.jetbrains.jet.lang.resolve.java.AsmTypeConstants.OBJECT_TYPE; import static org.jetbrains.jet.lang.resolve.java.AsmTypeConstants.PROPERTY_METADATA_TYPE; +import static org.jetbrains.jet.lang.resolve.java.diagnostics.DiagnosticsPackage.OtherOrigin; import static org.jetbrains.org.objectweb.asm.Opcodes.*; public class PropertyCodegen { @@ -112,17 +113,55 @@ public class PropertyCodegen { } } - generateGetter(declaration, descriptor, getter); - generateSetter(declaration, descriptor, setter); + if (isAccessorNeeded(declaration, descriptor, getter)) { + generateGetter(declaration, descriptor, getter); + } + if (isAccessorNeeded(declaration, descriptor, setter)) { + generateSetter(declaration, descriptor, setter); + } context.recordSyntheticAccessorIfNeeded(descriptor, bindingContext); } + /** + * Determines if it's necessary to generate an accessor to the property, i.e. if this property can be referenced via getter/setter + * for any reason + * + * @see JvmCodegenUtil#couldUseDirectAccessToProperty + */ + private boolean isAccessorNeeded( + @Nullable JetProperty declaration, + @NotNull PropertyDescriptor descriptor, + @Nullable JetPropertyAccessor accessor + ) { + boolean isDefaultAccessor = accessor == null || !accessor.hasBody(); + + // Don't generate accessors for trait properties with default accessors in TRAIT_IMPL + if (kind == OwnerKind.TRAIT_IMPL && isDefaultAccessor) return false; + + if (declaration == null) return true; + + // Delegated or extension properties can only be referenced via accessors + if (declaration.hasDelegate() || declaration.getReceiverTypeRef() != null) return true; + + // Class object properties always should have accessors, because their backing fields are moved/copied to the outer class + if (isClassObject(descriptor.getContainingDeclaration())) return true; + + // Private class properties have accessors only in cases when those accessors are non-trivial + if (kind == OwnerKind.IMPLEMENTATION && descriptor.getVisibility() == Visibilities.PRIVATE) { + return !isDefaultAccessor; + } + + return true; + } + public void generatePrimaryConstructorProperty(JetParameter p, PropertyDescriptor descriptor) { generateBackingField(p, descriptor); - generateGetter(p, descriptor, null); - if (descriptor.isVar()) { - generateSetter(p, descriptor, null); + if (descriptor.getVisibility() != Visibilities.PRIVATE) { + generateGetter(p, descriptor, null); + if (descriptor.isVar()) { + generateSetter(p, descriptor, null); + } } } @@ -285,12 +324,8 @@ public class PropertyCodegen { @Nullable JetPropertyAccessor accessor, @NotNull PropertyAccessorDescriptor accessorDescriptor ) { - boolean isDefaultAccessor = accessor == null || accessor.getBodyExpression() == null; - - if (kind == OwnerKind.TRAIT_IMPL && isDefaultAccessor) return; - FunctionGenerationStrategy strategy; - if (isDefaultAccessor) { + if (accessor == null || !accessor.hasBody()) { if (p instanceof JetProperty && ((JetProperty) p).hasDelegate()) { strategy = new DelegatedPropertyAccessorStrategy(state, accessorDescriptor, indexOfDelegatedProperty((JetProperty) p)); } diff --git a/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInClass.java b/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInClass.java index 32bf3192ac3..21add3f1785 100644 --- a/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInClass.java +++ b/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInClass.java @@ -1,9 +1,8 @@ public final class PrivateInClass implements kotlin.jvm.internal.KObject { - private final java.lang.String nn = ""; - private final java.lang.String n = ""; - private final java.lang.String getNn() { /* compiled code */ } + private final void setNn(@jet.runtime.typeinfo.JetValueParameter(name = "value") java.lang.String value) { /* compiled code */ } + private final java.lang.String getN() { /* compiled code */ } private final java.lang.String bar(@jet.runtime.typeinfo.JetValueParameter(name = "a") java.lang.String a, @jet.runtime.typeinfo.JetValueParameter(name = "b", type = "?") java.lang.String b) { /* compiled code */ } diff --git a/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInClass.kt b/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInClass.kt index 993596d3762..001ebfced08 100644 --- a/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInClass.kt +++ b/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInClass.kt @@ -1,7 +1,10 @@ // PrivateInClass class PrivateInClass private (s: String?) { - private val nn: String = "" - private val n: String? = "" + private var nn: String + get() = "" + set(value) {} + private val n: String? + get() = "" private fun bar(a: String, b: String?): String? = null } \ No newline at end of file diff --git a/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInTrait.java b/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInTrait.java index 7a6584440f7..052cdba2245 100644 --- a/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInTrait.java +++ b/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInTrait.java @@ -2,6 +2,8 @@ public interface PrivateInTrait extends kotlin.jvm.internal.KObject { @org.jetbrains.annotations.NotNull java.lang.String getNn(); + void setNn(@jet.runtime.typeinfo.JetValueParameter(name = "value") @org.jetbrains.annotations.NotNull java.lang.String value); + @org.jetbrains.annotations.Nullable java.lang.String getN(); diff --git a/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInTrait.kt b/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInTrait.kt index c8e262b4c69..f15c852a545 100644 --- a/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInTrait.kt +++ b/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInTrait.kt @@ -1,7 +1,10 @@ // PrivateInTrait trait PrivateInTrait { - private val nn: String + private var nn: String + get() = "" + set(value) {} private val n: String? + get() = "" private fun bar(a: String, b: String?): String? } \ No newline at end of file diff --git a/compiler/testData/codegen/box/properties/privatePropertyInConstructor.kt b/compiler/testData/codegen/box/properties/privatePropertyInConstructor.kt new file mode 100644 index 00000000000..9a9e186c8de --- /dev/null +++ b/compiler/testData/codegen/box/properties/privatePropertyInConstructor.kt @@ -0,0 +1,18 @@ +class A( + private val x: String, + private var y: Double +) { + fun foo() { + val r = { + if (x != "abc") throw AssertionError("$x") + y = 0.0 + if (y != 0.0) throw AssertionError("$y") + } + r() + } +} + +fun box(): String { + A("abc", 3.14).foo() + return "OK" +} diff --git a/compiler/testData/codegen/properties/privateClassPropertyAccessors.kt b/compiler/testData/codegen/properties/privateClassPropertyAccessors.kt new file mode 100644 index 00000000000..147f144f847 --- /dev/null +++ b/compiler/testData/codegen/properties/privateClassPropertyAccessors.kt @@ -0,0 +1,31 @@ +class C { + // All these properties should have corresponding accessors + private val valWithGet: String + get() = "" + + private var varWithGetSet: Int + get() = 0 + set(value) {} + + private var delegated: Int by Delegate + + private var String.extension: String + get() = this + set(value) {} + + class object { + private val classObjectVal: Long + get() = 1L + } + + // This property should not have accessors + private var varNoAccessors = 0L + get set +} + + +object Delegate { + fun get(x: C, p: PropertyMetadata) = throw AssertionError() + + fun set(x: C, p: PropertyMetadata, value: Int) = throw AssertionError() +} diff --git a/compiler/testData/diagnostics/tests/duplicateJvmSignature/functionAndProperty/privateClassPropertyNoClash.kt b/compiler/testData/diagnostics/tests/duplicateJvmSignature/functionAndProperty/privateClassPropertyNoClash.kt new file mode 100644 index 00000000000..9fcf9ce0da1 --- /dev/null +++ b/compiler/testData/diagnostics/tests/duplicateJvmSignature/functionAndProperty/privateClassPropertyNoClash.kt @@ -0,0 +1,13 @@ +// FILE: A.java + +public class A { + public String getFoo() { + return "Foo"; + } +} + +// FILE: B.kt + +class B(private val foo: String) : A() { + override fun getFoo(): String = foo +} diff --git a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java index 194a0f68f60..70da74444b7 100644 --- a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java @@ -2936,6 +2936,11 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest { doTest("compiler/testData/diagnostics/tests/duplicateJvmSignature/functionAndProperty/objectExpressionInConstructor.kt"); } + @TestMetadata("privateClassPropertyNoClash.kt") + public void testPrivateClassPropertyNoClash() throws Exception { + doTest("compiler/testData/diagnostics/tests/duplicateJvmSignature/functionAndProperty/privateClassPropertyNoClash.kt"); + } + @TestMetadata("topLevel.kt") public void testTopLevel() throws Exception { doTest("compiler/testData/diagnostics/tests/duplicateJvmSignature/functionAndProperty/topLevel.kt"); diff --git a/compiler/tests/org/jetbrains/jet/codegen/CodegenTestUtil.java b/compiler/tests/org/jetbrains/jet/codegen/CodegenTestUtil.java index 793e5cca993..0a3c179a4a2 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/CodegenTestUtil.java +++ b/compiler/tests/org/jetbrains/jet/codegen/CodegenTestUtil.java @@ -96,12 +96,20 @@ public class CodegenTestUtil { @NotNull public static Method findDeclaredMethodByName(@NotNull Class aClass, @NotNull String name) { + Method result = findDeclaredMethodByNameOrNull(aClass, name); + if (result == null) { + throw new AssertionError("Method " + name + " is not found in " + aClass); + } + return result; + } + + public static Method findDeclaredMethodByNameOrNull(@NotNull Class aClass, @NotNull String name) { for (Method method : aClass.getDeclaredMethods()) { if (method.getName().equals(name)) { return method; } } - throw new AssertionError("Method " + name + " is not found in class " + aClass); + return null; } public static void assertIsCurrentTime(long returnValue) { diff --git a/compiler/tests/org/jetbrains/jet/codegen/PropertyGenTest.java b/compiler/tests/org/jetbrains/jet/codegen/PropertyGenTest.java index 242c52c6794..58aca51ab42 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/PropertyGenTest.java +++ b/compiler/tests/org/jetbrains/jet/codegen/PropertyGenTest.java @@ -18,12 +18,12 @@ package org.jetbrains.jet.codegen; import org.jetbrains.annotations.NotNull; import org.jetbrains.jet.ConfigurationKind; +import org.jetbrains.jet.lang.resolve.java.JvmAbi; import org.jetbrains.org.objectweb.asm.Opcodes; import java.lang.reflect.*; -import static org.jetbrains.jet.codegen.CodegenTestUtil.assertIsCurrentTime; -import static org.jetbrains.jet.codegen.CodegenTestUtil.findDeclaredMethodByName; +import static org.jetbrains.jet.codegen.CodegenTestUtil.*; public class PropertyGenTest extends CodegenTestCase { @Override @@ -236,4 +236,21 @@ public class PropertyGenTest extends CodegenTestCase { assertEquals(String.class, parameterizedType.getActualTypeArguments()[0]); } } + + public void testPrivateClassPropertyAccessors() throws Exception { + loadFile("properties/privateClassPropertyAccessors.kt"); + Class c = generateClass("C"); + findDeclaredMethodByName(c, "getValWithGet"); + findDeclaredMethodByName(c, "getVarWithGetSet"); + findDeclaredMethodByName(c, "setVarWithGetSet"); + findDeclaredMethodByName(c, "getDelegated"); + findDeclaredMethodByName(c, "setDelegated"); + findDeclaredMethodByName(c, "getExtension"); + findDeclaredMethodByName(c, "setExtension"); + + findDeclaredMethodByName(initializedClassLoader.loadClass("C" + JvmAbi.CLASS_OBJECT_SUFFIX), "getClassObjectVal"); + + assertNull("Property should not have a getter", findDeclaredMethodByNameOrNull(c, "getVarNoAccessors")); + assertNull("Property should not have a setter", findDeclaredMethodByNameOrNull(c, "setVarNoAccessors")); + } } diff --git a/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java index cdb718c7914..a6864d1e91f 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java @@ -4501,6 +4501,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest("compiler/testData/codegen/box/properties/primitiveOverrideDelegateAccessor.kt"); } + @TestMetadata("privatePropertyInConstructor.kt") + public void testPrivatePropertyInConstructor() throws Exception { + doTest("compiler/testData/codegen/box/properties/privatePropertyInConstructor.kt"); + } + @TestMetadata("privatePropertyWithoutBackingField.kt") public void testPrivatePropertyWithoutBackingField() throws Exception { doTest("compiler/testData/codegen/box/properties/privatePropertyWithoutBackingField.kt"); diff --git a/core/runtime.jvm/src/kotlin/reflect/jvm/internal/KMemberPropertyImpl.kt b/core/runtime.jvm/src/kotlin/reflect/jvm/internal/KMemberPropertyImpl.kt index b5b706c773c..35bc10b6a33 100644 --- a/core/runtime.jvm/src/kotlin/reflect/jvm/internal/KMemberPropertyImpl.kt +++ b/core/runtime.jvm/src/kotlin/reflect/jvm/internal/KMemberPropertyImpl.kt @@ -25,25 +25,30 @@ open class KMemberPropertyImpl( override val name: String, protected val owner: KClassImpl ) : KMemberProperty, KPropertyImpl { + // TODO: extract, make lazy (weak?), use our descriptors knowledge override val field: Field? - get() = try { - owner.jClass.getDeclaredField(name) + override val getter: Method? + + { + try { + field = owner.jClass.getDeclaredField(name) } catch (e: NoSuchFieldException) { - null + field = null } - // TODO: extract, make lazy (weak?), use our descriptors knowledge - override val getter: Method = try { - owner.jClass.getMaybeDeclaredMethod(getterName(name)) - } - catch (e: NoSuchMethodException) { - throw NoSuchPropertyException(e) + try { + getter = owner.jClass.getMaybeDeclaredMethod(getterName(name)) + } + catch (e: NoSuchMethodException) { + if (field == null) throw NoSuchPropertyException(e) + getter = null + } } override fun get(receiver: T): R { try { - return getter(receiver) as R + return (if (getter != null) getter!!(receiver) else field!!.get(receiver)) as R } catch (e: java.lang.IllegalAccessException) { throw kotlin.reflect.IllegalAccessException(e) @@ -65,16 +70,22 @@ class KMutableMemberPropertyImpl( name: String, owner: KClassImpl ) : KMutableMemberProperty, KMutablePropertyImpl, KMemberPropertyImpl(name, owner) { - override val setter: Method = try { - owner.jClass.getMaybeDeclaredMethod(setterName(name), getter.getReturnType()!!) - } - catch (e: NoSuchMethodException) { - throw NoSuchPropertyException(e) + override val setter: Method? + + { + try { + val returnType = if (getter != null) getter.getReturnType() else field!!.getType() + setter = owner.jClass.getMaybeDeclaredMethod(setterName(name), returnType!!) + } + catch (e: NoSuchMethodException) { + if (field == null) throw NoSuchPropertyException(e) + setter = null + } } override fun set(receiver: T, value: R) { try { - setter(receiver, value) + if (setter != null) setter!!(receiver, value) else field!!.set(receiver, value) } catch (e: java.lang.IllegalAccessException) { throw kotlin.reflect.IllegalAccessException(e) diff --git a/core/runtime.jvm/src/kotlin/reflect/jvm/properties.kt b/core/runtime.jvm/src/kotlin/reflect/jvm/properties.kt index 20df3d6b801..46d75b7385e 100644 --- a/core/runtime.jvm/src/kotlin/reflect/jvm/properties.kt +++ b/core/runtime.jvm/src/kotlin/reflect/jvm/properties.kt @@ -22,8 +22,13 @@ import kotlin.reflect.jvm.internal.* public var KProperty.accessible: Boolean get() { return when (this) { - is KMutableMemberPropertyImpl<*, R> -> getter.isAccessible() && setter.isAccessible() - is KMemberPropertyImpl<*, R> -> getter.isAccessible() + is KMutableMemberPropertyImpl<*, R> -> + field?.isAccessible() ?: true && + getter?.isAccessible() ?: true && + setter?.isAccessible() ?: true + is KMemberPropertyImpl<*, R> -> + field?.isAccessible() ?: true && + getter?.isAccessible() ?: true is KForeignMemberProperty<*, R> -> field.isAccessible() else -> { // Non-member properties always have public visibility on JVM, thus accessible has no effect on them @@ -34,11 +39,13 @@ public var KProperty.accessible: Boolean set(value) { when (this) { is KMutableMemberPropertyImpl<*, R> -> { - getter.setAccessible(value) - setter.setAccessible(value) + field?.setAccessible(value) + getter?.setAccessible(value) + setter?.setAccessible(value) } is KMemberPropertyImpl<*, R> -> { - getter.setAccessible(value) + field?.setAccessible(value) + getter?.setAccessible(value) } is KForeignMemberProperty<*, R> -> { field.setAccessible(value)