From 1e675bdc01b18c7e1b401ed8629c190abe73ecb6 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Mon, 16 Jul 2018 19:24:17 +0200 Subject: [PATCH] Simplify implementation of JvmField property in interface companion Move parts of the logic to the only places where they're needed: checking for public/final/val is only needed in JvmFieldApplicabilityChecker, checking the proto flag is only needed in reflection, checking the JvmField annotation presence is only needed in backend --- .../codegen/ImplementationBodyCodegen.java | 21 +++++- .../kotlin/codegen/PropertyCodegen.java | 8 +-- .../jetbrains/kotlin/codegen/codegenUtil.kt | 2 +- .../JvmSerializationBindings.java | 2 - .../serialization/JvmSerializerExtension.kt | 18 +++-- .../checkers/JvmFieldApplicabilityChecker.kt | 18 ++++- .../jetbrains/kotlin/load/java/JvmAbi.java | 65 ++++--------------- 7 files changed, 64 insertions(+), 70 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java index c33adaeb952..ebf7cc3e7b3 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java @@ -47,6 +47,8 @@ import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmClassSignature; import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodParameterKind; import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodParameterSignature; import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodSignature; +import org.jetbrains.kotlin.resolve.scopes.DescriptorKindFilter; +import org.jetbrains.kotlin.resolve.scopes.MemberScope; import org.jetbrains.kotlin.resolve.scopes.receivers.ExtensionReceiver; import org.jetbrains.kotlin.resolve.scopes.receivers.ImplicitReceiver; import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue; @@ -928,9 +930,13 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { boolean isNonCompanionObject = isNonCompanionObject(descriptor); boolean isInterfaceCompanion = isCompanionObjectInInterfaceNotIntrinsic(descriptor); + boolean isInterfaceCompanionWithBackingFieldsInOuter = isInterfaceCompanionWithBackingFieldsInOuter(descriptor); boolean isMappedIntrinsicCompanionObject = isMappedIntrinsicCompanionObject(descriptor); - boolean objectWithBackingFieldsInOuter = isCompanionObjectWithBackingFieldsInOuter(descriptor); - if (isNonCompanionObject || (isInterfaceCompanion && !objectWithBackingFieldsInOuter) || isMappedIntrinsicCompanionObject) { + boolean isClassCompanionWithBackingFieldsInOuter = isClassCompanionObjectWithBackingFieldsInOuter(descriptor); + if (isNonCompanionObject || + (isInterfaceCompanion && !isInterfaceCompanionWithBackingFieldsInOuter) || + isMappedIntrinsicCompanionObject + ) { ExpressionCodegen clInitCodegen = createOrGetClInitCodegen(); InstructionAdapter v = clInitCodegen.v; markLineNumberForElement(element.getPsiOrParent(), v); @@ -958,7 +964,7 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { ); } } - else if (objectWithBackingFieldsInOuter) { + else if (isClassCompanionWithBackingFieldsInOuter || isInterfaceCompanionWithBackingFieldsInOuter) { ImplementationBodyCodegen parentCodegen = (ImplementationBodyCodegen) getParentCodegen(); ExpressionCodegen parentClInitCodegen = parentCodegen.createOrGetClInitCodegen(); InstructionAdapter parentVisitor = parentClInitCodegen.v; @@ -977,6 +983,15 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { } } + private static boolean isInterfaceCompanionWithBackingFieldsInOuter(@NotNull DeclarationDescriptor declarationDescriptor) { + DeclarationDescriptor interfaceClass = declarationDescriptor.getContainingDeclaration(); + if (!isCompanionObject(declarationDescriptor) || !isJvmInterface(interfaceClass)) return false; + + Collection descriptors = ((ClassDescriptor) declarationDescriptor).getUnsubstitutedMemberScope() + .getContributedDescriptors(DescriptorKindFilter.ALL, MemberScope.Companion.getALL_NAME_FILTER()); + return CollectionsKt.any(descriptors, d -> d instanceof PropertyDescriptor && hasJvmFieldAnnotation((PropertyDescriptor) d)); + } + private void generateCompanionObjectBackingFieldCopies() { if (companionObjectPropertiesToCopy == null) return; diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyCodegen.java index 6288b33446a..a1041001318 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyCodegen.java @@ -57,9 +57,9 @@ import static org.jetbrains.kotlin.codegen.JvmCodegenUtil.isConstOrHasJvmFieldAn import static org.jetbrains.kotlin.codegen.JvmCodegenUtil.isJvmInterface; import static org.jetbrains.kotlin.codegen.binding.CodegenBinding.DELEGATED_PROPERTIES; import static org.jetbrains.kotlin.codegen.binding.CodegenBinding.DELEGATED_PROPERTY_METADATA_OWNER; -import static org.jetbrains.kotlin.codegen.serialization.JvmSerializationBindings.*; +import static org.jetbrains.kotlin.codegen.serialization.JvmSerializationBindings.FIELD_FOR_PROPERTY; +import static org.jetbrains.kotlin.codegen.serialization.JvmSerializationBindings.SYNTHETIC_METHOD_FOR_PROPERTY; import static org.jetbrains.kotlin.diagnostics.Errors.EXPECTED_FUNCTION_SOURCE_WITH_DEFAULT_ARGUMENTS_NOT_FOUND; -import static org.jetbrains.kotlin.load.java.JvmAbi.isInterfaceCompanionWithBackingFieldsInOuter; import static org.jetbrains.kotlin.resolve.DescriptorUtils.isCompanionObject; import static org.jetbrains.kotlin.resolve.DescriptorUtils.isInterface; import static org.jetbrains.kotlin.resolve.jvm.AsmTypes.K_PROPERTY_TYPE; @@ -420,10 +420,6 @@ public class PropertyCodegen { String name = backingFieldContext.getFieldName(propertyDescriptor, isDelegate); v.getSerializationBindings().put(FIELD_FOR_PROPERTY, propertyDescriptor, Pair.create(type, name)); - if (hasJvmFieldAnnotation(propertyDescriptor) && - isInterfaceCompanionWithBackingFieldsInOuter(propertyDescriptor.getContainingDeclaration())) { - v.getSerializationBindings().put(FIELD_MOVED_FROM_INTERFACE_COMPANION, propertyDescriptor, true); - } FieldVisitor fv = builder.newField( JvmDeclarationOriginKt.OtherOrigin(element, propertyDescriptor), modifiers, name, type.getDescriptor(), diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/codegenUtil.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/codegenUtil.kt index 309a268b4b2..100f77db58e 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/codegenUtil.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/codegenUtil.kt @@ -140,7 +140,7 @@ fun populateCompanionBackingFieldNamesToOuterContextIfNeeded( return } - if (!JvmAbi.isCompanionObjectWithBackingFieldsInOuter(descriptor)) { + if (!JvmAbi.isClassCompanionObjectWithBackingFieldsInOuter(descriptor)) { return } val properties = companion.declarations.filterIsInstance() diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmSerializationBindings.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmSerializationBindings.java index 00392bb3fc9..459964efac7 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmSerializationBindings.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmSerializationBindings.java @@ -35,8 +35,6 @@ public final class JvmSerializationBindings { SerializationMappingSlice.create(); public static final SerializationMappingSlice SYNTHETIC_METHOD_FOR_PROPERTY = SerializationMappingSlice.create(); - public static final SerializationMappingSlice FIELD_MOVED_FROM_INTERFACE_COMPANION = - SerializationMappingSlice.create(); static final class SerializationMappingSlice extends BasicWritableSlice { public SerializationMappingSlice() { diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmSerializerExtension.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmSerializerExtension.kt index 4a77ec209ba..6f785f71b56 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmSerializerExtension.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmSerializerExtension.kt @@ -181,18 +181,26 @@ class JvmSerializerExtension(private val bindings: JvmSerializationBindings, sta ) proto.setExtension(JvmProtoBuf.propertySignature, signature) - val flags = JvmFlags.getPropertyFlags(bindings.get(FIELD_MOVED_FROM_INTERFACE_COMPANION, descriptor) == true) - if (flags != 0) { - proto.setExtension(JvmProtoBuf.flags, flags) - } - if (JvmAbi.isInterfaceCompanionWithBackingFieldsInOuter(descriptor.containingDeclaration)) { + if (descriptor.isJvmFieldPropertyInInterfaceCompanion()) { + proto.setExtension(JvmProtoBuf.flags, JvmFlags.getPropertyFlags(true)) + assert(!proto.hasVersionRequirement()) { "VersionRequirement should be empty for $descriptor" } proto.versionRequirement = writeVersionRequirement(1, 2, 70, ProtoBuf.VersionRequirement.VersionKind.COMPILER_VERSION, versionRequirementTable) } } + private fun PropertyDescriptor.isJvmFieldPropertyInInterfaceCompanion(): Boolean { + if (!JvmAbi.hasJvmFieldAnnotation(this)) return false + + val container = containingDeclaration + if (!DescriptorUtils.isCompanionObject(container)) return false + + val grandParent = (container as ClassDescriptor).containingDeclaration + return DescriptorUtils.isInterface(grandParent) || DescriptorUtils.isAnnotationClass(grandParent) + } + override fun serializeErrorType(type: KotlinType, builder: ProtoBuf.Type.Builder) { if (classBuilderMode === ClassBuilderMode.KAPT3) { builder.className = stringTable.getStringIndex(NON_EXISTENT_CLASS_NAME) diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JvmFieldApplicabilityChecker.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JvmFieldApplicabilityChecker.kt index 44ff6058b41..1a409133046 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JvmFieldApplicabilityChecker.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JvmFieldApplicabilityChecker.kt @@ -31,6 +31,8 @@ import org.jetbrains.kotlin.resolve.checkers.DeclarationCheckerContext import org.jetbrains.kotlin.resolve.jvm.annotations.findJvmFieldAnnotation import org.jetbrains.kotlin.resolve.jvm.checkers.JvmFieldApplicabilityChecker.Problem.* import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm +import org.jetbrains.kotlin.resolve.scopes.DescriptorKindFilter +import org.jetbrains.kotlin.resolve.scopes.MemberScope class JvmFieldApplicabilityChecker : DeclarationChecker { @@ -64,7 +66,7 @@ class JvmFieldApplicabilityChecker : DeclarationChecker { if (!context.languageVersionSettings.supportsFeature(LanguageFeature.JvmFieldInInterface)) INSIDE_COMPANION_OF_INTERFACE else { - if (!JvmAbi.isInterfaceCompanionWithBackingFieldsInOuter(descriptor.containingDeclaration)) { + if (!isInterfaceCompanionWithPublicJvmFieldProperties(descriptor.containingDeclaration as ClassDescriptor)) { NOT_PUBLIC_VAL_WITH_JVMFIELD } else return } @@ -77,6 +79,19 @@ class JvmFieldApplicabilityChecker : DeclarationChecker { context.trace.report(ErrorsJvm.INAPPLICABLE_JVM_FIELD.on(annotationEntry, problem.errorMessage)) } + private fun isInterfaceCompanionWithPublicJvmFieldProperties(companionObject: ClassDescriptor): Boolean { + for (next in companionObject.unsubstitutedMemberScope.getContributedDescriptors( + DescriptorKindFilter.VARIABLES, MemberScope.ALL_NAME_FILTER + )) { + if (next !is PropertyDescriptor) continue + + if (next.visibility != Visibilities.PUBLIC || next.isVar || next.modality != Modality.FINAL) return false + + if (!JvmAbi.hasJvmFieldAnnotation(next)) return false + } + return true + } + private fun PropertyDescriptor.hasCustomAccessor() = !(getter?.isDefault ?: true) || !(setter?.isDefault ?: true) private fun PropertyDescriptor.hasBackingField(bindingContext: BindingContext) = @@ -89,5 +104,4 @@ class JvmFieldApplicabilityChecker : DeclarationChecker { val outerClassKind = (containingClass.containingDeclaration as? ClassDescriptor)?.kind return outerClassKind == ClassKind.INTERFACE || outerClassKind == ClassKind.ANNOTATION_CLASS } - } diff --git a/core/descriptors.jvm/src/org/jetbrains/kotlin/load/java/JvmAbi.java b/core/descriptors.jvm/src/org/jetbrains/kotlin/load/java/JvmAbi.java index 7f1c0d3c3e1..a787d3a293a 100644 --- a/core/descriptors.jvm/src/org/jetbrains/kotlin/load/java/JvmAbi.java +++ b/core/descriptors.jvm/src/org/jetbrains/kotlin/load/java/JvmAbi.java @@ -7,23 +7,21 @@ package org.jetbrains.kotlin.load.java; import org.jetbrains.annotations.NotNull; import org.jetbrains.kotlin.builtins.CompanionObjectMapping; -import org.jetbrains.kotlin.descriptors.*; +import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor; +import org.jetbrains.kotlin.descriptors.ClassDescriptor; +import org.jetbrains.kotlin.descriptors.DeclarationDescriptor; +import org.jetbrains.kotlin.descriptors.PropertyDescriptor; import org.jetbrains.kotlin.descriptors.annotations.AnnotationUseSiteTarget; import org.jetbrains.kotlin.descriptors.annotations.AnnotationWithTarget; -import org.jetbrains.kotlin.metadata.ProtoBuf; -import org.jetbrains.kotlin.metadata.jvm.deserialization.JvmProtoBufUtil; import org.jetbrains.kotlin.name.ClassId; import org.jetbrains.kotlin.name.FqName; import org.jetbrains.kotlin.name.Name; -import org.jetbrains.kotlin.resolve.scopes.DescriptorKindFilter; -import org.jetbrains.kotlin.resolve.scopes.MemberScope; -import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedPropertyDescriptor; import org.jetbrains.kotlin.util.capitalizeDecapitalize.CapitalizeDecapitalizeKt; -import java.util.Collection; import java.util.List; -import static org.jetbrains.kotlin.resolve.DescriptorUtils.*; +import static org.jetbrains.kotlin.resolve.DescriptorUtils.isClassOrEnumClass; +import static org.jetbrains.kotlin.resolve.DescriptorUtils.isCompanionObject; public final class JvmAbi { public static final String DEFAULT_IMPLS_CLASS_NAME = "DefaultImpls"; @@ -103,59 +101,24 @@ public final class JvmAbi { } public static boolean isPropertyWithBackingFieldInOuterClass(@NotNull PropertyDescriptor propertyDescriptor) { - return propertyDescriptor.getKind() != CallableMemberDescriptor.Kind.FAKE_OVERRIDE && - (isCompanionObjectWithBackingFieldsInOuter(propertyDescriptor.getContainingDeclaration()) - || - (isCompanionObject(propertyDescriptor.getContainingDeclaration()) && - hasJvmFieldAnnotation(propertyDescriptor))); + if (propertyDescriptor.getKind() == CallableMemberDescriptor.Kind.FAKE_OVERRIDE) return false; + + if (isClassCompanionObjectWithBackingFieldsInOuter(propertyDescriptor.getContainingDeclaration())) return true; + + return isCompanionObject(propertyDescriptor.getContainingDeclaration()) && hasJvmFieldAnnotation(propertyDescriptor); } - public static boolean isCompanionObjectWithBackingFieldsInOuter(@NotNull DeclarationDescriptor companionObject) { + public static boolean isClassCompanionObjectWithBackingFieldsInOuter(@NotNull DeclarationDescriptor companionObject) { return isCompanionObject(companionObject) && - (isClassOrEnumClass(companionObject.getContainingDeclaration()) || - isInterfaceCompanionWithBackingFieldsInOuter(companionObject)) && + isClassOrEnumClass(companionObject.getContainingDeclaration()) && !isMappedIntrinsicCompanionObject((ClassDescriptor) companionObject); - - } - - public static boolean isInterfaceCompanionWithBackingFieldsInOuter(@NotNull DeclarationDescriptor declarationDescriptor) { - DeclarationDescriptor interfaceClass = declarationDescriptor.getContainingDeclaration(); - if (!isCompanionObject(declarationDescriptor) || - (!isInterface(interfaceClass) && !isAnnotationClass(interfaceClass))) { - return false; - } - Collection descriptors = ((ClassDescriptor) declarationDescriptor).getUnsubstitutedMemberScope() - .getContributedDescriptors(DescriptorKindFilter.ALL, MemberScope.Companion.getALL_NAME_FILTER()); - boolean hasJvmFieldAnnotation = false; - for (DeclarationDescriptor next : descriptors) { - if (!(next instanceof PropertyDescriptor)) continue; - PropertyDescriptor propertyDescriptor = (PropertyDescriptor) next; - - if (propertyDescriptor.getVisibility() != Visibilities.PUBLIC || - propertyDescriptor.isVar() || - propertyDescriptor.getModality() != Modality.FINAL) { - return false; - } - - if (!isMovedFromInterfaceCompanion(propertyDescriptor) && !hasJvmFieldAnnotation(propertyDescriptor)) return false; - hasJvmFieldAnnotation = true; - } - return hasJvmFieldAnnotation; - } - - private static boolean isMovedFromInterfaceCompanion(@NotNull PropertyDescriptor propertyDescriptor) { - if (propertyDescriptor instanceof DeserializedPropertyDescriptor) { - ProtoBuf.Property proto = ((DeserializedPropertyDescriptor) propertyDescriptor).getProto(); - return JvmProtoBufUtil.isMovedFromInterfaceCompanion(proto); - } - return false; } public static boolean isMappedIntrinsicCompanionObject(@NotNull ClassDescriptor companionObject) { return CompanionObjectMapping.INSTANCE.isMappedIntrinsicCompanionObject(companionObject); } - private static boolean hasJvmFieldAnnotation(@NotNull CallableMemberDescriptor memberDescriptor) { + public static boolean hasJvmFieldAnnotation(@NotNull CallableMemberDescriptor memberDescriptor) { List annotations = memberDescriptor.getAnnotations().getUseSiteTargetedAnnotations(); for (AnnotationWithTarget annotationWithTarget : annotations) { if (AnnotationUseSiteTarget.FIELD.equals(annotationWithTarget.getTarget()) &&