From 30e2af7b66f80fa60f51e6e5656d841bae0f4a4f Mon Sep 17 00:00:00 2001 From: Dmitriy Novozhilov Date: Wed, 13 Mar 2024 12:10:20 +0200 Subject: [PATCH] [FIR] Threat some error candidate applicabilities with same priority There is a thing that `CandidateCollector` adds error candidate to the list of resulting candidates only if its applicability at least the same as current applicability of the collector Also there is a problem, that deserialized symbol provider in CLI compiler and stub-based symbol provider in AA may return the same declarations in different order. This provokes the difference in the resulting set of candidates between the two modes: ``` val x by unresolved ``` During the resolution of this code compiler tries to find function `getValue`, and there are 6 of them in the stdlib. From them we are interseted in specific three: 1. `fun Map.getValue(key: R|K|): R|V|` 2. `inline operator fun Map.getValue(thisRef: Any?, property: KProperty<*>): V1` 3. `inline operator fun MutableMap.getValue(thisRef: Any?, property: KProperty<*>): V1` - (1) is inapplicable with `INAPPLICABLE_ARGUMENTS_MAPPING_ERROR` - (2) and (3) are inapplicable with `INAPPLICABLE_WRONG_RECEIVER` - `INAPPLICABLE_ARGUMENTS_MAPPING_ERROR` is more specific applicability than `INAPPLICABLE_WRONG_RECEIVER` - CLI compiler always sees those functions in order 1 -> 2 -> 3 - AA providers sometimes returns them in order 2 -> 3 -> 1 So in CLI compilation candidates (2) and (3) are not added to the resulting set, as they are "less applicable" than (1), but in AA compilation they can be added to the set before (1), which causes sporadic change in FIR dump of `unsafeAssignmentExtra.kt` To workaround this problem it was decided to treat `INAPPLICABLE_ARGUMENTS_MAPPING_ERROR` and `INAPPLICABLE_WRONG_RECEIVER` applicabilities as "equally specific" ^KT-65218 Fixed --- .../kotlin/fir/resolve/calls/CandidateCollector.kt | 14 +++++++++++++- .../functionExpectedWhenSeveralInvokesExist.fir.kt | 2 +- .../builderInference/unsafeAssignmentExtra.fir.txt | 2 +- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/CandidateCollector.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/CandidateCollector.kt index a7525392058..c0c830eed1e 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/CandidateCollector.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/CandidateCollector.kt @@ -8,6 +8,8 @@ package org.jetbrains.kotlin.fir.resolve.calls import org.jetbrains.kotlin.fir.resolve.BodyResolveComponents import org.jetbrains.kotlin.fir.resolve.calls.tower.TowerGroup import org.jetbrains.kotlin.resolve.calls.tower.CandidateApplicability +import org.jetbrains.kotlin.resolve.calls.tower.CandidateApplicability.INAPPLICABLE_ARGUMENTS_MAPPING_ERROR +import org.jetbrains.kotlin.resolve.calls.tower.CandidateApplicability.INAPPLICABLE_WRONG_RECEIVER import org.jetbrains.kotlin.resolve.calls.tower.isSuccess import org.jetbrains.kotlin.resolve.calls.tower.shouldStopResolve @@ -51,7 +53,17 @@ open class CandidateCollector( bestGroup = group } - if (applicability == currentApplicability && group == bestGroup) { + /* + * Here we would like to consider error candidates with `INAPPLICABLE_WRONG_RECEIVER` and `INAPPLICABLE_ARGUMENTS_MAPPING_ERROR` kinda + * "same error level". Generally it's questionable which candidates we should keep and which of those applicabilities is more + * specific and we should consider it during work on improvement of error reporting. But this particular check is needed + * to fix the KT-65218, which provoked by different stdlib declarations order in CLI compilation mode and AA mode (see + * the issue for more details) + */ + if ( + (applicability == currentApplicability && group == bestGroup) || + (currentApplicability == INAPPLICABLE_ARGUMENTS_MAPPING_ERROR && applicability == INAPPLICABLE_WRONG_RECEIVER) + ) { candidates.add(candidate) } diff --git a/compiler/testData/diagnostics/tests/resolve/invoke/functionExpectedWhenSeveralInvokesExist.fir.kt b/compiler/testData/diagnostics/tests/resolve/invoke/functionExpectedWhenSeveralInvokesExist.fir.kt index a449ea3efd2..aa35269e61a 100644 --- a/compiler/testData/diagnostics/tests/resolve/invoke/functionExpectedWhenSeveralInvokesExist.fir.kt +++ b/compiler/testData/diagnostics/tests/resolve/invoke/functionExpectedWhenSeveralInvokesExist.fir.kt @@ -8,6 +8,6 @@ class SomeClass fun test(identifier: SomeClass, fn: String.() -> Unit) { identifier() identifier(123) - identifier(1, 2) + identifier(1, 2) 1.fn() } diff --git a/compiler/testData/diagnostics/testsWithStdLib/builderInference/unsafeAssignmentExtra.fir.txt b/compiler/testData/diagnostics/testsWithStdLib/builderInference/unsafeAssignmentExtra.fir.txt index 19c6f6a48d0..8bd66314281 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/builderInference/unsafeAssignmentExtra.fir.txt +++ b/compiler/testData/diagnostics/testsWithStdLib/builderInference/unsafeAssignmentExtra.fir.txt @@ -60,7 +60,7 @@ FILE: unsafeAssignmentExtra.fir.kt } } - lval x: by this@R|special/anonymous|.R|SubstitutionOverride| + lval x: by this@R|special/anonymous|.R|SubstitutionOverride| this@R|special/anonymous|.R|/change|( = change@fun R|Foo|.(): R|kotlin/Unit| { this@R|special/anonymous|.R|SubstitutionOverride| = Int(99) }