From 469224789708fd9870e3e91fb136e30fc133e5ff Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Wed, 13 Feb 2019 18:57:25 +0100 Subject: [PATCH] Do not check backing field presence in AnnotationSplitter This code can be invoked early, during body resolution and before the fact that a property has backing field (which is only known for certain after body resolution, because an implicit 'field' identifier may be used). Since split annotations are cached until the end of the program, they may end up on incorrect elements in the bytecode (or disappear completely) as in KT-29507 or KT-28182. Because the FIELD target has the lowest priority among implicit annotation targets (see TARGET_PRIORITIES), it's safe to always assume that FIELD is a valid target when splitting annotations. This only changes the way annotations are split in case of incorrect code, as changes in test data show. #KT-28182 Fixed #KT-29507 Fixed --- .../annotations/AnnotationSplitter.kt | 48 +++++++------------ .../kotlin/resolve/DescriptorResolver.java | 6 +-- .../annotations/options/targets/field.txt | 12 ++--- .../withUseSiteTarget/FieldAnnotations.txt | 4 +- .../jvmField/jvmFieldApplicability.kt | 2 +- .../jvmField/jvmFieldApplicability.txt | 12 ++--- 6 files changed, 34 insertions(+), 50 deletions(-) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/descriptors/annotations/AnnotationSplitter.kt b/compiler/frontend/src/org/jetbrains/kotlin/descriptors/annotations/AnnotationSplitter.kt index 23b7dce941b..07ea3b123b6 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/descriptors/annotations/AnnotationSplitter.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/descriptors/annotations/AnnotationSplitter.kt @@ -22,7 +22,6 @@ import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.KtDeclaration import org.jetbrains.kotlin.psi.KtProperty import org.jetbrains.kotlin.resolve.AnnotationChecker -import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.lazy.LazyEntity import org.jetbrains.kotlin.storage.StorageManager import org.jetbrains.kotlin.storage.getValue @@ -49,39 +48,26 @@ class AnnotationSplitter( private val TARGET_PRIORITIES = setOf(CONSTRUCTOR_PARAMETER, PROPERTY, FIELD) @JvmStatic - fun create( - storageManager: StorageManager, - annotations: Annotations, - targets: Set - ): AnnotationSplitter { - return AnnotationSplitter(storageManager, annotations, { targets }) - } + fun create(storageManager: StorageManager, annotations: Annotations, targets: Set): AnnotationSplitter = + AnnotationSplitter(storageManager, annotations) { targets } @JvmStatic - fun getTargetSet(parameter: Boolean, context: BindingContext, wrapper: PropertyWrapper): Set { - val descriptor = wrapper.descriptor - assert(descriptor != null) - val hasBackingField = context[BindingContext.BACKING_FIELD_REQUIRED, descriptor] ?: false - val hasDelegate = wrapper.declaration is KtProperty && wrapper.declaration.hasDelegate() - return getTargetSet(parameter, descriptor!!.isVar, hasBackingField, hasDelegate) - } - - @JvmStatic - fun getTargetSet( - parameter: Boolean, isVar: Boolean, hasBackingField: Boolean, hasDelegate: Boolean - ): Set = hashSetOf(PROPERTY, PROPERTY_GETTER).apply { - if (parameter) { - add(CONSTRUCTOR_PARAMETER) - add(PROPERTY_GETTER) - add(PROPERTY_SETTER) + fun getTargetSet(parameter: Boolean, wrapper: PropertyWrapper): Set = + hashSetOf(PROPERTY, PROPERTY_GETTER).apply { + if (parameter) { + add(CONSTRUCTOR_PARAMETER) + add(PROPERTY_GETTER) + add(PROPERTY_SETTER) + } + add(FIELD) + if (wrapper.descriptor!!.isVar) { + add(PROPERTY_SETTER) + add(SETTER_PARAMETER) + } + if (wrapper.declaration is KtProperty && wrapper.declaration.hasDelegate()) { + add(PROPERTY_DELEGATE_FIELD) + } } - if (hasBackingField) add(FIELD) - if (isVar) { - add(PROPERTY_SETTER) - add(SETTER_PARAMETER) - } - if (hasDelegate) add(PROPERTY_DELEGATE_FIELD) - } } class PropertyWrapper @JvmOverloads constructor(val declaration: KtDeclaration, var descriptor: PropertyDescriptor? = null) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DescriptorResolver.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DescriptorResolver.java index eae48a2e2b0..600674e55f2 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DescriptorResolver.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DescriptorResolver.java @@ -872,8 +872,7 @@ public class DescriptorResolver { Annotations allAnnotations = annotationResolver.resolveAnnotationsWithoutArguments(scopeForDeclarationResolution, modifierList, trace); AnnotationSplitter annotationSplitter = - new AnnotationSplitter(storageManager, allAnnotations, - () -> AnnotationSplitter.getTargetSet(false, trace.getBindingContext(), wrapper)); + new AnnotationSplitter(storageManager, allAnnotations, () -> AnnotationSplitter.getTargetSet(false, wrapper)); Annotations propertyAnnotations = new CompositeAnnotations(CollectionsKt.listOf( annotationSplitter.getAnnotationsForTarget(PROPERTY), @@ -1228,8 +1227,7 @@ public class DescriptorResolver { AnnotationSplitter.PropertyWrapper propertyWrapper = new AnnotationSplitter.PropertyWrapper(parameter); Annotations allAnnotations = annotationResolver.resolveAnnotationsWithoutArguments(scope, parameter.getModifierList(), trace); AnnotationSplitter annotationSplitter = - new AnnotationSplitter(storageManager, allAnnotations, - () -> AnnotationSplitter.getTargetSet(true, trace.getBindingContext(), propertyWrapper)); + new AnnotationSplitter(storageManager, allAnnotations, () -> AnnotationSplitter.getTargetSet(true, propertyWrapper)); Annotations propertyAnnotations = new CompositeAnnotations( annotationSplitter.getAnnotationsForTarget(PROPERTY), diff --git a/compiler/testData/diagnostics/tests/annotations/options/targets/field.txt b/compiler/testData/diagnostics/tests/annotations/options/targets/field.txt index 1e6ceea09ec..2b5f6d9550c 100644 --- a/compiler/testData/diagnostics/tests/annotations/options/targets/field.txt +++ b/compiler/testData/diagnostics/tests/annotations/options/targets/field.txt @@ -1,7 +1,7 @@ package @field:Field public val x: kotlin.Int = 42 -@Field public val y: kotlin.Int +@field:Field public val y: kotlin.Int @Field public final annotation class Another : kotlin.Annotation { public constructor Another() @@ -18,8 +18,8 @@ package } public interface His { - @Field public abstract val x: kotlin.Int - @Field public open val y: kotlin.Int + @field:Field public abstract val x: kotlin.Int + @field:Field public open val y: kotlin.Int public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String @@ -27,11 +27,11 @@ public interface His { @Field public abstract class My { public constructor My(/*0*/ @Field arg: kotlin.Int, /*1*/ w: kotlin.Int) - @Field public final val v: kotlin.Int + @field:Field public final val v: kotlin.Int @field:Field public final val w: kotlin.Int @field:Field public final val x: kotlin.Int - @Field public final val y: kotlin.Int - @Field public abstract val z: kotlin.Int + @field:Field public final val y: kotlin.Int + @field:Field public abstract val z: kotlin.Int public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean @Field public final fun foo(): kotlin.Unit public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int diff --git a/compiler/testData/diagnostics/tests/annotations/withUseSiteTarget/FieldAnnotations.txt b/compiler/testData/diagnostics/tests/annotations/withUseSiteTarget/FieldAnnotations.txt index f9ccb8f496a..d7f83e859ad 100644 --- a/compiler/testData/diagnostics/tests/annotations/withUseSiteTarget/FieldAnnotations.txt +++ b/compiler/testData/diagnostics/tests/annotations/withUseSiteTarget/FieldAnnotations.txt @@ -17,8 +17,8 @@ public final class CustomDelegate { @Ann public final class SomeClass { public constructor SomeClass() - protected final val delegatedProperty: kotlin.String - public final val propertyWithCustomGetter: kotlin.Int + @field:Ann protected final val delegatedProperty: kotlin.String + @field:Ann public final val propertyWithCustomGetter: kotlin.Int @field:Ann protected final val simpleProperty: kotlin.String = "text" @field:Ann protected final val simplePropertyWithAnnotationList: kotlin.String = "text" public final fun anotherFun(/*0*/ s: kotlin.String): kotlin.Unit diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmField/jvmFieldApplicability.kt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmField/jvmFieldApplicability.kt index cbea3f5e0ab..b92ad1e8d35 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmField/jvmFieldApplicability.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmField/jvmFieldApplicability.kt @@ -17,7 +17,7 @@ abstract class C : I{ @kotlin.jvm.JvmField private fun foo(s: String = "OK") { } - @JvmField val a: String by lazy { "A" } + @JvmField val a: String by lazy { "A" } @JvmField open val b: Int = 3 diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmField/jvmFieldApplicability.txt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmField/jvmFieldApplicability.txt index 7437e96135e..2926afabe09 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmField/jvmFieldApplicability.txt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmField/jvmFieldApplicability.txt @@ -7,18 +7,18 @@ package @kotlin.jvm.JvmField public abstract class C : I { @kotlin.jvm.JvmField public constructor C(/*0*/ s: kotlin.String) - @kotlin.jvm.JvmField public final val a: kotlin.String + @field:kotlin.jvm.JvmField public final val a: kotlin.String @field:kotlin.jvm.JvmField public final override /*1*/ val ai: kotlin.Int = 3 @field:kotlin.jvm.JvmField public open val b: kotlin.Int = 3 - @kotlin.jvm.JvmField public open override /*1*/ /*fake_override*/ val bi: kotlin.Int - @kotlin.jvm.JvmField public abstract val c: kotlin.Int + @field:kotlin.jvm.JvmField public open override /*1*/ /*fake_override*/ val bi: kotlin.Int + @field:kotlin.jvm.JvmField public abstract val c: kotlin.Int @field:kotlin.jvm.JvmField public final val customGetter: kotlin.String = "" @field:kotlin.jvm.JvmField public final var customSetter: kotlin.String @field:kotlin.jvm.JvmField @get:DemoAnnotation public final val explicitDefaultAnnotatedGetter: kotlin.String = "" @field:kotlin.jvm.JvmField @set:DemoAnnotation public final var explicitDefaultAnnotatedSetter: kotlin.String @field:kotlin.jvm.JvmField public final val explicitDefaultGetter: kotlin.String = "" @field:kotlin.jvm.JvmField public final var explicitDefaultSetter: kotlin.String - @kotlin.jvm.JvmField public final val noBackingField: kotlin.String + @field:kotlin.jvm.JvmField public final val noBackingField: kotlin.String @field:kotlin.jvm.JvmField private final val private: kotlin.Int = 3 public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean @kotlin.jvm.JvmField private final fun foo(/*0*/ s: kotlin.String = ...): kotlin.Unit @@ -66,8 +66,8 @@ public final class H { } public interface I { - @kotlin.jvm.JvmField public abstract val ai: kotlin.Int - @kotlin.jvm.JvmField public open val bi: kotlin.Int + @field:kotlin.jvm.JvmField public abstract val ai: kotlin.Int + @field:kotlin.jvm.JvmField public open val bi: kotlin.Int public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String