From 3d1858a8c595b468f4dde91ecc3f4346cc13cd27 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 7 Feb 2019 19:20:27 +0100 Subject: [PATCH] Refactor and rename CodegenUtil.getDeclarationsToGenerate Move logic regarding expect classes to the only relevant call site at PackageCodegenImpl, and return the list of members to remove code duplication at remaining call sites --- .../kotlin/backend/common/CodegenUtil.kt | 25 +++++------ .../kotlin/codegen/MultifileClassCodegen.kt | 41 ++++++++----------- .../codegen/MultifileClassPartCodegen.kt | 8 +--- .../kotlin/codegen/PackageCodegenImpl.java | 32 +++++++++------ .../kotlin/codegen/PackagePartCodegen.java | 29 ++++--------- .../kotlin/codegen/SamWrapperCodegen.java | 6 +-- .../codegen/AbstractBlackBoxCodegenTest.java | 18 ++++---- 7 files changed, 63 insertions(+), 96 deletions(-) diff --git a/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/CodegenUtil.kt b/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/CodegenUtil.kt index 8f021dd1b36..7180ac11dcc 100644 --- a/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/CodegenUtil.kt +++ b/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/CodegenUtil.kt @@ -20,7 +20,6 @@ import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils import org.jetbrains.kotlin.resolve.DescriptorUtils import org.jetbrains.kotlin.resolve.bindingContextUtil.isUsedAsExpression import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall -import org.jetbrains.kotlin.resolve.checkers.ExpectedActualDeclarationChecker import org.jetbrains.kotlin.resolve.multiplatform.ExpectedActualResolver import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.utils.DFS @@ -169,23 +168,19 @@ object CodegenUtil { } /** - * Returns declarations in the given [file] which should be generated by the back-end. This includes all declarations - * minus all expected declarations (except annotation classes annotated with @OptionalExpectation). + * Returns functions, properties and type aliases in the given [file] which should be generated by the back-end. */ @JvmStatic - fun getDeclarationsToGenerate(file: KtFile, bindingContext: BindingContext): List = - file.declarations.filter(fun(declaration: KtDeclaration): Boolean { - if (!declaration.hasExpectModifier()) return true + fun getMemberDeclarationsToGenerate(file: KtFile): List = + file.declarations.filter { declaration -> + !declaration.hasExpectModifier() && (declaration is KtNamedFunction || declaration is KtProperty || declaration is KtTypeAlias) + } - if (declaration is KtClass) { - val descriptor = bindingContext.get(BindingContext.CLASS, declaration) - if (descriptor != null && ExpectedActualDeclarationChecker.shouldGenerateExpectClass(descriptor)) { - return true - } - } - - return false - }) + @JvmStatic + fun getMemberDescriptorsToGenerate(file: KtFile, bindingContext: BindingContext): List = + getMemberDeclarationsToGenerate(file).mapNotNull { declaration -> + bindingContext.get(BindingContext.DECLARATION_TO_DESCRIPTOR, declaration) as MemberDescriptor? + } @JvmStatic fun findExpectedFunctionForActual(descriptor: FunctionDescriptor): FunctionDescriptor? { diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/MultifileClassCodegen.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/MultifileClassCodegen.kt index 7d94958651b..84df394633b 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/MultifileClassCodegen.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/MultifileClassCodegen.kt @@ -34,7 +34,10 @@ import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader import org.jetbrains.kotlin.load.kotlin.incremental.IncrementalPackageFragmentProvider import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.progress.ProgressIndicatorAndCompilationCanceledStatus -import org.jetbrains.kotlin.psi.* +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 import org.jetbrains.kotlin.resolve.jvm.AsmTypes @@ -85,7 +88,7 @@ class MultifileClassCodegenImpl( private val partInternalNamesSorted = run { val partInternalNamesSet = hashSetOf() for (file in files) { - if (file.hasDeclarationsForPartClass(state.bindingContext)) { + if (file.hasDeclarationsForPartClass()) { partInternalNamesSet.add(JvmFileClassUtil.getFileClassInternalName(file)) } } @@ -122,7 +125,7 @@ class MultifileClassCodegenImpl( val singleSourceFile = if (previouslyCompiledCallables.isEmpty()) - files.singleOrNull { it.hasDeclarationsForPartClass(state.bindingContext) } + files.singleOrNull { it.hasDeclarationsForPartClass() } else null @@ -209,9 +212,7 @@ class MultifileClassCodegenImpl( generateNonPartClassDeclarations(file, partContext) - if (!state.generateDeclaredClassFilter.shouldGeneratePackagePart(file) || - !file.hasDeclarationsForPartClass(state.bindingContext) - ) return + if (!state.generateDeclaredClassFilter.shouldGeneratePackagePart(file) || !file.hasDeclarationsForPartClass()) return state.factory.packagePartRegistry.addPart(packageFragment.fqName, partType.internalName, facadeClassType.internalName) @@ -245,26 +246,18 @@ class MultifileClassCodegenImpl( private fun addDelegateGenerationTasksForDeclarationsInFile(file: KtFile, packageFragment: PackageFragmentDescriptor, partType: Type) { val facadeContext = state.rootContext.intoMultifileClass(packageFragment, facadeClassType, partType) val memberCodegen = createCodegenForDelegatesInMultifileFacade(facadeContext) - for (declaration in CodegenUtil.getDeclarationsToGenerate(file, state.bindingContext)) { - if (shouldGenerateInFacade(declaration)) { - val descriptor = state.bindingContext.get(BindingContext.DECLARATION_TO_DESCRIPTOR, declaration) - if (descriptor !is MemberDescriptor) { - throw AssertionError("Expected callable member, was " + descriptor + " for " + declaration.text) - } - addDelegateGenerationTaskIfNeeded(descriptor, { memberCodegen.genSimpleMember(declaration) }) + for (declaration in CodegenUtil.getMemberDeclarationsToGenerate(file)) { + // In light classes, we intentionally do not analyze type aliases, since they're metadata-only + if (declaration is KtTypeAlias && !state.classBuilderMode.generateMetadata) continue + + val descriptor = state.bindingContext.get(BindingContext.DECLARATION_TO_DESCRIPTOR, declaration) + if (descriptor !is MemberDescriptor) { + throw AssertionError("Expected callable member, was " + descriptor + " for " + declaration.text) } + addDelegateGenerationTaskIfNeeded(descriptor) { memberCodegen.genSimpleMember(declaration) } } } - private fun shouldGenerateInFacade(declaration: KtDeclaration): Boolean { - if (declaration is KtNamedFunction || declaration is KtProperty) return true - - // In light classes, we intentionally do not analyze type aliases, since they're metadata-only - if (declaration is KtTypeAlias && state.classBuilderMode.generateMetadata) return true - - return false - } - private fun shouldGenerateInFacade(descriptor: MemberDescriptor): Boolean { if (Visibilities.isPrivate(descriptor.visibility)) return false if (AsmUtil.getVisibilityAccessFlag(descriptor) == Opcodes.ACC_PRIVATE) return false @@ -386,8 +379,8 @@ class MultifileClassCodegenImpl( return fragments.firstOrNull() } - private fun KtFile.hasDeclarationsForPartClass(bindingContext: BindingContext) = - CodegenUtil.getDeclarationsToGenerate(this, bindingContext).any { it is KtProperty || it is KtFunction || it is KtTypeAlias } + private fun KtFile.hasDeclarationsForPartClass() = + CodegenUtil.getMemberDeclarationsToGenerate(this).isNotEmpty() private fun getCompiledPackageFragment( facadeFqName: FqName, state: GenerationState diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/MultifileClassPartCodegen.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/MultifileClassPartCodegen.kt index 77ccbcfcaa6..dc42f8eab96 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/MultifileClassPartCodegen.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/MultifileClassPartCodegen.kt @@ -24,9 +24,7 @@ import org.jetbrains.kotlin.descriptors.PackageFragmentDescriptor import org.jetbrains.kotlin.load.java.JvmAnnotationNames import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader import org.jetbrains.kotlin.psi.KtFile -import org.jetbrains.kotlin.psi.KtNamedFunction import org.jetbrains.kotlin.psi.KtProperty -import org.jetbrains.kotlin.psi.KtTypeAlias import org.jetbrains.kotlin.resolve.jvm.diagnostics.OtherOrigin import org.jetbrains.org.objectweb.asm.Opcodes @@ -82,10 +80,8 @@ class MultifileClassPartCodegen( } override fun generateBody() { - for (declaration in CodegenUtil.getDeclarationsToGenerate(element, state.bindingContext)) { - if (declaration is KtNamedFunction || declaration is KtProperty || declaration is KtTypeAlias) { - genSimpleMember(declaration) - } + for (declaration in CodegenUtil.getMemberDeclarationsToGenerate(element)) { + genSimpleMember(declaration) } if (state.classBuilderMode.generateBodies) { diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/PackageCodegenImpl.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/PackageCodegenImpl.java index afcde024067..805be14c9c7 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/PackageCodegenImpl.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/PackageCodegenImpl.java @@ -25,13 +25,20 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.backend.common.CodegenUtil; import org.jetbrains.kotlin.codegen.context.PackageContext; import org.jetbrains.kotlin.codegen.state.GenerationState; +import org.jetbrains.kotlin.descriptors.ClassDescriptor; import org.jetbrains.kotlin.descriptors.PackageFragmentDescriptor; import org.jetbrains.kotlin.diagnostics.DiagnosticUtils; import org.jetbrains.kotlin.fileClasses.JvmFileClassInfo; import org.jetbrains.kotlin.fileClasses.JvmFileClassUtil; import org.jetbrains.kotlin.name.FqName; import org.jetbrains.kotlin.progress.ProgressIndicatorAndCompilationCanceledStatus; -import org.jetbrains.kotlin.psi.*; +import org.jetbrains.kotlin.psi.KtClassOrObject; +import org.jetbrains.kotlin.psi.KtDeclaration; +import org.jetbrains.kotlin.psi.KtFile; +import org.jetbrains.kotlin.psi.KtScript; +import org.jetbrains.kotlin.psi.psiUtil.PsiUtilsKt; +import org.jetbrains.kotlin.resolve.BindingContext; +import org.jetbrains.kotlin.resolve.checkers.ExpectedActualDeclarationChecker; import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOriginKt; import org.jetbrains.kotlin.resolve.lazy.descriptors.PackageDescriptorUtilKt; import org.jetbrains.org.objectweb.asm.Type; @@ -91,15 +98,16 @@ public class PackageCodegenImpl implements PackageCodegen { Type fileClassType = AsmUtil.asmTypeByFqNameWithoutInnerClasses(fileClassInfo.getFileClassFqName()); PackageContext packagePartContext = state.getRootContext().intoPackagePart(packageFragment, fileClassType, file); - boolean generatePackagePart = false; - List classOrObjects = new ArrayList<>(); - for (KtDeclaration declaration : CodegenUtil.getDeclarationsToGenerate(file, state.getBindingContext())) { - if (isFilePartDeclaration(declaration)) { - generatePackagePart = true; - } - else if (declaration instanceof KtClassOrObject) { + 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); @@ -115,7 +123,9 @@ public class PackageCodegenImpl implements PackageCodegen { } generateClassesAndObjectsInFile(classOrObjects, packagePartContext); - if (!generatePackagePart || !state.getGenerateDeclaredClassFilter().shouldGeneratePackagePart(file)) return; + if (!state.getGenerateDeclaredClassFilter().shouldGeneratePackagePart(file)) return; + + if (CodegenUtil.getMemberDeclarationsToGenerate(file).isEmpty()) return; state.getFactory().getPackagePartRegistry().addPart(packageFragment.getFqName(), fileClassType.getInternalName(), null); @@ -124,10 +134,6 @@ public class PackageCodegenImpl implements PackageCodegen { new PackagePartCodegen(builder, file, fileClassType, packagePartContext, state).generate(); } - public static boolean isFilePartDeclaration(KtDeclaration declaration) { - return declaration instanceof KtProperty || declaration instanceof KtNamedFunction || declaration instanceof KtTypeAlias; - } - @Nullable private PackageFragmentDescriptor getOnlyPackageFragment(@NotNull FqName expectedPackageFqName) { SmartList fragments = new SmartList<>(); diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/PackagePartCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/PackagePartCodegen.java index 5fde51ab6d9..08f37f6472d 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/PackagePartCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/PackagePartCodegen.java @@ -19,13 +19,12 @@ package org.jetbrains.kotlin.codegen; import com.intellij.util.ArrayUtil; import kotlin.Pair; import kotlin.Unit; -import kotlin.collections.CollectionsKt; import org.jetbrains.annotations.NotNull; import org.jetbrains.kotlin.backend.common.CodegenUtil; import org.jetbrains.kotlin.codegen.context.FieldOwnerContext; import org.jetbrains.kotlin.codegen.serialization.JvmSerializerExtension; import org.jetbrains.kotlin.codegen.state.GenerationState; -import org.jetbrains.kotlin.descriptors.DeclarationDescriptor; +import org.jetbrains.kotlin.descriptors.MemberDescriptor; import org.jetbrains.kotlin.descriptors.annotations.Annotated; import org.jetbrains.kotlin.descriptors.annotations.AnnotatedImpl; import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor; @@ -33,7 +32,9 @@ import org.jetbrains.kotlin.descriptors.annotations.Annotations; import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader; import org.jetbrains.kotlin.metadata.ProtoBuf; import org.jetbrains.kotlin.name.FqName; -import org.jetbrains.kotlin.psi.*; +import org.jetbrains.kotlin.psi.KtAnnotationEntry; +import org.jetbrains.kotlin.psi.KtDeclaration; +import org.jetbrains.kotlin.psi.KtFile; import org.jetbrains.kotlin.resolve.BindingContext; import org.jetbrains.kotlin.resolve.jvm.JvmClassName; import org.jetbrains.kotlin.serialization.DescriptorSerializer; @@ -90,10 +91,8 @@ public class PackagePartCodegen extends MemberCodegen { @Override protected void generateBody() { - for (KtDeclaration declaration : CodegenUtil.getDeclarationsToGenerate(element, state.getBindingContext())) { - if (declaration instanceof KtNamedFunction || declaration instanceof KtProperty || declaration instanceof KtTypeAlias) { - genSimpleMember(declaration); - } + for (KtDeclaration declaration : CodegenUtil.getMemberDeclarationsToGenerate(element)) { + genSimpleMember(declaration); } if (state.getClassBuilderMode().generateBodies) { @@ -122,21 +121,7 @@ public class PackagePartCodegen extends MemberCodegen { @NotNull MemberCodegen codegen, @NotNull Type packagePartType ) { - BindingContext bindingContext = codegen.bindingContext; - List allDeclarations = CodegenUtil.getDeclarationsToGenerate(codegen.element, bindingContext); - List members = CollectionsKt.mapNotNull(allDeclarations, declaration -> { - if (declaration instanceof KtNamedFunction) { - return bindingContext.get(BindingContext.FUNCTION, declaration); - } - else if (declaration instanceof KtProperty) { - return bindingContext.get(BindingContext.VARIABLE, declaration); - } - else if (declaration instanceof KtTypeAlias) { - return bindingContext.get(BindingContext.TYPE_ALIAS, declaration); - } - - return null; - }); + List members = CodegenUtil.getMemberDescriptorsToGenerate(codegen.element, codegen.bindingContext); JvmSerializerExtension extension = new JvmSerializerExtension(codegen.v.getSerializationBindings(), codegen.state); DescriptorSerializer serializer = DescriptorSerializer.createTopLevel(extension); diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/SamWrapperCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/SamWrapperCodegen.java index 0b35bbe007a..64ba64392fb 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/SamWrapperCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/SamWrapperCodegen.java @@ -16,7 +16,6 @@ package org.jetbrains.kotlin.codegen; -import kotlin.collections.CollectionsKt; import kotlin.text.StringsKt; import org.jetbrains.annotations.NotNull; import org.jetbrains.kotlin.backend.common.CodegenUtil; @@ -181,10 +180,7 @@ public class SamWrapperCodegen { @NotNull KtFile containingFile, CallableMemberDescriptor contextDescriptor ) { - boolean hasPackagePartClass = CollectionsKt.any( - CodegenUtil.getDeclarationsToGenerate(containingFile, state.getBindingContext()), - PackageCodegenImpl::isFilePartDeclaration - ); + boolean hasPackagePartClass = !CodegenUtil.getMemberDeclarationsToGenerate(containingFile).isEmpty(); FqName filePartFqName = JvmFileClassUtil.getFileClassInfoNoResolve(containingFile).getFileClassFqName(); FqName outermostOwner; diff --git a/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/AbstractBlackBoxCodegenTest.java b/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/AbstractBlackBoxCodegenTest.java index a64131cd7ab..3d24ef46672 100644 --- a/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/AbstractBlackBoxCodegenTest.java +++ b/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/AbstractBlackBoxCodegenTest.java @@ -12,8 +12,7 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.TestsRuntimeError; import org.jetbrains.kotlin.backend.common.CodegenUtil; import org.jetbrains.kotlin.fileClasses.JvmFileClassUtil; -import org.jetbrains.kotlin.psi.*; -import org.jetbrains.kotlin.resolve.BindingContext; +import org.jetbrains.kotlin.psi.KtFile; import org.jetbrains.kotlin.test.InTextDirectivesUtils; import org.jetbrains.kotlin.test.TargetBackend; import org.jetbrains.kotlin.utils.ExceptionUtilsKt; @@ -29,7 +28,7 @@ import static org.jetbrains.kotlin.test.clientserver.TestProcessServerKt.getGene public abstract class AbstractBlackBoxCodegenTest extends CodegenTestCase { - public static final boolean IGNORE_EXPECTED_FAILURES = + private static final boolean IGNORE_EXPECTED_FAILURES = Boolean.getBoolean("kotlin.suppress.expected.test.failures"); @Override @@ -103,7 +102,7 @@ public abstract class AbstractBlackBoxCodegenTest extends CodegenTestCase { // If there are many files, the first 'box(): String' function will be executed. GeneratedClassLoader generatedClassLoader = generateAndCreateClassLoader(reportProblems); for (KtFile firstFile : myFiles.getPsiFiles()) { - String className = getFacadeFqName(firstFile, classFileFactory.getGenerationState().getBindingContext()); + String className = getFacadeFqName(firstFile); if (className == null) continue; Class aClass = getGeneratedClass(generatedClassLoader, className); try { @@ -127,13 +126,10 @@ public abstract class AbstractBlackBoxCodegenTest extends CodegenTestCase { } @Nullable - private static String getFacadeFqName(@NotNull KtFile firstFile, @NotNull BindingContext bindingContext) { - for (KtDeclaration declaration : CodegenUtil.getDeclarationsToGenerate(firstFile, bindingContext)) { - if (declaration instanceof KtProperty || declaration instanceof KtNamedFunction || declaration instanceof KtTypeAlias) { - return JvmFileClassUtil.getFileClassInfoNoResolve(firstFile).getFacadeClassFqName().asString(); - } - } - return null; + private static String getFacadeFqName(@NotNull KtFile file) { + return CodegenUtil.getMemberDeclarationsToGenerate(file).isEmpty() + ? null + : JvmFileClassUtil.getFileClassInfoNoResolve(file).getFacadeClassFqName().asString(); } protected TargetBackend getBackend() {