From 5d76e463d3f9a53b1a1fab83062bc072b191e3d2 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Wed, 13 Jun 2018 18:00:12 +0200 Subject: [PATCH] Fix exception from reflection on local delegated properties The problem was that in JvmSerializerExtension.writeLocalProperties, we only serialized metadata for local properties, but indices generated in MemberCodegen.generatePropertyMetadataArrayFieldIfNeeded were among all delegated properties in the class (not only local). This behaved incorrectly as long as there was a local and a non-local delegated property in the same class. For example, if there were 5 non-local properties and then one local, that local property would get the index 5 and the synthetic signature "". But there would only be one Property entry in the metadata, and so reflection would fail here trying to load the 5th element of the list which contains only one element. Now, the index for a local delegated property is computed only as the number of _local_ delegated properties above it in the class, i.e. the first local delegated property gets index 0 (and synthetic signature ""), the next one -- index 1, and so on. #KT-23413 Fixed --- .../codegen/PropertyReferenceCodegen.kt | 5 +-- .../codegen/binding/CodegenBinding.java | 8 +++++ .../serialization/JvmSerializerExtension.java | 13 +++---- .../localDelegated/localAndNonLocal.kt | 34 +++++++++++++++++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 5 +++ .../codegen/BlackBoxCodegenTestGenerated.java | 5 +++ .../LightAnalysisModeTestGenerated.java | 5 +++ .../IrJsCodegenBoxTestGenerated.java | 6 ++++ 8 files changed, 71 insertions(+), 10 deletions(-) create mode 100644 compiler/testData/codegen/box/reflection/properties/localDelegated/localAndNonLocal.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyReferenceCodegen.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyReferenceCodegen.kt index dab31527c17..48030434c81 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyReferenceCodegen.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyReferenceCodegen.kt @@ -197,8 +197,9 @@ class PropertyReferenceCodegen( fun generateCallableReferenceSignature(iv: InstructionAdapter, callable: CallableDescriptor, state: GenerationState) { if (callable is LocalVariableDescriptor) { val asmType = state.bindingContext.get(CodegenBinding.DELEGATED_PROPERTY_METADATA_OWNER, callable) - val allDelegatedProperties = state.bindingContext.get(CodegenBinding.DELEGATED_PROPERTIES, asmType) - val index = allDelegatedProperties?.indexOf(callable) ?: -1 + ?: throw AssertionError("No delegated property metadata owner for $callable") + val localDelegatedProperties = CodegenBinding.getLocalDelegatedProperties(state.bindingContext, asmType) + val index = localDelegatedProperties?.indexOf(callable) ?: -1 if (index < 0) { throw AssertionError("Local delegated property is not found in $asmType: $callable") } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/binding/CodegenBinding.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/binding/CodegenBinding.java index 2179efc5ae4..20d7a08b409 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/binding/CodegenBinding.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/binding/CodegenBinding.java @@ -6,6 +6,7 @@ package org.jetbrains.kotlin.codegen.binding; import com.intellij.openapi.vfs.VirtualFile; +import kotlin.collections.CollectionsKt; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.codegen.JvmCodegenUtil; @@ -13,6 +14,7 @@ import org.jetbrains.kotlin.codegen.SamType; import org.jetbrains.kotlin.codegen.state.GenerationState; import org.jetbrains.kotlin.codegen.when.WhenByEnumsMapping; import org.jetbrains.kotlin.descriptors.*; +import org.jetbrains.kotlin.descriptors.impl.LocalVariableDescriptor; import org.jetbrains.kotlin.name.FqName; import org.jetbrains.kotlin.psi.*; import org.jetbrains.kotlin.psi.psiUtil.PsiUtilsKt; @@ -72,6 +74,12 @@ public class CodegenBinding { private CodegenBinding() { } + @Nullable + public static List getLocalDelegatedProperties(@NotNull BindingContext bindingContext, @NotNull Type owner) { + List properties = bindingContext.get(DELEGATED_PROPERTIES, owner); + return properties == null ? null : CollectionsKt.filterIsInstance(properties, LocalVariableDescriptor.class); + } + public static void initTrace(@NotNull GenerationState state) { CodegenAnnotatingVisitor visitor = new CodegenAnnotatingVisitor(state); for (KtFile file : allFilesInPackages(state.getBindingContext(), state.getFiles())) { diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmSerializerExtension.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmSerializerExtension.java index b0d6c8faa80..2a59ca57913 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmSerializerExtension.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmSerializerExtension.java @@ -104,16 +104,13 @@ public class JvmSerializerExtension extends SerializerExtension { @NotNull Type classAsmType, @NotNull GeneratedMessageLite.GeneratedExtension> extension ) { - List localVariables = codegenBinding.get(CodegenBinding.DELEGATED_PROPERTIES, classAsmType); + List localVariables = CodegenBinding.getLocalDelegatedProperties(codegenBinding, classAsmType); if (localVariables == null) return; - for (VariableDescriptorWithAccessors localVariable : localVariables) { - if (localVariable instanceof LocalVariableDescriptor) { - PropertyDescriptor propertyDescriptor = - FakeDescriptorsForReferencesKt.createFreeFakeLocalPropertyDescriptor((LocalVariableDescriptor) localVariable); - DescriptorSerializer serializer = DescriptorSerializer.createForLambda(this); - proto.addExtension(extension, serializer.propertyProto(propertyDescriptor).build()); - } + for (LocalVariableDescriptor localVariable : localVariables) { + PropertyDescriptor propertyDescriptor = FakeDescriptorsForReferencesKt.createFreeFakeLocalPropertyDescriptor(localVariable); + DescriptorSerializer serializer = DescriptorSerializer.createForLambda(this); + proto.addExtension(extension, serializer.propertyProto(propertyDescriptor).build()); } } diff --git a/compiler/testData/codegen/box/reflection/properties/localDelegated/localAndNonLocal.kt b/compiler/testData/codegen/box/reflection/properties/localDelegated/localAndNonLocal.kt new file mode 100644 index 00000000000..f3558748e1f --- /dev/null +++ b/compiler/testData/codegen/box/reflection/properties/localDelegated/localAndNonLocal.kt @@ -0,0 +1,34 @@ +// TARGET_BACKEND: JVM +// WITH_REFLECT + +import kotlin.reflect.KProperty + +object Delegate { + operator fun getValue(thiz: Any?, property: KProperty<*>): String { + return property.name + ":" + property.returnType + } +} + +class C { + val a by Delegate + + fun test(): String { + if (a != "a:kotlin.String") return "Fail a: $a" + + val b by Delegate + if (b != "b:kotlin.String") return "Fail b: $b" + + return "OK" + } +} + +val x by Delegate + +fun box(): String { + if (x != "x:kotlin.String") return "Fail x: $x" + + val y by Delegate + if (y != "y:kotlin.String") return "Fail y: $y" + + return C().test() +} diff --git a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 72729fa2984..afe39726f68 100644 --- a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -18410,6 +18410,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/reflection/properties/localDelegated/inlineFun.kt"); } + @TestMetadata("localAndNonLocal.kt") + public void testLocalAndNonLocal() throws Exception { + runTest("compiler/testData/codegen/box/reflection/properties/localDelegated/localAndNonLocal.kt"); + } + @TestMetadata("localDelegatedProperty.kt") public void testLocalDelegatedProperty() throws Exception { runTest("compiler/testData/codegen/box/reflection/properties/localDelegated/localDelegatedProperty.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index c01d08cb0c7..ca72e092a55 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -18410,6 +18410,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/reflection/properties/localDelegated/inlineFun.kt"); } + @TestMetadata("localAndNonLocal.kt") + public void testLocalAndNonLocal() throws Exception { + runTest("compiler/testData/codegen/box/reflection/properties/localDelegated/localAndNonLocal.kt"); + } + @TestMetadata("localDelegatedProperty.kt") public void testLocalDelegatedProperty() throws Exception { runTest("compiler/testData/codegen/box/reflection/properties/localDelegated/localDelegatedProperty.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 1c145f63633..b72ac1f8fcd 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -18410,6 +18410,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/reflection/properties/localDelegated/inlineFun.kt"); } + @TestMetadata("localAndNonLocal.kt") + public void testLocalAndNonLocal() throws Exception { + runTest("compiler/testData/codegen/box/reflection/properties/localDelegated/localAndNonLocal.kt"); + } + @TestMetadata("localDelegatedProperty.kt") public void testLocalDelegatedProperty() throws Exception { runTest("compiler/testData/codegen/box/reflection/properties/localDelegated/localDelegatedProperty.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/IrJsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/IrJsCodegenBoxTestGenerated.java index c4cc770afe5..378d360f8cf 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/IrJsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/IrJsCodegenBoxTestGenerated.java @@ -6838,6 +6838,12 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that."); } + @TestMetadata("inlineWithJava.kt") + public void testInlineWithJava_1_2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/multiModule/inlineWithJava.kt"); + doTestWithCoroutinesPackageReplacement(fileName, "kotlin.coroutines.experimental"); + } + @TestMetadata("simple.kt") public void testSimple_1_2() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/multiModule/simple.kt");