From 2f003ef545a50bf9925e9fd2028db833e3422c8a Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Fri, 5 Apr 2019 18:40:28 +0200 Subject: [PATCH] Generate classes in MultifileClassCodegen exactly the same as in PackageCodegen Two known issues with generateNonPartClassDeclarations that was here before were the fact that we didn't sort sealed classes and its subclasses which led to NoSuchMethodError (KT-27097), and the fact that we didn't skip expect classes which led to incorrect duplicate JVM class name diagnostic (KT-30843) #KT-27097 Fixed #KT-30843 Fixed --- .../kotlin/codegen/MultifileClassCodegen.kt | 18 +---- .../kotlin/codegen/PackageCodegen.java | 7 -- .../kotlin/codegen/PackageCodegenImpl.java | 68 ++++++++++--------- .../jetbrains/kotlin/codegen/codegenUtil.kt | 7 +- .../kotlin/backend/jvm/JvmIrCodegenFactory.kt | 6 -- .../asJava/builder/LightClassDataProvider.kt | 3 +- .../multifileClasses/sealedClassHierarchy.kt | 13 ++++ .../expectClassInJvmMultifileFacade.kt | 25 +++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 10 +++ .../LightAnalysisModeTestGenerated.java | 10 +++ .../ir/IrBlackBoxCodegenTestGenerated.java | 10 +++ 11 files changed, 109 insertions(+), 68 deletions(-) create mode 100644 compiler/testData/codegen/box/multifileClasses/sealedClassHierarchy.kt create mode 100644 compiler/testData/codegen/box/multiplatform/expectClassInJvmMultifileFacade.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/MultifileClassCodegen.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/MultifileClassCodegen.kt index 1cc91a73237..5ff6fe2fa4e 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/MultifileClassCodegen.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/MultifileClassCodegen.kt @@ -34,7 +34,6 @@ import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.progress.ProgressIndicatorAndCompilationCanceledStatus import org.jetbrains.kotlin.psi.KtClassOrObject import org.jetbrains.kotlin.psi.KtFile -import org.jetbrains.kotlin.psi.KtScript import org.jetbrains.kotlin.psi.KtTypeAlias import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.MemberComparator @@ -199,7 +198,7 @@ class MultifileClassCodegenImpl( val partType = Type.getObjectType(JvmFileClassUtil.getFileClassInternalName(file)) val partContext = state.rootContext.intoMultifileClassPart(packageFragment, facadeClassType, partType, file) - generateNonPartClassDeclarations(file, partContext) + PackageCodegenImpl.generateClassesAndObjectsInFile(file, partContext, state) if (!state.generateDeclaredClassFilter.shouldGeneratePackagePart(file) || !file.hasDeclarationsForPartClass()) return @@ -214,21 +213,6 @@ class MultifileClassCodegenImpl( addDelegateGenerationTasksForDeclarationsInFile(file, packageFragment, partType) } - private fun generateNonPartClassDeclarations(file: KtFile, partContext: FieldOwnerContext) { - for (declaration in file.declarations) { - when (declaration) { - is KtClassOrObject -> - if (state.generateDeclaredClassFilter.shouldGenerateClass(declaration)) { - generateClassOrObject(declaration, partContext) - } - is KtScript -> - if (state.generateDeclaredClassFilter.shouldGenerateScript(declaration)) { - ScriptCodegen.createScriptCodegen(declaration, state, partContext).generate() - } - } - } - } - private fun addDelegateGenerationTasksForDeclarationsInFile(file: KtFile, packageFragment: PackageFragmentDescriptor, partType: Type) { val facadeContext = state.rootContext.intoMultifileClass(packageFragment, facadeClassType, partType) val memberCodegen = createCodegenForDelegatesInMultifileFacade(facadeContext) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/PackageCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/PackageCodegen.java index 4e28438275c..aee93d63b96 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/PackageCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/PackageCodegen.java @@ -17,17 +17,10 @@ package org.jetbrains.kotlin.codegen; import org.jetbrains.annotations.NotNull; -import org.jetbrains.kotlin.codegen.context.PackageContext; import org.jetbrains.kotlin.descriptors.PackageFragmentDescriptor; -import org.jetbrains.kotlin.psi.KtClassOrObject; -import org.jetbrains.kotlin.psi.KtFile; - -import java.util.Collection; public interface PackageCodegen { void generate(@NotNull CompilationErrorHandler errorHandler); - void generateClassOrObject(@NotNull KtClassOrObject classOrObject, @NotNull PackageContext packagePartContext); - PackageFragmentDescriptor getPackageFragment(); } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/PackageCodegenImpl.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/PackageCodegenImpl.java index 3e543b3a613..2f82405ca09 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/PackageCodegenImpl.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/PackageCodegenImpl.java @@ -23,6 +23,7 @@ import com.intellij.util.SmartList; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.backend.common.CodegenUtil; +import org.jetbrains.kotlin.codegen.context.CodegenContext; import org.jetbrains.kotlin.codegen.context.PackageContext; import org.jetbrains.kotlin.codegen.state.GenerationState; import org.jetbrains.kotlin.descriptors.ClassDescriptor; @@ -82,9 +83,39 @@ public class PackageCodegenImpl implements PackageCodegen { } } - private void generateClassesAndObjectsInFile(@NotNull List classOrObjects, @NotNull PackageContext packagePartContext) { - for (KtClassOrObject classOrObject : CodegenUtilKt.sortTopLevelClassesAndPrepareContextForSealedClasses(classOrObjects, packagePartContext, state)) { - generateClassOrObject(classOrObject, packagePartContext); + public static void generateClassesAndObjectsInFile( + @NotNull KtFile file, + @NotNull CodegenContext context, + @NotNull GenerationState state + ) { + List classOrObjects = new ArrayList<>(); + + for (KtDeclaration declaration : file.getDeclarations()) { + if (declaration instanceof KtClassOrObject) { + ClassDescriptor descriptor = state.getBindingContext().get(BindingContext.CLASS, declaration); + if (PsiUtilsKt.hasExpectModifier(declaration) && + (descriptor == null || !ExpectedActualDeclarationChecker.shouldGenerateExpectClass(descriptor))) { + continue; + } + + KtClassOrObject classOrObject = (KtClassOrObject) declaration; + if (state.getGenerateDeclaredClassFilter().shouldGenerateClass(classOrObject)) { + classOrObjects.add(classOrObject); + } + } + else if (declaration instanceof KtScript) { + KtScript script = (KtScript) declaration; + + if (state.getGenerateDeclaredClassFilter().shouldGenerateScript(script)) { + ScriptCodegen.createScriptCodegen(script, state, context).generate(); + } + } + } + + List sortedClasses = + CodegenUtilKt.sortTopLevelClassesAndPrepareContextForSealedClasses(classOrObjects, context, state); + for (KtClassOrObject classOrObject : sortedClasses) { + MemberCodegen.genClassOrObject(context, classOrObject, state, null); } } @@ -103,31 +134,7 @@ public class PackageCodegenImpl implements PackageCodegen { CodeFragmentCodegen.createCodegen((KtCodeFragment) file, state, packagePartContext).generate(); } } else { - List classOrObjects = new ArrayList<>(); - - for (KtDeclaration declaration : file.getDeclarations()) { - if (declaration instanceof KtClassOrObject) { - ClassDescriptor descriptor = state.getBindingContext().get(BindingContext.CLASS, declaration); - if (PsiUtilsKt.hasExpectModifier(declaration) && - (descriptor == null || !ExpectedActualDeclarationChecker.shouldGenerateExpectClass(descriptor))) { - continue; - } - - KtClassOrObject classOrObject = (KtClassOrObject) declaration; - if (state.getGenerateDeclaredClassFilter().shouldGenerateClass(classOrObject)) { - classOrObjects.add(classOrObject); - } - } - else if (declaration instanceof KtScript) { - KtScript script = (KtScript) declaration; - - if (state.getGenerateDeclaredClassFilter().shouldGenerateScript(script)) { - ScriptCodegen.createScriptCodegen(script, state, packagePartContext).generate(); - } - } - } - - generateClassesAndObjectsInFile(classOrObjects, packagePartContext); + generateClassesAndObjectsInFile(file, packagePartContext, state); } if (!state.getGenerateDeclaredClassFilter().shouldGeneratePackagePart(file)) return; @@ -166,11 +173,6 @@ public class PackageCodegenImpl implements PackageCodegen { return fragments.get(0); } - @Override - public void generateClassOrObject(@NotNull KtClassOrObject classOrObject, @NotNull PackageContext packagePartContext) { - MemberCodegen.genClassOrObject(packagePartContext, classOrObject, state, null); - } - @Override public PackageFragmentDescriptor getPackageFragment() { return packageFragment; diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/codegenUtil.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/codegenUtil.kt index 5ecd3d0c2e4..32e04ba6294 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/codegenUtil.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/codegenUtil.kt @@ -8,12 +8,11 @@ package org.jetbrains.kotlin.codegen import com.intellij.psi.PsiElement import org.jetbrains.annotations.TestOnly import org.jetbrains.kotlin.builtins.KotlinBuiltIns -import org.jetbrains.kotlin.codegen.binding.CodegenBinding import org.jetbrains.kotlin.builtins.UnsignedTypes +import org.jetbrains.kotlin.codegen.binding.CodegenBinding import org.jetbrains.kotlin.codegen.context.CodegenContext import org.jetbrains.kotlin.codegen.context.FieldOwnerContext import org.jetbrains.kotlin.codegen.context.MultifileClassFacadeContext -import org.jetbrains.kotlin.codegen.context.PackageContext import org.jetbrains.kotlin.codegen.coroutines.continuationAsmType import org.jetbrains.kotlin.codegen.coroutines.unwrapInitialDescriptorForSuspendFunction import org.jetbrains.kotlin.codegen.inline.NUMBERED_FUNCTION_PREFIX @@ -169,13 +168,13 @@ fun populateCompanionBackingFieldNamesToOuterContextIfNeeded( // so that we'd generate the necessary accessor for its constructor afterwards fun sortTopLevelClassesAndPrepareContextForSealedClasses( classOrObjects: List, - packagePartContext: PackageContext, + context: CodegenContext<*>, state: GenerationState ): List { fun prepareContextIfNeeded(descriptor: ClassDescriptor?) { if (DescriptorUtils.isSealedClass(descriptor)) { // save context for sealed class - packagePartContext.intoClass(descriptor!!, OwnerKind.IMPLEMENTATION, state) + context.intoClass(descriptor!!, OwnerKind.IMPLEMENTATION, state) } } diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmIrCodegenFactory.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmIrCodegenFactory.kt index bd6e222b463..c6f28885e81 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmIrCodegenFactory.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmIrCodegenFactory.kt @@ -18,11 +18,9 @@ package org.jetbrains.kotlin.backend.jvm import org.jetbrains.kotlin.backend.common.phaser.PhaseConfig import org.jetbrains.kotlin.codegen.* -import org.jetbrains.kotlin.codegen.context.PackageContext import org.jetbrains.kotlin.codegen.state.GenerationState import org.jetbrains.kotlin.descriptors.PackageFragmentDescriptor import org.jetbrains.kotlin.name.FqName -import org.jetbrains.kotlin.psi.KtClassOrObject import org.jetbrains.kotlin.psi.KtFile import org.jetbrains.kotlin.psi2ir.Psi2IrTranslator @@ -45,10 +43,6 @@ class JvmIrCodegenFactory(private val phaseConfig: PhaseConfig) : CodegenFactory JvmBackendFacade.doGenerateFiles(files, state, errorHandler, phaseConfig) } - override fun generateClassOrObject(classOrObject: KtClassOrObject, packagePartContext: PackageContext) { - TODO() - } - override fun getPackageFragment(): PackageFragmentDescriptor { return impl.packageFragment } diff --git a/compiler/light-classes/src/org/jetbrains/kotlin/asJava/builder/LightClassDataProvider.kt b/compiler/light-classes/src/org/jetbrains/kotlin/asJava/builder/LightClassDataProvider.kt index 2df96b5d547..64d8b153990 100644 --- a/compiler/light-classes/src/org/jetbrains/kotlin/asJava/builder/LightClassDataProvider.kt +++ b/compiler/light-classes/src/org/jetbrains/kotlin/asJava/builder/LightClassDataProvider.kt @@ -27,6 +27,7 @@ import org.jetbrains.kotlin.asJava.KotlinAsJavaSupport import org.jetbrains.kotlin.asJava.LightClassGenerationSupport import org.jetbrains.kotlin.asJava.classes.getOutermostClassOrObject import org.jetbrains.kotlin.codegen.CompilationErrorHandler +import org.jetbrains.kotlin.codegen.MemberCodegen import org.jetbrains.kotlin.codegen.state.GenerationState import org.jetbrains.kotlin.fileClasses.JvmFileClassUtil import org.jetbrains.kotlin.name.FqName @@ -48,7 +49,7 @@ class LightClassDataProviderForClassOrObject( val packageCodegen = state.factory.forPackage(packageFqName, files) val packagePartType = Type.getObjectType(JvmFileClassUtil.getFileClassInternalName(file)) val context = state.rootContext.intoPackagePart(packageCodegen.packageFragment, packagePartType, file) - packageCodegen.generateClassOrObject(getOutermostClassOrObject(classOrObject), context) + MemberCodegen.genClassOrObject(context, getOutermostClassOrObject(classOrObject), state, null) state.factory.done() } } diff --git a/compiler/testData/codegen/box/multifileClasses/sealedClassHierarchy.kt b/compiler/testData/codegen/box/multifileClasses/sealedClassHierarchy.kt new file mode 100644 index 00000000000..8900cfceaca --- /dev/null +++ b/compiler/testData/codegen/box/multifileClasses/sealedClassHierarchy.kt @@ -0,0 +1,13 @@ +// IGNORE_BACKEND: JVM_IR +// TARGET_BACKEND: JVM +// WITH_RUNTIME + +@file:JvmMultifileClass +@file:JvmName("Test") +package test + +sealed class Foo(val value: String) + +class Bar : Foo("OK") + +fun box(): String = Bar().value diff --git a/compiler/testData/codegen/box/multiplatform/expectClassInJvmMultifileFacade.kt b/compiler/testData/codegen/box/multiplatform/expectClassInJvmMultifileFacade.kt new file mode 100644 index 00000000000..94315d4fc49 --- /dev/null +++ b/compiler/testData/codegen/box/multiplatform/expectClassInJvmMultifileFacade.kt @@ -0,0 +1,25 @@ +// !LANGUAGE: +MultiPlatformProjects +// IGNORE_BACKEND: JVM_IR +// TARGET_BACKEND: JVM +// WITH_RUNTIME +// FILE: common.kt + +@file:JvmMultifileClass +@file:JvmName("Test") +package test + +expect class Foo { + val value: String +} + +// FILE: jvm.kt + +@file:JvmMultifileClass +@file:JvmName("Test") +package test + +actual class Foo(actual val value: String) + +fun box(): String { + return Foo("OK").value +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index c02cb653b99..3daf89f97e0 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -15822,6 +15822,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/multifileClasses/samePartNameDifferentFacades.kt"); } + @TestMetadata("sealedClassHierarchy.kt") + public void testSealedClassHierarchy() throws Exception { + runTest("compiler/testData/codegen/box/multifileClasses/sealedClassHierarchy.kt"); + } + @TestMetadata("compiler/testData/codegen/box/multifileClasses/optimized") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) @@ -15878,6 +15883,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/multiplatform"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("expectClassInJvmMultifileFacade.kt") + public void testExpectClassInJvmMultifileFacade() throws Exception { + runTest("compiler/testData/codegen/box/multiplatform/expectClassInJvmMultifileFacade.kt"); + } + @TestMetadata("noArgActualConstructor.kt") public void testNoArgActualConstructor() throws Exception { runTest("compiler/testData/codegen/box/multiplatform/noArgActualConstructor.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index fe77d9feb44..45e1208ac2c 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -15822,6 +15822,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/multifileClasses/samePartNameDifferentFacades.kt"); } + @TestMetadata("sealedClassHierarchy.kt") + public void testSealedClassHierarchy() throws Exception { + runTest("compiler/testData/codegen/box/multifileClasses/sealedClassHierarchy.kt"); + } + @TestMetadata("compiler/testData/codegen/box/multifileClasses/optimized") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) @@ -15878,6 +15883,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/multiplatform"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("expectClassInJvmMultifileFacade.kt") + public void testExpectClassInJvmMultifileFacade() throws Exception { + runTest("compiler/testData/codegen/box/multiplatform/expectClassInJvmMultifileFacade.kt"); + } + @TestMetadata("noArgActualConstructor.kt") public void testNoArgActualConstructor() throws Exception { runTest("compiler/testData/codegen/box/multiplatform/noArgActualConstructor.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index f8d4890c05e..28bb7da6186 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -15827,6 +15827,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/multifileClasses/samePartNameDifferentFacades.kt"); } + @TestMetadata("sealedClassHierarchy.kt") + public void testSealedClassHierarchy() throws Exception { + runTest("compiler/testData/codegen/box/multifileClasses/sealedClassHierarchy.kt"); + } + @TestMetadata("compiler/testData/codegen/box/multifileClasses/optimized") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) @@ -15883,6 +15888,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/multiplatform"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM_IR, true); } + @TestMetadata("expectClassInJvmMultifileFacade.kt") + public void testExpectClassInJvmMultifileFacade() throws Exception { + runTest("compiler/testData/codegen/box/multiplatform/expectClassInJvmMultifileFacade.kt"); + } + @TestMetadata("noArgActualConstructor.kt") public void testNoArgActualConstructor() throws Exception { runTest("compiler/testData/codegen/box/multiplatform/noArgActualConstructor.kt");