From 96e893223fa719f86424e92c5994584385e54bb5 Mon Sep 17 00:00:00 2001 From: "sebastian.sellmair" Date: Wed, 22 Dec 2021 17:22:14 +0100 Subject: [PATCH] Revert "[MPP] Improve performance of ExpectedActualDeclarationChecker.kt" This reverts commit 8880ea1c164ca60b54b8ae548d922110725c6828. --- .../ExpectedActualDeclarationChecker.kt | 108 ++++++++++-------- 1 file changed, 59 insertions(+), 49 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 19fb8f85c80..e261ed65b27 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExpectedActualDeclarationChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExpectedActualDeclarationChecker.kt @@ -36,6 +36,7 @@ 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 +import org.jetbrains.kotlin.utils.ifEmpty import java.io.File class ExpectedActualDeclarationChecker( @@ -92,34 +93,20 @@ class ExpectedActualDeclarationChecker( expectActualTracker: ExpectActualTracker ) { val allActualizationPaths = moduleStructureOracle.findAllReversedDependsOnPaths(descriptor.module) - val allLeafModules = allActualizationPaths.map { it.nodes.last() }.toSet() - - allLeafModules.forEach { leafModule -> - val actuals = ExpectedActualResolver.findActualForExpected(descriptor, leafModule) ?: return@forEach + val leafModuleToVisibleModules = allActualizationPaths.groupBy { it.nodes.last() } + .mapValues { it.value.flatMap { it.nodes }.toSet() } + for ((leafModule, modulesVisibleFromLeaf) in leafModuleToVisibleModules) { checkExpectedDeclarationHasAtLeastOneActual( - reportOn, descriptor, actuals, trace, leafModule, checkActualModifier, expectActualTracker - ) - - checkExpectedDeclarationHasAtMostOneActual( - reportOn, descriptor, actuals, allActualizationPaths, trace + reportOn, + descriptor, + trace, + leafModule, + checkActualModifier, + expectActualTracker, + moduleVisibilityFilter = { it in modulesVisibleFromLeaf } ) } - } - - private fun checkExpectedDeclarationHasAtMostOneActual( - reportOn: KtNamedDeclaration, - expectDescriptor: MemberDescriptor, - actuals: ActualsMap, - modulePaths: List, - trace: BindingTrace, - ) { - val atLeastWeaklyCompatibleActuals = actuals - .filterKeys { compatibility -> compatibility.isCompatibleOrWeakCompatible() } - .values.flatten() - - // Eagerly return here: We won't find a duplicate in any module path in this case - if (atLeastWeaklyCompatibleActuals.size <= 1) return /* Note that we have to check for 'duplicate actuals' separately, considering paths @@ -138,51 +125,76 @@ class ExpectedActualDeclarationChecker( If we merge behaviour (e.g. decide to report ERROR for first case too) for those two cases, we can drop separate logic for DUPLICATE_ACTUALS */ - val actualsByModulePath = modulePaths.associateWith { path -> - atLeastWeaklyCompatibleActuals.filter { it.module in path.nodes } + for (path in allActualizationPaths) { + val modulesOnThisPath = path.nodes.toSet() + checkExpectedDeclarationHasAtMostOneActual( + reportOn, descriptor, trace, path, moduleVisibilityFilter = { it in modulesOnThisPath } + ) } + } - actualsByModulePath.forEach { (_, actualsInPath) -> - if (actualsInPath.size > 1) { - trace.report(Errors.AMBIGUOUS_ACTUALS.on( - reportOn, - expectDescriptor, - actualsInPath - .map { it.module } - .sortedBy { it.name.asString() } - )) + private fun checkExpectedDeclarationHasAtMostOneActual( + reportOn: KtNamedDeclaration, + descriptor: MemberDescriptor, + trace: BindingTrace, + path: ModulePath, + moduleVisibilityFilter: ModuleFilter + ) { + val compatibility = path.nodes + .mapNotNull { ExpectedActualResolver.findActualForExpected(descriptor, it, moduleVisibilityFilter) } + .ifEmpty { return } + .fold(LinkedHashMap, List>()) { resultMap, partialMap -> + resultMap.apply { putAll(partialMap) } } + + // Several compatible actuals on one path: report AMBIGUIOUS_ACTUALS here + val atLeastWeaklyCompatibleActuals = compatibility + .filterKeys { it.isCompatibleOrWeakCompatible() } + .values + .flatten() + .distinct() + + if (atLeastWeaklyCompatibleActuals.size > 1) { + trace.report(Errors.AMBIGUOUS_ACTUALS.on( + reportOn, + descriptor, + atLeastWeaklyCompatibleActuals + .map { it.module } + .sortedBy { it.name.asString() } + )) } } private fun checkExpectedDeclarationHasAtLeastOneActual( reportOn: KtNamedDeclaration, - expectDescriptor: MemberDescriptor, - actuals: ActualsMap, + descriptor: MemberDescriptor, trace: BindingTrace, module: ModuleDescriptor, checkActualModifier: Boolean, - expectActualTracker: ExpectActualTracker + expectActualTracker: ExpectActualTracker, + moduleVisibilityFilter: ModuleFilter ) { // Only look for top level actual members; class members will be handled as a part of that expected class - if (expectDescriptor.containingDeclaration !is PackageFragmentDescriptor) return + if (descriptor.containingDeclaration !is PackageFragmentDescriptor) return + + val compatibility = ExpectedActualResolver.findActualForExpected(descriptor, module, moduleVisibilityFilter) ?: return // Only strong incompatibilities, but this is an OptionalExpectation -- don't report it - if (actuals.allStrongIncompatibilities() && OptionalAnnotationUtil.isOptionalAnnotationClass(expectDescriptor)) return + if (compatibility.allStrongIncompatibilities() && OptionalAnnotationUtil.isOptionalAnnotationClass(descriptor)) return // Only strong incompatibilities, or error won't be reported on actual: report NO_ACTUAL_FOR_EXPECT here - if (actuals.allStrongIncompatibilities() || - Compatible !in actuals && expectDescriptor.hasNoActualWithDiagnostic(actuals) + if (compatibility.allStrongIncompatibilities() || + Compatible !in compatibility && descriptor.hasNoActualWithDiagnostic(compatibility) ) { - assert(actuals.keys.all { it is Incompatible }) + assert(compatibility.keys.all { it is Incompatible }) @Suppress("UNCHECKED_CAST") - val incompatibility = actuals as Map, Collection> - trace.report(Errors.NO_ACTUAL_FOR_EXPECT.on(reportOn, expectDescriptor, module, incompatibility)) + val incompatibility = compatibility as Map, Collection> + trace.report(Errors.NO_ACTUAL_FOR_EXPECT.on(reportOn, descriptor, module, incompatibility)) return } // Here we have exactly one compatible actual and/or some weakly incompatible. In either case, we don't report anything on expect... - val actualMembers = actuals.asSequence() + 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 @@ -190,7 +202,7 @@ class ExpectedActualDeclarationChecker( actualMembers.forEach { reportMissingActualModifier(it, reportOn = null, trace) } } - expectActualTracker.reportExpectActual(expected = expectDescriptor, actualMembers = actualMembers) + expectActualTracker.reportExpectActual(expected = descriptor, actualMembers = actualMembers) } private fun reportMissingActualModifier(actual: MemberDescriptor, reportOn: KtNamedDeclaration?, trace: BindingTrace) { @@ -403,5 +415,3 @@ class ExpectedActualDeclarationChecker( this.keys.all { it is Incompatible && it.kind == IncompatibilityKind.STRONG } } } - -private typealias ActualsMap = Map, List>