From 0cb48999ff67a3a689893c0d70286b20d98e58b8 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Tue, 17 Dec 2019 19:25:37 +0100 Subject: [PATCH] JVM IR: retain optional annotations as package private classes Similarly to how it's done in the old backend (see PackageCodegenImpl.generateClassesAndObjectsInFile and AnnotationCodegen.genAnnotation) --- .../lower/ExpectDeclarationsRemoveLowering.kt | 4 ++-- .../jetbrains/kotlin/backend/jvm/JvmLower.kt | 4 ++-- .../backend/jvm/codegen/AnnotationCodegen.kt | 8 +++++++ .../ir/util/ExpectDeclarationRemover.kt | 23 ++++++++++++------- .../jetbrains/kotlin/ir/backend/js/klib.kt | 2 +- .../optionalAnnotation.kt | 1 - .../multiplatform/optionalExpectation.kt | 12 ++++++++++ .../flags/WriteFlagsTestGenerated.java | 18 +++++++++++++++ .../codegen/ir/IrWriteFlagsTestGenerated.java | 18 +++++++++++++++ 9 files changed, 76 insertions(+), 14 deletions(-) create mode 100644 compiler/testData/writeFlags/multiplatform/optionalExpectation.kt diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/ExpectDeclarationsRemoveLowering.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/ExpectDeclarationsRemoveLowering.kt index 2ad2c48dd6b..5cadf6f61b3 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/ExpectDeclarationsRemoveLowering.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/ExpectDeclarationsRemoveLowering.kt @@ -15,9 +15,9 @@ import org.jetbrains.kotlin.ir.visitors.acceptVoid /** * This pass removes all declarations with `isExpect == true`. */ -class ExpectDeclarationsRemoveLowering(val context: BackendContext) : FileLoweringPass { +class ExpectDeclarationsRemoveLowering(val context: BackendContext, keepOptionalAnnotations: Boolean = false) : FileLoweringPass { - val visitor = ExpectDeclarationRemover(context.ir.symbols.externalSymbolTable, true) + val visitor = ExpectDeclarationRemover(context.ir.symbols.externalSymbolTable, doRemove = true, keepOptionalAnnotations) override fun lower(irFile: IrFile) { irFile.acceptVoid(visitor) diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt index 9f5df16e953..fb5308bf8e8 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt @@ -74,8 +74,8 @@ private val arrayConstructorPhase = makeIrFilePhase( description = "Transform `Array(size) { index -> value }` into a loop" ) -private val expectDeclarationsRemovingPhase = makeIrModulePhase( - ::ExpectDeclarationsRemoveLowering, +private val expectDeclarationsRemovingPhase = makeIrModulePhase( + { context -> ExpectDeclarationsRemoveLowering(context, keepOptionalAnnotations = true) }, name = "ExpectDeclarationsRemoving", description = "Remove expect declaration from module fragment" ) diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/AnnotationCodegen.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/AnnotationCodegen.kt index cb35841c3c4..481ff7c9ecd 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/AnnotationCodegen.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/AnnotationCodegen.kt @@ -35,6 +35,7 @@ import org.jetbrains.kotlin.ir.types.isNullable import org.jetbrains.kotlin.ir.util.* import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.name.Name +import org.jetbrains.kotlin.resolve.checkers.ExpectedActualDeclarationChecker import org.jetbrains.kotlin.synthetic.isVisibleOutside import org.jetbrains.org.objectweb.asm.AnnotationVisitor import org.jetbrains.org.objectweb.asm.Type @@ -156,6 +157,13 @@ class AnnotationCodegen( val retentionPolicy = getRetentionPolicy(annotationClass) if (retentionPolicy == RetentionPolicy.SOURCE) return null + // We do not generate annotations whose classes are optional (annotated with `@OptionalExpectation`) because if an annotation entry + // is resolved to the expected declaration, this means that annotation has no actual class, and thus should not be generated. + // (Otherwise we would've resolved the entry to the actual annotation class.) + if (ExpectedActualDeclarationChecker.isOptionalAnnotationClass(annotationClass.descriptor)) { + return null + } + innerClassConsumer.addInnerClassInfoFromAnnotation(annotationClass) val asmTypeDescriptor = typeMapper.mapType(annotation.type).descriptor diff --git a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/util/ExpectDeclarationRemover.kt b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/util/ExpectDeclarationRemover.kt index 36a507c7631..fdd91989b6a 100644 --- a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/util/ExpectDeclarationRemover.kt +++ b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/util/ExpectDeclarationRemover.kt @@ -5,6 +5,7 @@ package org.jetbrains.kotlin.ir.util +import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.MemberDescriptor import org.jetbrains.kotlin.ir.IrElement import org.jetbrains.kotlin.ir.declarations.* @@ -22,7 +23,11 @@ import org.jetbrains.kotlin.resolve.descriptorUtil.module import org.jetbrains.kotlin.resolve.multiplatform.ExpectedActualResolver // `doRemove` means should expect-declaration be removed from IR -class ExpectDeclarationRemover(val symbolTable: ReferenceSymbolTable, private val doRemove: Boolean) : IrElementVisitorVoid { +class ExpectDeclarationRemover( + val symbolTable: ReferenceSymbolTable, + private val doRemove: Boolean, + private val keepOptionalAnnotations: Boolean +) : IrElementVisitorVoid { override fun visitElement(element: IrElement) { element.acceptChildrenVoid(this) } @@ -30,12 +35,16 @@ class ExpectDeclarationRemover(val symbolTable: ReferenceSymbolTable, private va override fun visitFile(declaration: IrFile) { // All declarations with `isExpect == true` are nested into a top-level declaration with `isExpect == true`. declaration.declarations.removeAll { + // TODO: rewrite findCompatibleActualForExpected using IR structures instead of descriptors val descriptor = it.descriptor - if (descriptor is MemberDescriptor && descriptor.isExpect) { + if (descriptor !is MemberDescriptor || !descriptor.isExpect || + (keepOptionalAnnotations && descriptor is ClassDescriptor && + ExpectedActualDeclarationChecker.shouldGenerateExpectClass(descriptor)) + ) { + false + } else { copyDefaultArgumentsFromExpectToActual(it) doRemove - } else { - false } } } @@ -56,9 +65,7 @@ class ExpectDeclarationRemover(val symbolTable: ReferenceSymbolTable, private va assert(function.valueParameters[index] == declaration) if (function is IrConstructor && - ExpectedActualDeclarationChecker.isOptionalAnnotationClass( - function.descriptor.constructedClass - ) + ExpectedActualDeclarationChecker.isOptionalAnnotationClass(function.descriptor.constructedClass) ) { return } @@ -145,4 +152,4 @@ class ExpectDeclarationRemover(val symbolTable: ReferenceSymbolTable, private va else -> error(parent) } } -} \ No newline at end of file +} diff --git a/compiler/ir/serialization.js/src/org/jetbrains/kotlin/ir/backend/js/klib.kt b/compiler/ir/serialization.js/src/org/jetbrains/kotlin/ir/backend/js/klib.kt index efd8f8e9fea..29f25767c62 100644 --- a/compiler/ir/serialization.js/src/org/jetbrains/kotlin/ir/backend/js/klib.kt +++ b/compiler/ir/serialization.js/src/org/jetbrains/kotlin/ir/backend/js/klib.kt @@ -136,7 +136,7 @@ fun generateKLib( val moduleName = configuration[CommonConfigurationKeys.MODULE_NAME]!! if (!configuration.klibMpp) { - moduleFragment.acceptVoid(ExpectDeclarationRemover(psi2IrContext.symbolTable, false)) + moduleFragment.acceptVoid(ExpectDeclarationRemover(psi2IrContext.symbolTable, doRemove = false, keepOptionalAnnotations = false)) } serializeModuleIntoKlib( diff --git a/compiler/testData/compileKotlinAgainstKotlin/optionalAnnotation.kt b/compiler/testData/compileKotlinAgainstKotlin/optionalAnnotation.kt index 1956de7f788..9c8bebf4988 100644 --- a/compiler/testData/compileKotlinAgainstKotlin/optionalAnnotation.kt +++ b/compiler/testData/compileKotlinAgainstKotlin/optionalAnnotation.kt @@ -1,6 +1,5 @@ // !LANGUAGE: +MultiPlatformProjects // !USE_EXPERIMENTAL: kotlin.ExperimentalMultiplatform -// IGNORE_BACKEND: JVM_IR // TARGET_BACKEND: JVM // FULL_JDK // FILE: A.kt diff --git a/compiler/testData/writeFlags/multiplatform/optionalExpectation.kt b/compiler/testData/writeFlags/multiplatform/optionalExpectation.kt new file mode 100644 index 00000000000..a9dd4446a1f --- /dev/null +++ b/compiler/testData/writeFlags/multiplatform/optionalExpectation.kt @@ -0,0 +1,12 @@ +// !LANGUAGE: +MultiPlatformProjects +// !USE_EXPERIMENTAL: kotlin.ExperimentalMultiplatform +// WITH_RUNTIME + +@file:Suppress("OPTIONAL_DECLARATION_USAGE_IN_NON_COMMON_SOURCE") // TODO: support common sources in the test infrastructure + +@OptionalExpectation +expect annotation class Anno(val s: String) + +// TESTED_OBJECT_KIND: class +// TESTED_OBJECTS: Anno +// FLAGS: ACC_INTERFACE, ACC_ABSTRACT, ACC_ANNOTATION diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/flags/WriteFlagsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/flags/WriteFlagsTestGenerated.java index 495a71c81c9..f925f0f862f 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/flags/WriteFlagsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/flags/WriteFlagsTestGenerated.java @@ -872,6 +872,24 @@ public class WriteFlagsTestGenerated extends AbstractWriteFlagsTest { } } + @TestMetadata("compiler/testData/writeFlags/multiplatform") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class Multiplatform extends AbstractWriteFlagsTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, TargetBackend.JVM, testDataFilePath); + } + + public void testAllFilesPresentInMultiplatform() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/writeFlags/multiplatform"), Pattern.compile("^(.+)\\.kt$"), null, TargetBackend.JVM, true); + } + + @TestMetadata("optionalExpectation.kt") + public void testOptionalExpectation() throws Exception { + runTest("compiler/testData/writeFlags/multiplatform/optionalExpectation.kt"); + } + } + @TestMetadata("compiler/testData/writeFlags/property") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrWriteFlagsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrWriteFlagsTestGenerated.java index d7d01083f92..fbfc1a8abf4 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrWriteFlagsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrWriteFlagsTestGenerated.java @@ -872,6 +872,24 @@ public class IrWriteFlagsTestGenerated extends AbstractIrWriteFlagsTest { } } + @TestMetadata("compiler/testData/writeFlags/multiplatform") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class Multiplatform extends AbstractIrWriteFlagsTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, TargetBackend.JVM_IR, testDataFilePath); + } + + public void testAllFilesPresentInMultiplatform() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/writeFlags/multiplatform"), Pattern.compile("^(.+)\\.kt$"), null, TargetBackend.JVM_IR, true); + } + + @TestMetadata("optionalExpectation.kt") + public void testOptionalExpectation() throws Exception { + runTest("compiler/testData/writeFlags/multiplatform/optionalExpectation.kt"); + } + } + @TestMetadata("compiler/testData/writeFlags/property") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)