From 3b5179686eaba0a71bcca53c2cc922a54cc9241f Mon Sep 17 00:00:00 2001 From: Pavel Mikhailovskii Date: Wed, 1 Jun 2022 11:44:00 +0200 Subject: [PATCH] KT-52592 Fix NPE from KProperty.getExtensionDelegate on property delegated to another property; make $delegate methods private --- .../codegen/FirBlackBoxCodegenTestGenerated.java | 6 ++++++ .../backend/jvm/lower/JvmPropertiesLowering.kt | 11 +++++++---- .../getExtensionDelegateForDelegatedToAnother.kt | 16 ++++++++++++++++ .../getDelegate/noSetAccessibleTrue.kt | 12 ++++++++++++ .../delegateMethodIsNonOverridable_ir.txt | 4 ++-- .../delegateToAnother.kt | 2 +- .../codegen/BlackBoxCodegenTestGenerated.java | 6 ++++++ .../codegen/IrBlackBoxCodegenTestGenerated.java | 6 ++++++ .../codegen/LightAnalysisModeTestGenerated.java | 5 +++++ .../kotlin/reflect/jvm/internal/KPropertyImpl.kt | 7 +++---- 10 files changed, 64 insertions(+), 11 deletions(-) create mode 100644 compiler/testData/codegen/box/reflection/properties/getDelegate/getExtensionDelegateForDelegatedToAnother.kt diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java index 31df7b5fdc1..73ba0cc647f 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java @@ -42723,6 +42723,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/reflection/properties/getDelegate/getExtensionDelegate.kt"); } + @Test + @TestMetadata("getExtensionDelegateForDelegatedToAnother.kt") + public void testGetExtensionDelegateForDelegatedToAnother() throws Exception { + runTest("compiler/testData/codegen/box/reflection/properties/getDelegate/getExtensionDelegateForDelegatedToAnother.kt"); + } + @Test @TestMetadata("kPropertyForDelegatedProperty.kt") public void testKPropertyForDelegatedProperty() throws Exception { diff --git a/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/JvmPropertiesLowering.kt b/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/JvmPropertiesLowering.kt index f9b2d8b40ed..b3ef7cd3d8b 100644 --- a/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/JvmPropertiesLowering.kt +++ b/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/JvmPropertiesLowering.kt @@ -16,6 +16,8 @@ import org.jetbrains.kotlin.backend.jvm.ir.needsAccessor import org.jetbrains.kotlin.backend.jvm.requiresMangling import org.jetbrains.kotlin.config.LanguageFeature import org.jetbrains.kotlin.descriptors.ClassKind +import org.jetbrains.kotlin.descriptors.DescriptorVisibilities +import org.jetbrains.kotlin.descriptors.DescriptorVisibility import org.jetbrains.kotlin.descriptors.Modality import org.jetbrains.kotlin.ir.IrStatement import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET @@ -167,7 +169,7 @@ class JvmPropertiesLowering(private val backendContext: JvmBackendContext) : IrE JvmLoweredDeclarationOrigin.SYNTHETIC_METHOD_FOR_PROPERTY_OR_TYPEALIAS_ANNOTATIONS, // TODO: technically JVM permits having fields with same name but different type, so we could potentially // generate two properties like that; should this be the getter's return type instead? - isStatic = true, returnType = backendContext.irBuiltIns.unitType + isStatic = true, returnType = backendContext.irBuiltIns.unitType, visibility = declaration.visibility ).apply { body = IrBlockBodyImpl(UNDEFINED_OFFSET, UNDEFINED_OFFSET) annotations = declaration.annotations @@ -179,13 +181,14 @@ class JvmPropertiesLowering(private val backendContext: JvmBackendContext) : IrE suffix: String, origin: IrDeclarationOrigin, isStatic: Boolean, - returnType: IrType + returnType: IrType, + visibility: DescriptorVisibility ) = irFactory.buildFun { name = Name.identifier(computeSyntheticMethodName(declaration, suffix)) modality = Modality.OPEN - visibility = declaration.visibility this.origin = origin this.returnType = returnType + this.visibility = visibility }.apply { if (!isStatic) { dispatchReceiverParameter = declaration.getter?.dispatchReceiverParameter?.let { @@ -207,7 +210,7 @@ class JvmPropertiesLowering(private val backendContext: JvmBackendContext) : IrE declaration, JvmAbi.DELEGATED_PROPERTY_NAME_SUFFIX, IrDeclarationOrigin.PROPERTY_DELEGATE, - isStatic = false, returnType = irBuiltIns.anyNType + isStatic = false, returnType = irBuiltIns.anyNType, visibility = DescriptorVisibilities.PRIVATE ) private fun JvmBackendContext.computeSyntheticMethodName(property: IrProperty, suffix: String): String { diff --git a/compiler/testData/codegen/box/reflection/properties/getDelegate/getExtensionDelegateForDelegatedToAnother.kt b/compiler/testData/codegen/box/reflection/properties/getDelegate/getExtensionDelegateForDelegatedToAnother.kt new file mode 100644 index 00000000000..ebb1d608690 --- /dev/null +++ b/compiler/testData/codegen/box/reflection/properties/getDelegate/getExtensionDelegateForDelegatedToAnother.kt @@ -0,0 +1,16 @@ +// TARGET_BACKEND: JVM +// WITH_REFLECT + +import kotlin.reflect.full.getExtensionDelegate +import kotlin.reflect.jvm.isAccessible +import kotlin.test.assertEquals + +class A + +val A.x: Int get() = 1 +val A.y: Int by A::x + +fun box(): String { + assertEquals(A::x, A::y.apply { isAccessible = true }.getExtensionDelegate()) + return "OK" +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/reflection/properties/getDelegate/noSetAccessibleTrue.kt b/compiler/testData/codegen/box/reflection/properties/getDelegate/noSetAccessibleTrue.kt index 9b7a170c2c0..03c9907cbf5 100644 --- a/compiler/testData/codegen/box/reflection/properties/getDelegate/noSetAccessibleTrue.kt +++ b/compiler/testData/codegen/box/reflection/properties/getDelegate/noSetAccessibleTrue.kt @@ -11,11 +11,15 @@ object Delegate { } val topLevel: Boolean by Delegate +val delegated: Boolean by ::topLevel val String.extension: Boolean by Delegate +val String.delegated: Boolean by String::delegated class Foo { val member: Boolean by Delegate + val delegated: Boolean by ::member val String.memberExtension: Boolean by Delegate + val String.memberExtensionDelegated: Boolean by ::member } inline fun check(block: () -> Unit) { @@ -29,16 +33,24 @@ inline fun check(block: () -> Unit) { fun box(): String { check { ::topLevel.getDelegate() } + check { ::delegated.getDelegate() } check { String::extension.getDelegate("") } check { ""::extension.getDelegate() } + check { String::delegated.getDelegate("") } + check { ""::delegated.getDelegate() } val foo = Foo() check { Foo::member.getDelegate(foo) } check { foo::member.getDelegate() } + check { Foo::delegated.getDelegate(foo) } + check { foo::delegated.getDelegate() } val me = Foo::class.members.single { it.name == "memberExtension" } as KProperty2 check { me.getDelegate(foo, "") } + val med = Foo::class.members.single { it.name == "memberExtensionDelegated" } as KProperty2 + check { med.getDelegate(foo, "") } + return "OK" } diff --git a/compiler/testData/codegen/bytecodeListing/delegatedProperty/delegateMethodIsNonOverridable_ir.txt b/compiler/testData/codegen/bytecodeListing/delegatedProperty/delegateMethodIsNonOverridable_ir.txt index 73b1786be67..4d7524e9107 100644 --- a/compiler/testData/codegen/bytecodeListing/delegatedProperty/delegateMethodIsNonOverridable_ir.txt +++ b/compiler/testData/codegen/bytecodeListing/delegatedProperty/delegateMethodIsNonOverridable_ir.txt @@ -16,7 +16,7 @@ public class C { inner (anonymous) class C$x$2$1 public method (): void public method getX(): int - public static method getY$delegate(p0: C): java.lang.Object + private static method getY$delegate(p0: C): java.lang.Object public method getY(): int } @@ -38,7 +38,7 @@ public final class D { inner (anonymous) class D$x$2$1 public method (): void public method getX(): int - public static method getY$delegate(p0: D): java.lang.Object + private static method getY$delegate(p0: D): java.lang.Object public method getY(): int } diff --git a/compiler/testData/codegen/bytecodeText/optimizedDelegatedProperties/delegateToAnother.kt b/compiler/testData/codegen/bytecodeText/optimizedDelegatedProperties/delegateToAnother.kt index 48d4bce989f..1d23cf6d2ee 100644 --- a/compiler/testData/codegen/bytecodeText/optimizedDelegatedProperties/delegateToAnother.kt +++ b/compiler/testData/codegen/bytecodeText/optimizedDelegatedProperties/delegateToAnother.kt @@ -35,7 +35,7 @@ fun local() { // 0 private final( static)? Lkotlin/reflect/KMutableProperty[0-2]; [xyz]m?\$delegate // 2 private final( static)? LC; [xyz]m?\$receiver // 0 LOCALVARIABLE [xyz]m? Lkotlin/reflect/KMutableProperty[0-2]; -// 12 public( static)? get[XYZ]m?\$delegate +// 12 private( static)? get[XYZ]m?\$delegate // JVM_TEMPLATES // Not optimized, references created as classes and stored in fields: diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java index 18a6c7aec34..c1da4264fc8 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java @@ -42201,6 +42201,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/reflection/properties/getDelegate/getExtensionDelegate.kt"); } + @Test + @TestMetadata("getExtensionDelegateForDelegatedToAnother.kt") + public void testGetExtensionDelegateForDelegatedToAnother() throws Exception { + runTest("compiler/testData/codegen/box/reflection/properties/getDelegate/getExtensionDelegateForDelegatedToAnother.kt"); + } + @Test @TestMetadata("kPropertyForDelegatedProperty.kt") public void testKPropertyForDelegatedProperty() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java index eb3bd316381..6330ad83b86 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java @@ -42723,6 +42723,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/reflection/properties/getDelegate/getExtensionDelegate.kt"); } + @Test + @TestMetadata("getExtensionDelegateForDelegatedToAnother.kt") + public void testGetExtensionDelegateForDelegatedToAnother() throws Exception { + runTest("compiler/testData/codegen/box/reflection/properties/getDelegate/getExtensionDelegateForDelegatedToAnother.kt"); + } + @Test @TestMetadata("kPropertyForDelegatedProperty.kt") public void testKPropertyForDelegatedProperty() throws Exception { diff --git a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index af6117ac7e0..33b764a9150 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -33942,6 +33942,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/reflection/properties/getDelegate/getExtensionDelegate.kt"); } + @TestMetadata("getExtensionDelegateForDelegatedToAnother.kt") + public void testGetExtensionDelegateForDelegatedToAnother() throws Exception { + runTest("compiler/testData/codegen/box/reflection/properties/getDelegate/getExtensionDelegateForDelegatedToAnother.kt"); + } + @TestMetadata("kPropertyForDelegatedProperty.kt") public void testKPropertyForDelegatedProperty() throws Exception { runTest("compiler/testData/codegen/box/reflection/properties/getDelegate/kPropertyForDelegatedProperty.kt"); diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPropertyImpl.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPropertyImpl.kt index 88bd9704e98..6cc949f4390 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPropertyImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPropertyImpl.kt @@ -14,10 +14,7 @@ import org.jetbrains.kotlin.resolve.DescriptorUtils import org.jetbrains.kotlin.resolve.isUnderlyingPropertyOfInlineClass import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedPropertyDescriptor import org.jetbrains.kotlin.types.TypeUtils -import java.lang.reflect.Field -import java.lang.reflect.Member -import java.lang.reflect.Method -import java.lang.reflect.Modifier +import java.lang.reflect.* import kotlin.jvm.internal.CallableReference import kotlin.reflect.KFunction import kotlin.reflect.KMutableProperty @@ -25,6 +22,7 @@ import kotlin.reflect.KProperty import kotlin.reflect.full.IllegalPropertyDelegateAccessException import kotlin.reflect.jvm.internal.JvmPropertySignature.* import kotlin.reflect.jvm.internal.calls.* +import kotlin.reflect.jvm.isAccessible internal abstract class KPropertyImpl private constructor( override val container: KDeclarationContainerImpl, @@ -105,6 +103,7 @@ internal abstract class KPropertyImpl private constructor( val realReceiver1 = (if (isBound) boundReceiver else receiver1).takeIf { it !== EXTENSION_PROPERTY_DELEGATE } val realReceiver2 = (if (isBound) receiver1 else receiver2).takeIf { it !== EXTENSION_PROPERTY_DELEGATE } + (fieldOrMethod as? AccessibleObject)?.isAccessible = isAccessible when (fieldOrMethod) { null -> null is Field -> fieldOrMethod.get(realReceiver1)