From 4a533168d9e92a9b545767d11f249ebe4bf0ef0c Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Mon, 28 Mar 2016 15:56:08 +0300 Subject: [PATCH] Rework const val generation in multifile classes Do not query MultifileClassCodegen#classBuilder early on: this causes the class file for the facade to be prematurely dumped to the disk in some cases, when that class file is not yet completely generated. Instead fork the logic in PropertyCodegen#generateSyntheticMethodIfNeeded: save metadata in parts, generate method in the facade --- .../kotlin/codegen/MultifileClassCodegen.kt | 4 +- .../kotlin/codegen/PropertyCodegen.java | 63 +++++++------------ .../codegen/context/CodegenContext.java | 3 +- .../context/MultifileClassPartContext.java | 9 --- .../multifileFacadeMembers.json | 3 +- .../MultifileClass.expected.kt | 2 + .../MultifileClass/MultifileClass.kt | 3 + .../MultifileClass/MultifileClass.kt | 6 +- .../MultifileClass/MultifileClass.txt | 14 +++++ 9 files changed, 52 insertions(+), 55 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/MultifileClassCodegen.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/MultifileClassCodegen.kt index 8b856acd4af..efa5cb6ea7e 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/MultifileClassCodegen.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/MultifileClassCodegen.kt @@ -73,7 +73,7 @@ class MultifileClassCodegen( private fun getDeserializedCallables(compiledPackageFragment: PackageFragmentDescriptor) = compiledPackageFragment.getMemberScope().getContributedDescriptors(DescriptorKindFilter.CALLABLES, MemberScope.ALL_NAME_FILTER).filterIsInstance() - val classBuilder = ClassBuilderOnDemand { + private val classBuilder = ClassBuilderOnDemand { val originFile = files.firstOrNull() val actualPackageFragment = packageFragment ?: compiledPackageFragment ?: @@ -163,7 +163,7 @@ class MultifileClassCodegen( var generatePart = false val partClassInfo = state.fileClassesProvider.getFileClassInfo(file) val partType = AsmUtil.asmTypeByFqNameWithoutInnerClasses(partClassInfo.fileClassFqName) - val partContext = state.rootContext.intoMultifileClassPart(this, packageFragment, facadeClassType, partType, file) + val partContext = state.rootContext.intoMultifileClassPart(packageFragment, facadeClassType, partType, file) for (declaration in file.declarations) { when (declaration) { diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyCodegen.java index fc0ac4a8836..5ad439bae76 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyCodegen.java @@ -119,11 +119,8 @@ public class PropertyCodegen { assert kind == OwnerKind.PACKAGE || kind == OwnerKind.IMPLEMENTATION || kind == OwnerKind.DEFAULT_IMPLS : "Generating property with a wrong kind (" + kind + "): " + descriptor; - if (CodegenContextUtil.isImplClassOwner(context)) { - assert declaration != null : "Declaration is null for different context: " + context; - - genBackingFieldAndAnnotations(declaration, descriptor, false); - } + assert declaration != null : "Declaration is null: " + descriptor + " (context=" + context + ")"; + genBackingFieldAndAnnotations(declaration, descriptor, false); if (isAccessorNeeded(declaration, descriptor, getter)) { generateGetter(declaration, descriptor, getter); @@ -134,9 +131,6 @@ public class PropertyCodegen { } private void genBackingFieldAndAnnotations(@NotNull KtNamedDeclaration declaration, @NotNull PropertyDescriptor descriptor, boolean isParameter) { - ClassBuilder builder = getCorrectClassBuilder(descriptor); - if (builder == null) return; - boolean hasBackingField = hasBackingField(declaration, descriptor); boolean hasDelegate = declaration instanceof KtProperty && ((KtProperty) declaration).hasDelegate(); @@ -145,21 +139,23 @@ public class PropertyCodegen { descriptor.getAnnotations(), AnnotationSplitter.getTargetSet(isParameter, descriptor.isVar(), hasBackingField, hasDelegate)); - Annotations fieldAnnotations = annotationSplitter.getAnnotationsForTarget(AnnotationUseSiteTarget.FIELD); - Annotations delegateAnnotations = annotationSplitter.getAnnotationsForTarget(AnnotationUseSiteTarget.PROPERTY_DELEGATE_FIELD); Annotations propertyAnnotations = annotationSplitter.getAnnotationsForTarget(AnnotationUseSiteTarget.PROPERTY); - generateBackingField(builder, declaration, descriptor, fieldAnnotations, delegateAnnotations); - generateSyntheticMethodIfNeeded(builder, descriptor, propertyAnnotations); - } + // Fields and '$annotations' methods for const properties are generated in the multi-file facade + boolean isBackingFieldOwner = + descriptor.isConst() ? !(context instanceof MultifileClassPartContext) : CodegenContextUtil.isImplClassOwner(context); - @Nullable - private ClassBuilder getCorrectClassBuilder(@NotNull PropertyDescriptor descriptor) { - if (descriptor.isConst() && context instanceof MultifileClassPartContext) { - return ((MultifileClassPartContext) context).getMultifileClassCodegen().getClassBuilder(); + if (isBackingFieldOwner) { + Annotations fieldAnnotations = annotationSplitter.getAnnotationsForTarget(AnnotationUseSiteTarget.FIELD); + Annotations delegateAnnotations = annotationSplitter.getAnnotationsForTarget(AnnotationUseSiteTarget.PROPERTY_DELEGATE_FIELD); + generateBackingField(declaration, descriptor, fieldAnnotations, delegateAnnotations); + generateSyntheticMethodIfNeeded(descriptor, propertyAnnotations); } - return this.v; + if (!propertyAnnotations.getAllAnnotations().isEmpty() && kind != OwnerKind.DEFAULT_IMPLS && + CodegenContextUtil.isImplClassOwner(context)) { + v.getSerializationBindings().put(SYNTHETIC_METHOD_FOR_PROPERTY, descriptor, getSyntheticMethodSignature(descriptor)); + } } /** @@ -245,7 +241,6 @@ public class PropertyCodegen { } private boolean generateBackingField( - @NotNull ClassBuilder builder, @NotNull KtNamedDeclaration p, @NotNull PropertyDescriptor descriptor, @NotNull Annotations backingFieldAnnotations, @@ -256,10 +251,10 @@ public class PropertyCodegen { } if (p instanceof KtProperty && ((KtProperty) p).hasDelegate()) { - generatePropertyDelegateAccess(builder, (KtProperty) p, descriptor, delegateAnnotations); + generatePropertyDelegateAccess((KtProperty) p, descriptor, delegateAnnotations); } else if (Boolean.TRUE.equals(bindingContext.get(BindingContext.BACKING_FIELD_REQUIRED, descriptor))) { - generateBackingFieldAccess(builder, p, descriptor, backingFieldAnnotations); + generateBackingFieldAccess(p, descriptor, backingFieldAnnotations); } else { return false; @@ -269,29 +264,20 @@ public class PropertyCodegen { // Annotations on properties are stored in bytecode on an empty synthetic method. This way they're still // accessible via reflection, and 'deprecated' and 'private' flags prevent this method from being called accidentally - private void generateSyntheticMethodIfNeeded( - @NotNull ClassBuilder builder, - @NotNull PropertyDescriptor descriptor, - @NotNull Annotations annotations - ) { + private void generateSyntheticMethodIfNeeded(@NotNull PropertyDescriptor descriptor, @NotNull Annotations annotations) { if (annotations.getAllAnnotations().isEmpty()) return; - Method syntheticMethod = getSyntheticMethodSignature(descriptor); - if (!isInterface(context.getContextDescriptor()) || kind == OwnerKind.DEFAULT_IMPLS) { int flags = ACC_DEPRECATED | ACC_FINAL | ACC_PRIVATE | ACC_STATIC | ACC_SYNTHETIC; - MethodVisitor mv = builder.newMethod(JvmDeclarationOriginKt.OtherOrigin(descriptor), flags, syntheticMethod.getName(), - syntheticMethod.getDescriptor(), null, null); + Method syntheticMethod = getSyntheticMethodSignature(descriptor); + MethodVisitor mv = v.newMethod(JvmDeclarationOriginKt.OtherOrigin(descriptor), flags, syntheticMethod.getName(), + syntheticMethod.getDescriptor(), null, null); AnnotationCodegen.forMethod(mv, typeMapper) .genAnnotations(new AnnotatedSimple(annotations), Type.VOID_TYPE, AnnotationUseSiteTarget.PROPERTY); mv.visitCode(); mv.visitInsn(Opcodes.RETURN); mv.visitEnd(); } - - if (kind != OwnerKind.DEFAULT_IMPLS) { - v.getSerializationBindings().put(SYNTHETIC_METHOD_FOR_PROPERTY, descriptor, syntheticMethod); - } } @NotNull @@ -303,7 +289,6 @@ public class PropertyCodegen { } private void generateBackingField( - ClassBuilder builder, KtNamedDeclaration element, PropertyDescriptor propertyDescriptor, boolean isDelegate, @@ -333,6 +318,8 @@ public class PropertyCodegen { Type type = typeMapper.mapType(kotlinType); + ClassBuilder builder = v; + FieldOwnerContext backingFieldContext = context; if (AsmUtil.isInstancePropertyWithStaticBackingField(propertyDescriptor) ) { modifiers |= ACC_STATIC; @@ -365,7 +352,6 @@ public class PropertyCodegen { } private void generatePropertyDelegateAccess( - @NotNull ClassBuilder builder, @NotNull KtProperty p, @NotNull PropertyDescriptor propertyDescriptor, @NotNull Annotations annotations @@ -377,11 +363,10 @@ public class PropertyCodegen { delegateType = ErrorUtils.createErrorType("Delegate type"); } - generateBackingField(builder, p, propertyDescriptor, true, delegateType, null, annotations); + generateBackingField(p, propertyDescriptor, true, delegateType, null, annotations); } private void generateBackingFieldAccess( - @NotNull ClassBuilder builder, @NotNull KtNamedDeclaration p, @NotNull PropertyDescriptor propertyDescriptor, @NotNull Annotations annotations @@ -395,7 +380,7 @@ public class PropertyCodegen { } } - generateBackingField(builder, p, propertyDescriptor, false, propertyDescriptor.getType(), value, annotations); + generateBackingField(p, propertyDescriptor, false, propertyDescriptor.getType(), value, annotations); } private boolean shouldWriteFieldInitializer(@NotNull PropertyDescriptor descriptor) { diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/context/CodegenContext.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/context/CodegenContext.java index 754023e68ab..3361ed51f4a 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/context/CodegenContext.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/context/CodegenContext.java @@ -251,13 +251,12 @@ public abstract class CodegenContext { @NotNull public FieldOwnerContext intoMultifileClassPart( - @NotNull MultifileClassCodegen codegen, @NotNull PackageFragmentDescriptor descriptor, @NotNull Type multifileClassType, @NotNull Type filePartType, @NotNull KtFile sourceFile ) { - return new MultifileClassPartContext(codegen, descriptor, this, multifileClassType, filePartType, sourceFile); + return new MultifileClassPartContext(descriptor, this, multifileClassType, filePartType, sourceFile); } @NotNull diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/context/MultifileClassPartContext.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/context/MultifileClassPartContext.java index a6386b5466a..87ac6b61cb6 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/context/MultifileClassPartContext.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/context/MultifileClassPartContext.java @@ -18,17 +18,14 @@ package org.jetbrains.kotlin.codegen.context; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.jetbrains.kotlin.codegen.MultifileClassCodegen; import org.jetbrains.kotlin.descriptors.PackageFragmentDescriptor; import org.jetbrains.kotlin.psi.KtFile; import org.jetbrains.org.objectweb.asm.Type; public class MultifileClassPartContext extends MultifileClassContextBase implements DelegatingToPartContext, FacadePartWithSourceFile { private final KtFile sourceFile; - private final MultifileClassCodegen codegen; public MultifileClassPartContext( - @NotNull MultifileClassCodegen codegen, PackageFragmentDescriptor descriptor, CodegenContext parent, Type multifileClassType, @@ -37,7 +34,6 @@ public class MultifileClassPartContext extends MultifileClassContextBase impleme ) { super(descriptor, parent, multifileClassType, filePartType); this.sourceFile = sourceFile; - this.codegen = codegen; } @Nullable @@ -51,9 +47,4 @@ public class MultifileClassPartContext extends MultifileClassContextBase impleme public KtFile getSourceFile() { return sourceFile; } - - @NotNull - public MultifileClassCodegen getMultifileClassCodegen() { - return codegen; - } } diff --git a/compiler/testData/codegen/dumpDeclarations/multifileFacadeMembers.json b/compiler/testData/codegen/dumpDeclarations/multifileFacadeMembers.json index 1dac036134b..483250b9d18 100644 --- a/compiler/testData/codegen/dumpDeclarations/multifileFacadeMembers.json +++ b/compiler/testData/codegen/dumpDeclarations/multifileFacadeMembers.json @@ -42,12 +42,11 @@ "declaration": "package-fragment ", "class": "MultifileFacade", "members": [ - {"visibility": "public", "declaration": "const val publicConst: kotlin.Int", "name": "publicConst", "desc": "I"}, {"visibility": "internal", "declaration": "const val internalConst: kotlin.Int", "name": "internalConst", "desc": "I"}, - {"visibility": "private", "declaration": "const val privateConst: kotlin.Int", "name": "privateConst", "desc": "I"}, {"visibility": "internal", "declaration": "fun (): kotlin.Long", "name": "getInternalVal", "desc": "()J"}, {"visibility": "internal", "declaration": "fun (): kotlin.Long", "name": "getInternalVar", "desc": "()J"}, {"visibility": "internal", "declaration": "fun (: kotlin.Long): kotlin.Unit", "name": "setInternalVar", "desc": "(J)V"}, + {"visibility": "public", "declaration": "const val publicConst: kotlin.Int", "name": "publicConst", "desc": "I"}, {"visibility": "public", "declaration": "fun (): kotlin.Int", "name": "getPublicVal", "desc": "()I"}, {"visibility": "public", "declaration": "fun (): kotlin.Int", "name": "getPublicVar", "desc": "()I"}, {"visibility": "public", "declaration": "fun (: kotlin.Int): kotlin.Unit", "name": "setPublicVar", "desc": "(I)V"}, diff --git a/idea/testData/decompiler/decompiledTextJvm/MultifileClass.expected.kt b/idea/testData/decompiler/decompiledTextJvm/MultifileClass.expected.kt index ffda79591fc..30580afff52 100644 --- a/idea/testData/decompiler/decompiledTextJvm/MultifileClass.expected.kt +++ b/idea/testData/decompiler/decompiledTextJvm/MultifileClass.expected.kt @@ -11,6 +11,8 @@ public fun fn1b(): kotlin.Unit { /* compiled code */ } public fun kotlin.String.fn2b(): kotlin.Unit { /* compiled code */ } +@kotlin.Deprecated public const val annotatedConstVal: kotlin.Int /* compiled code */ + public val val1a: kotlin.Int /* compiled code */ private val kotlin.String.val2a: kotlin.Int /* compiled code */ diff --git a/idea/testData/decompiler/decompiledTextJvm/MultifileClass/MultifileClass.kt b/idea/testData/decompiler/decompiledTextJvm/MultifileClass/MultifileClass.kt index f11fd9d7726..0568491811c 100644 --- a/idea/testData/decompiler/decompiledTextJvm/MultifileClass/MultifileClass.kt +++ b/idea/testData/decompiler/decompiledTextJvm/MultifileClass/MultifileClass.kt @@ -9,3 +9,6 @@ public fun String.fn2a() {} class ShouldNotBeVisible1 interface ShouldNotBeVisible2 + +@Deprecated("deprecated") +const val annotatedConstVal = 42 diff --git a/idea/testData/decompiler/stubBuilder/MultifileClass/MultifileClass.kt b/idea/testData/decompiler/stubBuilder/MultifileClass/MultifileClass.kt index 9cf18d88079..cf774167b4b 100644 --- a/idea/testData/decompiler/stubBuilder/MultifileClass/MultifileClass.kt +++ b/idea/testData/decompiler/stubBuilder/MultifileClass/MultifileClass.kt @@ -5,6 +5,10 @@ fun p1Fun() {} fun String.p1ExtFun() {} fun p1ExprFun(): Int = 0 fun p1FunWithParams(x: Int): Int { return x } + val p1Val: Int = 0 val String.p1ExtVal: Int get() = 0 -var p1Var: Int = 0 \ No newline at end of file +var p1Var: Int = 0 + +@Deprecated("deprecated") +const val annotatedConstVal = 42 diff --git a/idea/testData/decompiler/stubBuilder/MultifileClass/MultifileClass.txt b/idea/testData/decompiler/stubBuilder/MultifileClass/MultifileClass.txt index a239ae68a8a..ab7991b8969 100644 --- a/idea/testData/decompiler/stubBuilder/MultifileClass/MultifileClass.txt +++ b/idea/testData/decompiler/stubBuilder/MultifileClass/MultifileClass.txt @@ -2,6 +2,20 @@ PsiJetFileStubImpl[package=test] PACKAGE_DIRECTIVE: REFERENCE_EXPRESSION:[referencedName=test] IMPORT_LIST: + PROPERTY:[fqName=test.annotatedConstVal, hasDelegate=false, hasDelegateExpression=false, hasInitializer=false, hasReturnTypeRef=true, isExtension=false, isTopLevel=true, isVar=false, name=annotatedConstVal] + MODIFIER_LIST:[public const] + ANNOTATION_ENTRY:[hasValueArguments=false, shortName=Deprecated] + CONSTRUCTOR_CALLEE: + TYPE_REFERENCE: + USER_TYPE:[isAbsoluteInRootPackage=false] + USER_TYPE:[isAbsoluteInRootPackage=false] + REFERENCE_EXPRESSION:[referencedName=kotlin] + REFERENCE_EXPRESSION:[referencedName=Deprecated] + TYPE_REFERENCE: + USER_TYPE:[isAbsoluteInRootPackage=false] + USER_TYPE:[isAbsoluteInRootPackage=false] + REFERENCE_EXPRESSION:[referencedName=kotlin] + REFERENCE_EXPRESSION:[referencedName=Int] PROPERTY:[fqName=test.p1Val, hasDelegate=false, hasDelegateExpression=false, hasInitializer=false, hasReturnTypeRef=true, isExtension=false, isTopLevel=true, isVar=false, name=p1Val] MODIFIER_LIST:[public] TYPE_REFERENCE: