From 56a51c1d223baed5d83c80f46a0aaf319908d221 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Fri, 27 Oct 2017 13:41:03 +0200 Subject: [PATCH] Do not treat annotation classes as interfaces in bridges codegen From Kotlin's point of view, everything in annotation classes is non-abstract. A class inheriting from an annotation has a non-abstract fake override for each property of the annotation class constructor. But because members of annotation classes themselves were considered as abstract in the bridge-generating code (see DescriptorBasedFunctionHandle.isAbstract), there was a situation where a concrete fake override has only one declaration among overridden descriptors and it was abstract. This situation is invalid (a concrete fake override must have exactly one concrete super-declaration), therefore an exception was thrown. The fix is to avoid considering annotation class members abstract for the purposes of bridge generation. It's reasonably safe because no bridges should be ever generated for annotation subclasses anyway, because annotations can only have members with simple return types (final and non-generic). Note that in KT-19928, the problem is reproducible because of an incorrect "inexact analysis" in light classes where "Target" is resolved to an annotation class kotlin.annotation.Target. This behavior of the analysis in light classes seems to do no harm otherwise, so it's not a goal of this commit to change anything in that regard #KT-19928 Fixed --- .../kotlin/codegen/FunctionCodegen.java | 5 +-- .../codegen/ImplementationBodyCodegen.java | 2 +- .../kotlin/codegen/JvmCodegenUtil.java | 14 +++---- .../backend/jvm/lower/BridgeLowering.kt | 9 ++-- .../annotations/inheritFromAnnotationClass.kt | 4 ++ .../inheritFromAnnotationClass.txt | 9 ++++ .../inheritFromAnnotationClass2.kt | 25 +++++++++++ .../inheritFromAnnotationClass2.txt | 42 +++++++++++++++++++ .../checkers/DiagnosticsTestGenerated.java | 12 ++++++ .../DiagnosticsUsingJavacTestGenerated.java | 12 ++++++ 10 files changed, 116 insertions(+), 18 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass.kt create mode 100644 compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass.txt create mode 100644 compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass2.kt create mode 100644 compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass2.txt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/FunctionCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/FunctionCodegen.java index dd68dfd1994..43ce22b24c0 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/FunctionCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/FunctionCodegen.java @@ -77,7 +77,6 @@ import java.util.Set; import static org.jetbrains.kotlin.builtins.KotlinBuiltIns.isNullableAny; import static org.jetbrains.kotlin.codegen.AsmUtil.*; -import static org.jetbrains.kotlin.codegen.JvmCodegenUtil.isAnnotationOrJvmInterfaceWithoutDefaults; import static org.jetbrains.kotlin.codegen.JvmCodegenUtil.isJvm8InterfaceWithDefaultsMember; import static org.jetbrains.kotlin.codegen.serialization.JvmSerializationBindings.METHOD_FOR_FUNCTION; import static org.jetbrains.kotlin.descriptors.CallableMemberDescriptor.Kind.DECLARATION; @@ -101,7 +100,7 @@ public class FunctionCodegen { private final Function1 IS_PURE_INTERFACE_CHECKER = new Function1() { @Override public Boolean invoke(DeclarationDescriptor descriptor) { - return JvmCodegenUtil.isAnnotationOrJvmInterfaceWithoutDefaults(descriptor, state); + return JvmCodegenUtil.isInterfaceWithoutDefaults(descriptor, state); } }; @@ -866,7 +865,7 @@ public class FunctionCodegen { public void generateBridges(@NotNull FunctionDescriptor descriptor) { if (descriptor instanceof ConstructorDescriptor) return; if (owner.getContextKind() == OwnerKind.DEFAULT_IMPLS) return; - if (isAnnotationOrJvmInterfaceWithoutDefaults(descriptor.getContainingDeclaration(), state)) return; + if (IS_PURE_INTERFACE_CHECKER.invoke(descriptor.getContainingDeclaration())) return; // equals(Any?), hashCode(), toString() never need bridges if (isMethodOfAny(descriptor)) return; diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java index 9fcc02e1c16..3fffcd23a7f 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java @@ -1338,7 +1338,7 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { } private void generateTraitMethods() { - if (isAnnotationOrJvmInterfaceWithoutDefaults(descriptor, state)) return; + if (isInterfaceWithoutDefaults(descriptor, state)) return; List restrictedInheritance = new ArrayList<>(); for (Map.Entry entry : CodegenUtil.getNonPrivateTraitMethods(descriptor).entrySet()) { diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/JvmCodegenUtil.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/JvmCodegenUtil.java index d0dce133e4a..5c3a7e2cc50 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/JvmCodegenUtil.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/JvmCodegenUtil.java @@ -42,7 +42,6 @@ import org.jetbrains.kotlin.resolve.BindingContext; import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils; import org.jetbrains.kotlin.resolve.DescriptorUtils; import org.jetbrains.kotlin.resolve.inline.InlineUtil; -import org.jetbrains.kotlin.resolve.jvm.JvmConstantsKt; import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue; import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedCallableMemberDescriptor; import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedClassDescriptor; @@ -63,15 +62,12 @@ public class JvmCodegenUtil { private JvmCodegenUtil() { } - public static boolean isAnnotationOrJvmInterfaceWithoutDefaults(@NotNull DeclarationDescriptor descriptor, @NotNull GenerationState state) { - return isAnnotationOrJvmInterfaceWithoutDefaults(descriptor, state.isJvm8Target(), state.isJvm8TargetWithDefaults()); + public static boolean isInterfaceWithoutDefaults(@NotNull DeclarationDescriptor descriptor, @NotNull GenerationState state) { + return isInterfaceWithoutDefaults(descriptor, state.isJvm8Target(), state.isJvm8TargetWithDefaults()); } - private static boolean isAnnotationOrJvmInterfaceWithoutDefaults(@NotNull DeclarationDescriptor descriptor, boolean isJvm8Target, boolean isJvm8TargetWithDefaults) { - if (!isJvmInterface(descriptor)) { - return false; - } - if (ANNOTATION_CLASS == ((ClassDescriptor) descriptor).getKind()) return true; + private static boolean isInterfaceWithoutDefaults(@NotNull DeclarationDescriptor descriptor, boolean isJvm8Target, boolean isJvm8TargetWithDefaults) { + if (!DescriptorUtils.isInterface(descriptor)) return false; if (descriptor instanceof DeserializedClassDescriptor) { SourceElement source = ((DeserializedClassDescriptor) descriptor).getSource(); @@ -91,7 +87,7 @@ public class JvmCodegenUtil { } public static boolean isJvm8InterfaceWithDefaults(@NotNull DeclarationDescriptor descriptor, boolean isJvm8Target, boolean isJvm8TargetWithDefaults) { - return DescriptorUtils.isInterface(descriptor) && !isAnnotationOrJvmInterfaceWithoutDefaults(descriptor, isJvm8Target, isJvm8TargetWithDefaults); + return DescriptorUtils.isInterface(descriptor) && !isInterfaceWithoutDefaults(descriptor, isJvm8Target, isJvm8TargetWithDefaults); } public static boolean isJvm8InterfaceWithDefaultsMember(@NotNull CallableMemberDescriptor descriptor, @NotNull GenerationState state) { diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/BridgeLowering.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/BridgeLowering.kt index 2b2ba58a21b..770ed6b955d 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/BridgeLowering.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/BridgeLowering.kt @@ -58,15 +58,14 @@ import org.jetbrains.org.objectweb.asm.commons.Method class BridgeLowering(val state: GenerationState) : ClassLoweringPass { - val typeMapper = state.typeMapper + private val typeMapper = state.typeMapper - private val IS_PURE_INTERFACE_CHECKER = fun(descriptor: DeclarationDescriptor): Boolean { - return JvmCodegenUtil.isAnnotationOrJvmInterfaceWithoutDefaults(descriptor, state) - } + private val IS_PURE_INTERFACE_CHECKER = fun(descriptor: DeclarationDescriptor): Boolean = + JvmCodegenUtil.isInterfaceWithoutDefaults(descriptor, state) override fun lower(irClass: IrClass) { val classDescriptor = irClass.descriptor - if (JvmCodegenUtil.isAnnotationOrJvmInterfaceWithoutDefaults(classDescriptor, state) || classDescriptor is FileClassDescriptor) return + if (IS_PURE_INTERFACE_CHECKER(classDescriptor) || classDescriptor is FileClassDescriptor) return if (classDescriptor is DefaultImplsClassDescriptor) { return /*TODO?*/ diff --git a/compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass.kt b/compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass.kt new file mode 100644 index 00000000000..17b9a770ad2 --- /dev/null +++ b/compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass.kt @@ -0,0 +1,4 @@ +// !DIAGNOSTICS: -FINAL_SUPERTYPE +// This error needs to be suppressed to cause light class generation + +class Foo : Target() diff --git a/compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass.txt b/compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass.txt new file mode 100644 index 00000000000..35b7a414e9f --- /dev/null +++ b/compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass.txt @@ -0,0 +1,9 @@ +package + +public final class Foo : kotlin.annotation.Target { + public constructor Foo() + public final override /*1*/ /*fake_override*/ val allowedTargets: kotlin.Array + 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 +} diff --git a/compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass2.kt b/compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass2.kt new file mode 100644 index 00000000000..c0b15937482 --- /dev/null +++ b/compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass2.kt @@ -0,0 +1,25 @@ +// !DIAGNOSTICS: -SUPERTYPES_FOR_ANNOTATION_CLASS -VIRTUAL_MEMBER_HIDDEN -FINAL_SUPERTYPE +// These errors need to be suppressed to cause light class generation +// FILE: test.kt + +annotation class Ann : Target() + +annotation class Ann2(vararg val allowedTargets: AnnotationTarget) : Target() + +interface I : J { + override fun foo(): List = throw Exception() +} +class C : I { + fun bar(): Set = throw Exception() +} +annotation class Ann3 : C() +annotation class Ann4 : I + +// FILE: J.java + +import java.util.Collection; +import kotlin.annotation.Target; + +public interface J extends Target { + Collection foo(); +} diff --git a/compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass2.txt b/compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass2.txt new file mode 100644 index 00000000000..b2400c819ee --- /dev/null +++ b/compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass2.txt @@ -0,0 +1,42 @@ +package + +public final annotation class Ann : kotlin.annotation.Target { + public constructor Ann() + public final override /*1*/ /*fake_override*/ val allowedTargets: kotlin.Array + 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 +} + +public final annotation class Ann2 : kotlin.annotation.Target { + public constructor Ann2(/*0*/ vararg allowedTargets: kotlin.annotation.AnnotationTarget /*kotlin.Array*/) + public final override /*1*/ val allowedTargets: kotlin.Array + 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 +} + +public final annotation class Ann3 : C { + public constructor Ann3() + public final override /*1*/ /*fake_override*/ fun bar(): kotlin.collections.Set + public open override /*1*/ /*fake_override*/ fun foo(): kotlin.collections.List +} + +public final annotation class Ann4 : I { + public constructor Ann4() + public open override /*1*/ /*fake_override*/ fun foo(): kotlin.collections.List +} + +public final class C : I { + public constructor C() + public final fun bar(): kotlin.collections.Set + public open override /*1*/ /*fake_override*/ fun foo(): kotlin.collections.List +} + +public interface I : J { + public open override /*1*/ fun foo(): kotlin.collections.List +} + +public interface J : Target { + public abstract fun foo(): kotlin.collections.(Mutable)Collection! +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 0742ef41725..3ed8e455d86 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -1084,6 +1084,18 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("inheritFromAnnotationClass.kt") + public void testInheritFromAnnotationClass() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass.kt"); + doTest(fileName); + } + + @TestMetadata("inheritFromAnnotationClass2.kt") + public void testInheritFromAnnotationClass2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass2.kt"); + doTest(fileName); + } + @TestMetadata("invalidTypesInAnnotationConstructor.kt") public void testInvalidTypesInAnnotationConstructor() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/annotations/invalidTypesInAnnotationConstructor.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java index e85d05d584d..73664e77e53 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java @@ -1084,6 +1084,18 @@ public class DiagnosticsUsingJavacTestGenerated extends AbstractDiagnosticsUsing doTest(fileName); } + @TestMetadata("inheritFromAnnotationClass.kt") + public void testInheritFromAnnotationClass() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass.kt"); + doTest(fileName); + } + + @TestMetadata("inheritFromAnnotationClass2.kt") + public void testInheritFromAnnotationClass2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/annotations/inheritFromAnnotationClass2.kt"); + doTest(fileName); + } + @TestMetadata("invalidTypesInAnnotationConstructor.kt") public void testInvalidTypesInAnnotationConstructor() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/annotations/invalidTypesInAnnotationConstructor.kt");