From 6cc6b9efcb8ef0e26f4a7cbac31655f4eeeeac42 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Wed, 1 Apr 2020 17:56:13 +0300 Subject: [PATCH] FIR: Fix overload resolution for some invoke cases Namely, it's about the cases when there are multiple variable candidates with the same priority and all of them should be collected and compared --- .../resolve/callResolution/invokeAmbiguity.kt | 17 +++++++ .../callResolution/invokeAmbiguity.txt | 47 +++++++++++++++++++ .../problems/invokePriority.kt | 4 +- .../problems/invokePriority.txt | 2 +- .../fir/FirDiagnosticsTestGenerated.java | 5 ++ ...DiagnosticsWithLightTreeTestGenerated.java | 5 ++ .../jetbrains/kotlin/fir/FirCallResolver.kt | 1 + .../kotlin/fir/resolve/calls/Candidate.kt | 2 + .../fir/resolve/calls/CandidateCollector.kt | 3 ++ .../calls/ConeOverloadConflictResolver.kt | 22 ++++++++- .../resolve/calls/FirTowerResolverSession.kt | 19 +++++--- .../kotlin/fir/resolve/calls/TowerGroup.kt | 39 ++++++++++++++- .../fir/resolve/calls/TowerResolveManager.kt | 18 +++---- 13 files changed, 163 insertions(+), 21 deletions(-) create mode 100644 compiler/fir/analysis-tests/testData/resolve/callResolution/invokeAmbiguity.kt create mode 100644 compiler/fir/analysis-tests/testData/resolve/callResolution/invokeAmbiguity.txt diff --git a/compiler/fir/analysis-tests/testData/resolve/callResolution/invokeAmbiguity.kt b/compiler/fir/analysis-tests/testData/resolve/callResolution/invokeAmbiguity.kt new file mode 100644 index 00000000000..d10d972fe04 --- /dev/null +++ b/compiler/fir/analysis-tests/testData/resolve/callResolution/invokeAmbiguity.kt @@ -0,0 +1,17 @@ +open class A { + operator fun invoke(f: () -> Unit): Int = 1 +} + +class B { + operator fun invoke(f: () -> Unit): CharSequence = "" +} + +open class C +val C.attr: A get() = TODO() + +open class D: C() +val D.attr: B get() = TODO() + +fun box(d: D) { + (d.attr {}).length +} diff --git a/compiler/fir/analysis-tests/testData/resolve/callResolution/invokeAmbiguity.txt b/compiler/fir/analysis-tests/testData/resolve/callResolution/invokeAmbiguity.txt new file mode 100644 index 00000000000..9d30b415b89 --- /dev/null +++ b/compiler/fir/analysis-tests/testData/resolve/callResolution/invokeAmbiguity.txt @@ -0,0 +1,47 @@ +FILE: invokeAmbiguity.kt + public open class A : R|kotlin/Any| { + public constructor(): R|A| { + super() + } + + public final operator fun invoke(f: R|() -> kotlin/Unit|): R|kotlin/Int| { + ^invoke Int(1) + } + + } + public final class B : R|kotlin/Any| { + public constructor(): R|B| { + super() + } + + public final operator fun invoke(f: R|() -> kotlin/Unit|): R|kotlin/CharSequence| { + ^invoke String() + } + + } + public open class C : R|kotlin/Any| { + public constructor(): R|C| { + super() + } + + } + public final val R|C|.attr: R|A| + public get(): R|A| { + ^ R|kotlin/TODO|() + } + public open class D : R|C| { + public constructor(): R|D| { + super() + } + + } + public final val R|D|.attr: R|B| + public get(): R|B| { + ^ R|kotlin/TODO|() + } + public final fun box(d: R|D|): R|kotlin/Unit| { + R|/d|.R|/attr|.R|/B.invoke|( = attr@fun (): R|kotlin/Unit| { + Unit + } + ).R|kotlin/CharSequence.length| + } diff --git a/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/invokePriority.kt b/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/invokePriority.kt index 47cd69fc4af..74b52ae311b 100644 --- a/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/invokePriority.kt +++ b/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/invokePriority.kt @@ -29,6 +29,6 @@ class E { } fun main() { - E.f() // Resolves to (2) in old FE (to (1) in FIR with object implicit invoke support) + E.f() // Resolves to (2) in old FE (ambiguity in FIR) E.f.invoke() // Resolves to (1) -} \ No newline at end of file +} diff --git a/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/invokePriority.txt b/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/invokePriority.txt index 5ed5662a409..f2be2a821d0 100644 --- a/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/invokePriority.txt +++ b/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/invokePriority.txt @@ -74,6 +74,6 @@ FILE: invokePriority.kt } public final fun main(): R|kotlin/Unit| { - Q|E|.R|/E.Companion.f|.R|/E.f.invoke|() + Q|E|.#() Q|E.f|.R|/E.f.invoke|() } diff --git a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java index e07e529c59f..87ad42ed7f4 100644 --- a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java @@ -584,6 +584,11 @@ public class FirDiagnosticsTestGenerated extends AbstractFirDiagnosticsTest { runTest("compiler/fir/analysis-tests/testData/resolve/callResolution/companionInvoke.kt"); } + @TestMetadata("invokeAmbiguity.kt") + public void testInvokeAmbiguity() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/callResolution/invokeAmbiguity.kt"); + } + @TestMetadata("objectInvoke.kt") public void testObjectInvoke() throws Exception { runTest("compiler/fir/analysis-tests/testData/resolve/callResolution/objectInvoke.kt"); diff --git a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsWithLightTreeTestGenerated.java index 6422f2fc31b..09818c5fafb 100644 --- a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsWithLightTreeTestGenerated.java @@ -584,6 +584,11 @@ public class FirDiagnosticsWithLightTreeTestGenerated extends AbstractFirDiagnos runTest("compiler/fir/analysis-tests/testData/resolve/callResolution/companionInvoke.kt"); } + @TestMetadata("invokeAmbiguity.kt") + public void testInvokeAmbiguity() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/callResolution/invokeAmbiguity.kt"); + } + @TestMetadata("objectInvoke.kt") public void testObjectInvoke() throws Exception { runTest("compiler/fir/analysis-tests/testData/resolve/callResolution/objectInvoke.kt"); diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/FirCallResolver.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/FirCallResolver.kt index 6db186bd33b..95f213a240d 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/FirCallResolver.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/FirCallResolver.kt @@ -354,6 +354,7 @@ class FirCallResolver( session, file, transformer.components.implicitReceiverStack, + candidateForCommonInvokeReceiver = null, // Additional things for callable reference resolve expectedType, outerConstraintSystemBuilder, diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/Candidate.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/Candidate.kt index 5de58b49485..71b5eac12de 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/Candidate.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/Candidate.kt @@ -43,6 +43,8 @@ data class CallInfo( val containingFile: FirFile, val implicitReceiverStack: ImplicitReceiverStack, + val candidateForCommonInvokeReceiver: Candidate? = null, + // Four properties for callable references only val expectedType: ConeKotlinType? = null, val outerCSBuilder: ConstraintSystemBuilder? = null, 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 0986ea7d8dd..f08e74f96e4 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 @@ -44,6 +44,9 @@ open class CandidateCollector( fun bestCandidates() = candidates + fun shouldStopAtTheLevel(group: TowerGroup) = + isSuccess() && bestGroup < group + fun isSuccess(): Boolean { return currentApplicability >= CandidateApplicability.SYNTHETIC_RESOLVED } diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ConeOverloadConflictResolver.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ConeOverloadConflictResolver.kt index eaa1313c124..ecb89da25f8 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ConeOverloadConflictResolver.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ConeOverloadConflictResolver.kt @@ -35,11 +35,31 @@ class ConeOverloadConflictResolver( discriminateGenerics: Boolean, discriminateAbstracts: Boolean ): Set { + if (candidates.size == 1) return candidates + val fixedCandidates = + if (candidates.first().callInfo.candidateForCommonInvokeReceiver != null) + chooseCandidatesWithMostSpecificInvokeReceiver(candidates) + else + candidates + return chooseMaximallySpecificCandidates( - candidates, discriminateGenerics, discriminateAbstracts, discriminateSAMs = true + fixedCandidates, discriminateGenerics, discriminateAbstracts, discriminateSAMs = true ) } + private fun chooseCandidatesWithMostSpecificInvokeReceiver(candidates: Set): Set { + val propertyReceiverCandidates = candidates.mapTo(mutableSetOf()) { + it.callInfo.candidateForCommonInvokeReceiver + ?: error("If one candidate within a group is property+invoke, other should be the same, but $it found") + } + + val bestInvokeReceiver = + chooseMaximallySpecificCandidates(propertyReceiverCandidates, discriminateGenerics = false) + .singleOrNull() ?: return candidates + + return candidates.filterTo(mutableSetOf()) { it.callInfo.candidateForCommonInvokeReceiver == bestInvokeReceiver } + } + private fun chooseMaximallySpecificCandidates( candidates: Set, discriminateGenerics: Boolean, diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/FirTowerResolverSession.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/FirTowerResolverSession.kt index 2d87503c835..6771f22ffa0 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/FirTowerResolverSession.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/FirTowerResolverSession.kt @@ -101,8 +101,6 @@ class FirTowerResolverSession internal constructor( enqueueResolverTasksForInvokeReceiverCandidates( invokeResolveMode, callInfo ) - }.also { - manager.stopIfSuccess() } } @@ -521,7 +519,11 @@ class FirTowerResolverSession internal constructor( group: TowerGroup, explicitReceiverKind: ExplicitReceiverKind, candidateFactory: CandidateFactory - ) = processLevel(towerLevel, callInfo, group, explicitReceiverKind, InvokeResolveMode.IMPLICIT_CALL_ON_GIVEN_RECEIVER, candidateFactory) + ) = processLevel( + towerLevel, callInfo, + group.InvokeResolvePriority(InvokeResolvePriority.COMMON_INVOKE), + explicitReceiverKind, InvokeResolveMode.IMPLICIT_CALL_ON_GIVEN_RECEIVER, candidateFactory + ) // Here we already know extension receiver for invoke, and it's stated in info as first argument private suspend fun runResolverForBuiltinInvokeExtensionWithExplicitArgument( @@ -531,7 +533,7 @@ class FirTowerResolverSession internal constructor( ) { processLevel( invokeReceiverValue.toMemberScopeTowerLevel(), - info, TowerGroup.Member, + info, TowerGroup.Member.InvokeResolvePriority(InvokeResolvePriority.INVOKE_EXTENSION), ExplicitReceiverKind.DISPATCH_RECEIVER, InvokeResolveMode.IMPLICIT_CALL_ON_GIVEN_RECEIVER, invokeOnGivenReceiverCandidateFactory @@ -545,13 +547,13 @@ class FirTowerResolverSession internal constructor( invokeOnGivenReceiverCandidateFactory: CandidateFactory ) { for ((implicitReceiverValue, depth) in implicitReceiversUsableAsValues) { - val parentGroup = TowerGroup.Implicit(depth) + val towerGroup = TowerGroup.Implicit(depth).InvokeExtension.InvokeResolvePriority(InvokeResolvePriority.INVOKE_EXTENSION) processLevel( invokeReceiverValue.toMemberScopeTowerLevel( extensionReceiver = implicitReceiverValue, implicitExtensionInvokeMode = true ), - info, parentGroup.InvokeExtension, + info, towerGroup, ExplicitReceiverKind.DISPATCH_RECEIVER, InvokeResolveMode.IMPLICIT_CALL_ON_GIVEN_RECEIVER, invokeOnGivenReceiverCandidateFactory @@ -587,7 +589,10 @@ class FirTowerResolverSession internal constructor( ) val invokeFunctionInfo = - info.copy(explicitReceiver = invokeReceiverExpression, name = OperatorNameConventions.INVOKE).let { + info.copy( + explicitReceiver = invokeReceiverExpression, name = OperatorNameConventions.INVOKE, + candidateForCommonInvokeReceiver = invokeReceiverCandidate.takeUnless { invokeBuiltinExtensionMode } + ).let { when { invokeBuiltinExtensionMode -> it.withReceiverAsArgument(info.explicitReceiver!!) else -> it diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/TowerGroup.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/TowerGroup.kt index 8c363dd1933..7a7770cf6c1 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/TowerGroup.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/TowerGroup.kt @@ -43,7 +43,11 @@ sealed class TowerGroupKind(private val index: Int) : Comparable } @Suppress("FunctionName", "unused", "PropertyName") -class TowerGroup private constructor(private val kinds: Array) : Comparable { +class TowerGroup +private constructor( + private val kinds: Array, + private val invokeResolvePriority: InvokeResolvePriority = InvokeResolvePriority.NONE +) : Comparable { companion object { private fun kindOf(kind: TowerGroupKind): TowerGroup = TowerGroup(arrayOf(kind)) @@ -84,6 +88,14 @@ class TowerGroup private constructor(private val kinds: Array) : fun Static(depth: Int) = kindOf(TowerGroupKind.Static(depth)) + // Treating `a.foo()` common calls as more prioritized than `a.foo.invoke()` + // It's not the same as TowerGroupKind because it's not about tower levels, but rather a different dimension semantically. + // It could be implemented via another TowerGroupKind, but it's not clear what priority should be assigned to the new TowerGroupKind + fun InvokeResolvePriority(invokeResolvePriority: InvokeResolvePriority): TowerGroup { + if (invokeResolvePriority == InvokeResolvePriority.NONE) return this + return TowerGroup(kinds, invokeResolvePriority) + } + override fun compareTo(other: TowerGroup): Int { var index = 0 while (index < kinds.size) { @@ -95,6 +107,29 @@ class TowerGroup private constructor(private val kinds: Array) : index++ } if (index < other.kinds.size) return -1 - return 0 + + return invokeResolvePriority.compareTo(other.invokeResolvePriority) + } + + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (javaClass != other?.javaClass) return false + + other as TowerGroup + + if (!kinds.contentEquals(other.kinds)) return false + if (invokeResolvePriority != other.invokeResolvePriority) return false + + return true + } + + override fun hashCode(): Int { + var result = kinds.contentHashCode() + result = 31 * result + invokeResolvePriority.hashCode() + return result } } + +enum class InvokeResolvePriority { + NONE, COMMON_INVOKE, INVOKE_EXTENSION; +} diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/TowerResolveManager.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/TowerResolveManager.kt index 8bea5a0c1ec..918ceb3b97b 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/TowerResolveManager.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/TowerResolveManager.kt @@ -8,9 +8,9 @@ package org.jetbrains.kotlin.fir.resolve.calls import java.util.* import kotlin.coroutines.* -class TowerResolveManager private constructor(private val isSuccess: () -> Boolean) { +class TowerResolveManager private constructor(private val shouldStopAtTheLevel: (TowerGroup) -> Boolean) { - constructor(collector: CandidateCollector) : this({ collector.isSuccess() }) + constructor(collector: CandidateCollector) : this(collector::shouldStopAtTheLevel) private val queue = PriorityQueue() @@ -21,6 +21,9 @@ class TowerResolveManager private constructor(private val isSuccess: () -> Boole private suspend fun suspendResolverTask(group: TowerGroup) = suspendCoroutine { queue += SuspendedResolverTask(it, group) } suspend fun requestGroup(requested: TowerGroup) { + if (shouldStopAtTheLevel(requested)) { + stopResolverTask() + } val peeked = queue.peek() // Task ordering should be FIFO @@ -32,10 +35,6 @@ class TowerResolveManager private constructor(private val isSuccess: () -> Boole private suspend fun stopResolverTask(): Nothing = suspendCoroutine { } - suspend fun stopIfSuccess() { - if (isSuccess()) stopResolverTask() - } - private data class SuspendedResolverTask( val continuation: Continuation, val group: TowerGroup @@ -62,8 +61,11 @@ class TowerResolveManager private constructor(private val isSuccess: () -> Boole fun runTasks() { while (queue.isNotEmpty()) { - queue.poll().continuation.resume(Unit) - if (isSuccess()) return + val current = queue.poll() + if (shouldStopAtTheLevel(current.group)) { + return + } + current.continuation.resume(Unit) } } }