From fdfd808d80cd902bfb6fac8363f736e8d08d9659 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Fri, 20 Feb 2015 19:22:30 +0300 Subject: [PATCH] Remove KForeignMemberProperty and KClassOrigin, use KMemberPropertyImpl instead --- .../reflection/mapping/javaFields.kt | 4 +- .../properties/equalsHashCodeToString.java | 11 +++ .../properties/equalsHashCodeToString.kt | 17 +++++ ...ackBoxAgainstJavaCodegenTestGenerated.java | 16 +++++ .../jvm/internal/DescriptorBasedProperty.kt | 30 ++++---- .../kotlin/reflect/jvm/internal/KClassImpl.kt | 38 +--------- .../jvm/internal/KForeignMemberProperty.kt | 72 ------------------- .../src/kotlin/reflect/jvm/properties.kt | 9 +-- 8 files changed, 69 insertions(+), 128 deletions(-) create mode 100644 compiler/testData/codegen/boxAgainstJava/reflection/properties/equalsHashCodeToString.java create mode 100644 compiler/testData/codegen/boxAgainstJava/reflection/properties/equalsHashCodeToString.kt delete mode 100644 core/reflection.jvm/src/kotlin/reflect/jvm/internal/KForeignMemberProperty.kt diff --git a/compiler/testData/codegen/boxAgainstJava/reflection/mapping/javaFields.kt b/compiler/testData/codegen/boxAgainstJava/reflection/mapping/javaFields.kt index a5de7e5bf12..76baeaf3855 100644 --- a/compiler/testData/codegen/boxAgainstJava/reflection/mapping/javaFields.kt +++ b/compiler/testData/codegen/boxAgainstJava/reflection/mapping/javaFields.kt @@ -9,8 +9,8 @@ fun box(): String { val s = J::s // Check that correct reflection objects are created - assert(i.javaClass.getSimpleName() == "KForeignMemberProperty", "Fail i class") - assert(s.javaClass.getSimpleName() == "KMutableForeignMemberProperty", "Fail s class") + assert(i.javaClass.getSimpleName() == "KMemberPropertyImpl", "Fail i class") + assert(s.javaClass.getSimpleName() == "KMutableMemberPropertyImpl", "Fail s class") // Check that no Method objects are created for such properties assert(i.javaGetter == null, "Fail i getter") diff --git a/compiler/testData/codegen/boxAgainstJava/reflection/properties/equalsHashCodeToString.java b/compiler/testData/codegen/boxAgainstJava/reflection/properties/equalsHashCodeToString.java new file mode 100644 index 00000000000..82d5b58a933 --- /dev/null +++ b/compiler/testData/codegen/boxAgainstJava/reflection/properties/equalsHashCodeToString.java @@ -0,0 +1,11 @@ +package test; + +public class equalsHashCodeToString { + public final boolean b; + public char c; + + public equalsHashCodeToString() { + this.b = false; + this.c = '0'; + } +} diff --git a/compiler/testData/codegen/boxAgainstJava/reflection/properties/equalsHashCodeToString.kt b/compiler/testData/codegen/boxAgainstJava/reflection/properties/equalsHashCodeToString.kt new file mode 100644 index 00000000000..ba0904a8f99 --- /dev/null +++ b/compiler/testData/codegen/boxAgainstJava/reflection/properties/equalsHashCodeToString.kt @@ -0,0 +1,17 @@ +package test + +import kotlin.test.* +import test.equalsHashCodeToString as J + +fun box(): String { + assertEquals("val test.equalsHashCodeToString.b", (J::b).toString()) + assertEquals("var test.equalsHashCodeToString.c", (J::c).toString()) + + assertTrue(J::b == J::b) + assertFalse(J::c == J::b) + + assertTrue(J::b.hashCode() == J::b.hashCode()) + assertFalse(J::b.hashCode() == J::c.hashCode()) + + return "OK" +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxAgainstJavaCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxAgainstJavaCodegenTestGenerated.java index f57c8fbcaaf..97a31e1bd90 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxAgainstJavaCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxAgainstJavaCodegenTestGenerated.java @@ -332,6 +332,7 @@ public class BlackBoxAgainstJavaCodegenTestGenerated extends AbstractBlackBoxCod @TestDataPath("$PROJECT_ROOT") @InnerTestClasses({ Reflection.Mapping.class, + Reflection.Properties.class, }) @RunWith(JUnit3RunnerWithInners.class) public static class Reflection extends AbstractBlackBoxCodegenTest { @@ -359,6 +360,21 @@ public class BlackBoxAgainstJavaCodegenTestGenerated extends AbstractBlackBoxCod doTestAgainstJava(fileName); } } + + @TestMetadata("compiler/testData/codegen/boxAgainstJava/reflection/properties") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class Properties extends AbstractBlackBoxCodegenTest { + public void testAllFilesPresentInProperties() throws Exception { + JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/boxAgainstJava/reflection/properties"), Pattern.compile("^(.+)\\.kt$"), true); + } + + @TestMetadata("equalsHashCodeToString.kt") + public void testEqualsHashCodeToString() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/boxAgainstJava/reflection/properties/equalsHashCodeToString.kt"); + doTestAgainstJava(fileName); + } + } } @TestMetadata("compiler/testData/codegen/boxAgainstJava/sam") diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/DescriptorBasedProperty.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/DescriptorBasedProperty.kt index eb353eb577e..99e5e0bf6d9 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/DescriptorBasedProperty.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/DescriptorBasedProperty.kt @@ -24,44 +24,48 @@ import org.jetbrains.kotlin.serialization.deserialization.descriptors.Deserializ import org.jetbrains.kotlin.serialization.jvm.JvmProtoBuf import java.lang.reflect.Field import java.lang.reflect.Method -import kotlin.reflect.KotlinReflectionInternalError abstract class DescriptorBasedProperty(computeDescriptor: () -> PropertyDescriptor) { protected abstract val container: KCallableContainerImpl - data class PropertyProtoData( + protected abstract val name: String + + private data class PropertyProtoData( val proto: ProtoBuf.Callable, val nameResolver: NameResolver, val signature: JvmProtoBuf.JvmPropertySignature ) - protected val descriptor: PropertyDescriptor by ReflectProperties.lazySoft { computeDescriptor() } + protected val descriptor: PropertyDescriptor by ReflectProperties.lazySoft(computeDescriptor) - protected val protoData: PropertyProtoData by ReflectProperties.lazyWeak { + // null if this is a property declared in a foreign (Java) class + private val protoData: PropertyProtoData? by ReflectProperties.lazyWeak @p {(): PropertyProtoData? -> val property = DescriptorUtils.unwrapFakeOverride(descriptor) as? DeserializedPropertyDescriptor - ?: throw KotlinReflectionInternalError("Member property resolved incorrectly: $descriptor") - val proto = property.proto - if (!proto.hasExtension(JvmProtoBuf.propertySignature)) { - throw KotlinReflectionInternalError("Member property lacks JVM signature: $descriptor") + if (property != null) { + val proto = property.proto + if (proto.hasExtension(JvmProtoBuf.propertySignature)) { + return@p PropertyProtoData(proto, property.nameResolver, proto.getExtension(JvmProtoBuf.propertySignature)) + } } - PropertyProtoData(proto, property.nameResolver, proto.getExtension(JvmProtoBuf.propertySignature)) + null } - val field: Field? by ReflectProperties.lazySoft { + open val field: Field? by ReflectProperties.lazySoft { val proto = protoData - if (!proto.signature.hasField()) null + if (proto == null) container.jClass.getField(name) + else if (!proto.signature.hasField()) null else container.findFieldBySignature(proto.proto, proto.signature.getField(), proto.nameResolver) } open val getter: Method? by ReflectProperties.lazySoft { val proto = protoData - if (!proto.signature.hasGetter()) null + if (proto == null || !proto.signature.hasGetter()) null else container.findMethodBySignature(proto.signature.getGetter(), proto.nameResolver) } open val setter: Method? by ReflectProperties.lazySoft { val proto = protoData - if (!proto.signature.hasSetter()) null + if (proto == null || !proto.signature.hasSetter()) null else container.findMethodBySignature(proto.signature.getSetter(), proto.nameResolver) } } diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KClassImpl.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KClassImpl.kt index f9b1d9207a2..a13c8c961ac 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KClassImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KClassImpl.kt @@ -16,26 +16,18 @@ package kotlin.reflect.jvm.internal -import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.resolve.scopes.JetScope import org.jetbrains.kotlin.serialization.deserialization.findClassAcrossModuleDependencies -import kotlin.jvm.internal.KotlinClass import kotlin.reflect.KClass import kotlin.reflect.KMemberProperty import kotlin.reflect.KMutableMemberProperty import kotlin.reflect.KotlinReflectionInternalError -enum class KClassOrigin { - BUILT_IN - KOTLIN - FOREIGN -} - class KClassImpl(override val jClass: Class) : KCallableContainerImpl(), KClass { // Don't use kotlin.properties.Delegates here because it's a Kotlin class which will invoke KClassImpl() in , // resulting in infinite recursion - val descriptor by ReflectProperties.lazySoft {(): ClassDescriptor -> + val descriptor by ReflectProperties.lazySoft { val moduleData = jClass.getOrCreateModule() val classId = RuntimeTypeMapper.mapJvmClassToKotlinClassId(jClass) @@ -48,36 +40,12 @@ class KClassImpl(override val jClass: Class) : KCallableContainerImpl(), K override val scope: JetScope get() = descriptor.getDefaultType().getMemberScope() - private val origin by ReflectProperties.lazy {(): KClassOrigin -> - if (jClass.isAnnotationPresent(javaClass())) { - KClassOrigin.KOTLIN - } - else { - KClassOrigin.FOREIGN - // TODO: built-in classes - } - } - fun memberProperty(name: String): KMemberProperty { - val computeDescriptor = findPropertyDescriptor(name) - - if (origin === KClassOrigin.KOTLIN) { - return KMemberPropertyImpl(this, computeDescriptor) - } - else { - return KForeignMemberProperty(name, this) - } + return KMemberPropertyImpl(this, findPropertyDescriptor(name)) } fun mutableMemberProperty(name: String): KMutableMemberProperty { - val computeDescriptor = findPropertyDescriptor(name) - - if (origin === KClassOrigin.KOTLIN) { - return KMutableMemberPropertyImpl(this, computeDescriptor) - } - else { - return KMutableForeignMemberProperty(name, this) - } + return KMutableMemberPropertyImpl(this, findPropertyDescriptor(name)) } override fun equals(other: Any?): Boolean = diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KForeignMemberProperty.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KForeignMemberProperty.kt deleted file mode 100644 index 2887cad2f3d..00000000000 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KForeignMemberProperty.kt +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Copyright 2010-2015 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package kotlin.reflect.jvm.internal - -import java.lang.reflect.* -import kotlin.reflect.* - -open class KForeignMemberProperty( - override val name: String, - protected val owner: KClassImpl -) : KMemberProperty, KPropertyImpl { - override val field: Field = try { - owner.jClass.getField(name) - } - catch (e: NoSuchFieldException) { - throw NoSuchPropertyException(e) - } - - override val getter: Method? get() = null - - override fun get(receiver: T): R { - try { - return field.get(receiver) as R - } - catch (e: IllegalAccessException) { - throw IllegalPropertyAccessException(e) - } - } - - override fun equals(other: Any?): Boolean = - other is KForeignMemberProperty<*, *> && name == other.name && owner == other.owner - - override fun hashCode(): Int = - name.hashCode() * 31 + owner.hashCode() - - // TODO: include visibility, return type - override fun toString(): String = - "val ${owner.jClass.getName()}.$name" -} - -class KMutableForeignMemberProperty( - name: String, - owner: KClassImpl -) : KMutableMemberProperty, KMutablePropertyImpl, KForeignMemberProperty(name, owner) { - override val setter: Method? get() = null - - override fun set(receiver: T, value: R) { - try { - field.set(receiver, value) - } - catch (e: IllegalAccessException) { - throw IllegalPropertyAccessException(e) - } - } - - override fun toString(): String = - "var ${owner.jClass.getName()}.$name" -} diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/properties.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/properties.kt index b3201c0d6bd..7b2256814dc 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/properties.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/properties.kt @@ -16,8 +16,9 @@ package kotlin.reflect.jvm -import kotlin.reflect.* -import kotlin.reflect.jvm.internal.* +import kotlin.reflect.KProperty +import kotlin.reflect.jvm.internal.KMemberPropertyImpl +import kotlin.reflect.jvm.internal.KMutableMemberPropertyImpl public var KProperty.accessible: Boolean get() { @@ -29,7 +30,6 @@ public var KProperty.accessible: Boolean 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 true @@ -47,8 +47,5 @@ public var KProperty.accessible: Boolean field?.setAccessible(value) getter?.setAccessible(value) } - is KForeignMemberProperty<*, R> -> { - field.setAccessible(value) - } } }