From 748a2d2e7ca952f064403e66512acdc3b16ac07c Mon Sep 17 00:00:00 2001 From: Stanislav Erokhin Date: Fri, 30 Jul 2021 14:27:49 +0200 Subject: [PATCH] [MPP] Performance optimization in expect/actual checker Previously, the checker executed for every declaration i.e. every declaration was considered as expect declaration. Because of that in some cases this checker could eat 6% of compilation time. After this commit only declarations marked with expect or actual are checked. To achieve that, logic, that do reporting about missing actual modifier was moved to the Actual part. Please note, that in cases where there is no expect/actual modifier at all other errors (like redeclaration and missing body on "actual" declaration) would be reported. Useful nodes: - In this checker reportOn is always the same as descriptor.sourceElement.ktElement. This is because the only case when it isn't true is PropertyAccessors and they are filtered - Annotation constructor descriptor isActual == true all the time - previously for weak incompatible members ACTUAL_MISSING was not reported - the logic here is super complicated and crazy, but I don't think that there is sense to refactor it in the old FE --- .../ExpectedActualDeclarationChecker.kt | 70 +++++++++++++------ ...ectDeclarationWithWeakIncompatibilities.kt | 4 +- .../headerClass/noImplKeywordOnMember.fir.kt | 4 +- .../headerClass/noImplKeywordOnMember.kt | 2 + .../headerClass/noImplKeywordOnMember.txt | 2 + .../output.txt | 7 +- 6 files changed, 60 insertions(+), 29 deletions(-) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExpectedActualDeclarationChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExpectedActualDeclarationChecker.kt index 325d470facd..62da140acdc 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExpectedActualDeclarationChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExpectedActualDeclarationChecker.kt @@ -18,14 +18,11 @@ package org.jetbrains.kotlin.resolve.checkers import com.intellij.openapi.vfs.VfsUtilCore import com.intellij.psi.PsiElement -import org.jetbrains.kotlin.analyzer.CombinedModuleInfo -import org.jetbrains.kotlin.analyzer.ModuleInfo import org.jetbrains.kotlin.config.AnalysisFlags import org.jetbrains.kotlin.config.LanguageFeature import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.diagnostics.Errors import org.jetbrains.kotlin.incremental.components.ExpectActualTracker -import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.hasActualModifier import org.jetbrains.kotlin.resolve.* @@ -38,6 +35,7 @@ import org.jetbrains.kotlin.resolve.multiplatform.ExpectActualCompatibility.* import org.jetbrains.kotlin.resolve.multiplatform.ExpectedActualResolver import org.jetbrains.kotlin.resolve.multiplatform.ModuleFilter import org.jetbrains.kotlin.resolve.multiplatform.OptionalAnnotationUtil +import org.jetbrains.kotlin.resolve.source.KotlinSourceElement import org.jetbrains.kotlin.resolve.source.PsiSourceFile import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.utils.addToStdlib.safeAs @@ -63,26 +61,38 @@ class ExpectedActualDeclarationChecker( if (declaration !is KtNamedDeclaration) return if (descriptor !is MemberDescriptor || DescriptorUtils.isEnumEntry(descriptor)) return - if (descriptor.isExpect) { - checkExpectedDeclarationHasProperActuals(declaration, descriptor, context.trace, context.expectActualTracker) - } else { - val checkActual = !context.languageVersionSettings.getFlag(AnalysisFlags.multiPlatformDoNotCheckActual) + val checkActualModifier = !context.languageVersionSettings.getFlag(AnalysisFlags.multiPlatformDoNotCheckActual) + if (descriptor.isExpect) { + checkExpectedDeclarationHasProperActuals( + declaration, descriptor, context.trace, + checkActualModifier, context.expectActualTracker + ) + } else if (descriptor.isActualOrSomeContainerIsActual()) { val allImplementedModules = moduleStructureOracle.findAllDependsOnPaths(descriptor.module).flatMap { it.nodes }.toHashSet() checkActualDeclarationHasExpected( declaration, descriptor, + checkActualModifier, context.trace, - checkActual, moduleVisibilityFilter = { it in allImplementedModules } ) } } + private fun MemberDescriptor.isActualOrSomeContainerIsActual(): Boolean { + var declaration: MemberDescriptor = this + while (true) { + if (declaration.isActual) return true + declaration = declaration.containingDeclaration as? MemberDescriptor ?: return false + } + } + private fun checkExpectedDeclarationHasProperActuals( reportOn: KtNamedDeclaration, descriptor: MemberDescriptor, trace: BindingTrace, + checkActualModifier: Boolean, expectActualTracker: ExpectActualTracker ) { val allActualizationPaths = moduleStructureOracle.findAllReversedDependsOnPaths(descriptor.module) @@ -95,6 +105,7 @@ class ExpectedActualDeclarationChecker( descriptor, trace, leafModule, + checkActualModifier, expectActualTracker, moduleVisibilityFilter = { it in modulesVisibleFromLeaf } ) @@ -162,6 +173,7 @@ class ExpectedActualDeclarationChecker( descriptor: MemberDescriptor, trace: BindingTrace, module: ModuleDescriptor, + checkActualModifier: Boolean, expectActualTracker: ExpectActualTracker, moduleVisibilityFilter: ModuleFilter ) { @@ -184,13 +196,28 @@ class ExpectedActualDeclarationChecker( return } - // Here we have exactly one compatible actual and/or some weakly incompatible. In either case, we don't report anything on expect + // Here we have exactly one compatible actual and/or some weakly incompatible. In either case, we don't report anything on expect... val actualMembers = compatibility.asSequence() .filter { it.key.isCompatibleOrWeakCompatible() }.flatMap { it.value.asSequence() } + // ...except diagnostics regarding missing actual keyword, because in that case we won't start looking for the actual at all + if (checkActualModifier) { + actualMembers.forEach { reportMissingActualModifier(it, reportOn = null, trace) } + } + expectActualTracker.reportExpectActual(expected = descriptor, actualMembers = actualMembers) } + private fun reportMissingActualModifier(actual: MemberDescriptor, reportOn: KtNamedDeclaration?, trace: BindingTrace) { + if (actual.isActual) return + @Suppress("NAME_SHADOWING") + val reportOn = reportOn ?: actual.source.safeAs()?.psi.safeAs() ?: return + + if (requireActualModifier(actual)) { + trace.report(Errors.ACTUAL_MISSING.on(reportOn)) + } + } + private fun MemberDescriptor.hasNoActualWithDiagnostic( compatibility: Map, List> ): Boolean { @@ -219,8 +246,8 @@ class ExpectedActualDeclarationChecker( private fun checkActualDeclarationHasExpected( reportOn: KtNamedDeclaration, descriptor: MemberDescriptor, + checkActualModifier: Boolean, trace: BindingTrace, - checkActual: Boolean, moduleVisibilityFilter: ModuleFilter ) { val compatibility = ExpectedActualResolver.findExpectedForActual(descriptor, descriptor.module, moduleVisibilityFilter) @@ -228,19 +255,20 @@ class ExpectedActualDeclarationChecker( checkAmbiguousExpects(compatibility, trace, reportOn, descriptor) - val hasActualModifier = descriptor.isActual && reportOn.hasActualModifier() - if (!hasActualModifier) { - if (compatibility.allStrongIncompatibilities()) return - - if (Compatible in compatibility) { - if (checkActual && requireActualModifier(descriptor)) { - trace.report(Errors.ACTUAL_MISSING.on(reportOn)) - } - - return - } + // For top-level declaration missing actual error reported in Actual checker + if (checkActualModifier + && descriptor.containingDeclaration !is PackageFragmentDescriptor + && compatibility.any { it.key.isCompatibleOrWeakCompatible() } + ) { + reportMissingActualModifier(descriptor, reportOn, trace) } + // Usually, reportOn.hasActualModifier() and descriptor.isActual are the same. + // The only one case where it isn't true is constructor of annotation class. In that case descriptor.isActual is true. + // See the FunctionDescriptorResolver.createConstructorDescriptor + // But in that case compatibility.allStrongIncompatibilities() == true means that in the expect class there is no constructor + if (!reportOn.hasActualModifier() && compatibility.allStrongIncompatibilities()) return + // 'firstOrNull' is needed because in diagnostic tests, common sources appear twice, so the same class is duplicated // TODO: replace with 'singleOrNull' as soon as multi-module diagnostic tests are refactored val singleIncompatibility = compatibility.keys.firstOrNull() diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerClass/expectDeclarationWithWeakIncompatibilities.kt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/expectDeclarationWithWeakIncompatibilities.kt index f3c614ceb61..a5a5475b2f1 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/headerClass/expectDeclarationWithWeakIncompatibilities.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/headerClass/expectDeclarationWithWeakIncompatibilities.kt @@ -16,7 +16,7 @@ expect open class interface Foo1 +interface Foo1 actual interface Foo2 actual var s: String = "value" @@ -25,4 +25,4 @@ fun foo2(): Int = 0 actual class Foo3 -class Foo3 +class Foo3 diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerClass/noImplKeywordOnMember.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/noImplKeywordOnMember.fir.kt index 00ae98397d9..f06b136f9c8 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/headerClass/noImplKeywordOnMember.fir.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/headerClass/noImplKeywordOnMember.fir.kt @@ -4,6 +4,7 @@ expect class Foo { fun bar(): String + fun bas(f: Int) } // MODULE: m2-jvm()()(m1-common) @@ -11,4 +12,5 @@ expect class Foo { actual class Foo { fun bar(): String = "bar" -} + fun bas(g: Int) {} +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerClass/noImplKeywordOnMember.kt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/noImplKeywordOnMember.kt index 663d74465db..55fc6cb430b 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/headerClass/noImplKeywordOnMember.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/headerClass/noImplKeywordOnMember.kt @@ -4,6 +4,7 @@ expect class Foo { fun bar(): String + fun bas(f: Int) } // MODULE: m2-jvm()()(m1-common) @@ -11,4 +12,5 @@ expect class Foo { actual class Foo { fun bar(): String = "bar" + fun bas(g: Int) {} } diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerClass/noImplKeywordOnMember.txt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/noImplKeywordOnMember.txt index 2866e0f02ab..66d71d19ae9 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/headerClass/noImplKeywordOnMember.txt +++ b/compiler/testData/diagnostics/tests/multiplatform/headerClass/noImplKeywordOnMember.txt @@ -3,6 +3,7 @@ package public final expect class Foo { public final expect fun bar(): kotlin.String + public final expect fun bas(/*0*/ f: kotlin.Int): kotlin.Unit 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 @@ -14,6 +15,7 @@ package public final actual class Foo { public constructor Foo() public final fun bar(): kotlin.String + public final fun bas(/*0*/ g: kotlin.Int): kotlin.Unit 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/multiplatform/weakIncompatibilityWithoutActualModifier/output.txt b/compiler/testData/multiplatform/weakIncompatibilityWithoutActualModifier/output.txt index a7c0d10151f..49a77a190b8 100644 --- a/compiler/testData/multiplatform/weakIncompatibilityWithoutActualModifier/output.txt +++ b/compiler/testData/multiplatform/weakIncompatibilityWithoutActualModifier/output.txt @@ -5,9 +5,6 @@ Output: -- JVM -- Exit code: COMPILATION_ERROR Output: -compiler/testData/multiplatform/weakIncompatibilityWithoutActualModifier/jvm.kt:1:1: error: interface 'Foo' has no corresponding expected declaration -The following declaration is incompatible because class kinds are different (class, interface, object, enum, annotation): - public final expect class Foo - +compiler/testData/multiplatform/weakIncompatibilityWithoutActualModifier/jvm.kt:1:11: error: declaration must be marked with 'actual' interface Foo -^ + ^