K2: Avoid false-positive overload resolution ambiguity with smart casts

The idea is that when we have successful candidates both from smart cast
and original type, we should discriminate in the favor of former ones.

While this problem (see kt55722.kt) existed before this branch is merged,
initially it was recognized on FP Ultimate when we stopped assuming
captured types from the same projections as equal (see kt55722Initial.kt).

^KT-55722 Fixed
^KT-55024 Fixed
^KT-56283 Related
^KT-56310 Related
This commit is contained in:
Denis.Zharkov
2023-01-02 12:00:14 +01:00
committed by Space Team
parent 7b6c6fceb6
commit b6b132a9a3
27 changed files with 1034 additions and 35 deletions
@@ -39,7 +39,9 @@ class Candidate(
private val baseSystem: ConstraintStorage,
override val callInfo: CallInfo,
val originScope: FirScope?,
val isFromCompanionObjectTypeScope: Boolean = false
val isFromCompanionObjectTypeScope: Boolean = false,
// It's only true if we're in the member scope of smart cast receiver and this particular candidate came from original type
val isFromOriginalTypeInPresenceOfSmartCast: Boolean = false,
) : AbstractCandidate() {
var systemInitialized: Boolean = false
@@ -54,7 +54,8 @@ class CandidateFactory private constructor(
scope: FirScope?,
dispatchReceiverValue: ReceiverValue? = null,
givenExtensionReceiverOptions: List<ReceiverValue> = emptyList(),
objectsByName: Boolean = false
objectsByName: Boolean = false,
isFromOriginalTypeInPresenceOfSmartCast: Boolean = false,
): Candidate {
@Suppress("NAME_SHADOWING")
val symbol = symbol.unwrapIntegerOperatorSymbolIfNeeded(callInfo)
@@ -74,7 +75,8 @@ class CandidateFactory private constructor(
ExplicitReceiverKind.DISPATCH_RECEIVER -> dispatchReceiverValue.isCandidateFromCompanionObjectTypeScope()
// The following cases are not applicable for companion objects.
ExplicitReceiverKind.NO_EXPLICIT_RECEIVER, ExplicitReceiverKind.BOTH_RECEIVERS -> false
}
},
isFromOriginalTypeInPresenceOfSmartCast,
)
// The counterpart in FE 1.0 checks if the given descriptor is VariableDescriptor yet not PropertyDescriptor.
@@ -0,0 +1,76 @@
/*
* Copyright 2010-2022 JetBrains s.r.o. and Kotlin Programming Language contributors.
* Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
*/
package org.jetbrains.kotlin.fir.resolve.calls
import org.jetbrains.kotlin.utils.sure
/**
* In case of MemberScopeTowerLevel with smart cast dispatch receiver, we may create candidates both from smart cast type and from
* the member scope of original expression's type (without smart cast).
*
* It might be necessary because the ones from smart cast might be invisible (e.g., because they are protected in other class).
*
* open class A {
* open protected fun foo(a: Derived) {}
* fun f(a: A, d: Derived) {
* when (a) {
* is B -> {
* a.foo(d) // should be resolved to A::foo, not the public B::foo
* }
* }
* }
* }
* class B : A() {
* override fun foo(a: Derived) {}
* public fun foo(a: Base) {}
* }
*
* If we would just resolve `a.foo(d)` if `a` had a type B, then we would choose a public B::foo, because the other one `foo` is protected in B,
* so we can't call it outside the B subclasses.
*
* But that resolution result would be less precise result that the one before smart-cast applied (A::foo has more specific parameters),
* so at MemberScopeTowerLevel we create candidates both from A's and B's scopes on the same level.
*
* But in case when there would be successful candidates from both types, we discriminate ones from original type, thus sticking to the candidates
* from smart cast type.
*
* See more details at KT-51460, KT-55722, KT-56310 and relevant tests
* - testData/diagnostics/tests/visibility/moreSpecificProtectedSimple.kt
* - testData/diagnostics/tests/smartCasts/kt51460.kt
*/
object FilteringOutOriginalInPresenceOfSmartCastConeCallConflictResolver : ConeCallConflictResolver() {
override fun chooseMaximallySpecificCandidates(
candidates: Set<Candidate>,
discriminateGenerics: Boolean,
discriminateAbstracts: Boolean
): Set<Candidate> {
val (originalIfSmartCastPresent, other) = candidates.partition { it.isFromOriginalTypeInPresenceOfSmartCast }
// If we have both successful candidates from smart cast and original, use the former one as they might have more correct return type
if (originalIfSmartCastPresent.isNotEmpty() && other.isNotEmpty()) return other.toSet().discriminateByInvokeVariablePriority()
return candidates.discriminateByInvokeVariablePriority()
}
// See the relevant test at testData/diagnostics/tests/resolve/invoke/kt9517.kt
private fun Set<Candidate>.discriminateByInvokeVariablePriority(): Set<Candidate> {
if (size <= 1) return this
// Resulting successful candidates should always belong to the same tower group.
// Thus, if one of them is not variable + invoke, it should be applied to others, too.
if (first().callInfo.candidateForCommonInvokeReceiver == null) return this
val (originalIfSmartCastPresent, other) = partition {
it.callInfo.candidateForCommonInvokeReceiver.sure {
"If one candidate within a group is variable+invoke, other should be the same, but $it found"
}.isFromOriginalTypeInPresenceOfSmartCast
}
if (originalIfSmartCastPresent.isNotEmpty() && other.isNotEmpty()) return other.toSet()
return this
}
}
@@ -76,7 +76,8 @@ private class TowerScopeLevelProcessor(
dispatchReceiverValue: ReceiverValue?,
givenExtensionReceiverOptions: List<ReceiverValue>,
scope: FirScope,
objectsByName: Boolean
objectsByName: Boolean,
isFromOriginalTypeInPresenceOfSmartCast: Boolean
) {
resultCollector.consumeCandidate(
group, candidateFactory.createCandidate(
@@ -86,7 +87,8 @@ private class TowerScopeLevelProcessor(
scope,
dispatchReceiverValue,
givenExtensionReceiverOptions,
objectsByName
objectsByName,
isFromOriginalTypeInPresenceOfSmartCast
), candidateFactory.context
)
}
@@ -9,9 +9,6 @@ import org.jetbrains.kotlin.KtFakeSourceElementKind
import org.jetbrains.kotlin.KtSourceElement
import org.jetbrains.kotlin.fakeElement
import org.jetbrains.kotlin.fir.*
import org.jetbrains.kotlin.fir.declarations.ContextReceiverGroup
import org.jetbrains.kotlin.fir.declarations.FirConstructor
import org.jetbrains.kotlin.fir.declarations.getAnnotationByClassId
import org.jetbrains.kotlin.fir.declarations.*
import org.jetbrains.kotlin.fir.declarations.utils.isInner
import org.jetbrains.kotlin.fir.expressions.FirSmartCastExpression
@@ -59,7 +56,8 @@ abstract class TowerScopeLevel {
dispatchReceiverValue: ReceiverValue?,
givenExtensionReceiverOptions: List<ReceiverValue>,
scope: FirScope,
objectsByName: Boolean = false
objectsByName: Boolean = false,
isFromOriginalTypeInPresenceOfSmartCast: Boolean = false,
)
}
}
@@ -87,9 +85,7 @@ class MemberScopeTowerLevel(
val scope = dispatchReceiverValue.scope(session, scopeSession) ?: return ProcessResult.SCOPE_EMPTY
var (empty, candidates) = scope.collectCandidates(processScopeMembers)
val scopeWithoutSmartcast = (dispatchReceiverValue.receiverExpression as? FirSmartCastExpression)
?.takeIf { it.isStable }
?.originalExpression?.typeRef
val scopeWithoutSmartcast = getOriginalReceiverExpressionIfStableSmartCast()?.typeRef
?.coneType
?.scope(
session,
@@ -169,12 +165,16 @@ class MemberScopeTowerLevel(
candidatesFromSmartcast.forEach { put(it, true) }
}
val candidates = mutableListOf<MemberWithBaseScope<T>>()
// The code below is assumed to be for sake of optimization only.
// It helps to avoid creating candidates both for some member in the original type and for its override in the smart cast
// when they are both visible.
// But semantically it should be OK to declare `val candidates` as `candidatesMapping.keys.toList()`
val overridableGroups = session.overrideService.createOverridableGroups(
candidatesFromOriginalType + candidatesFromSmartcast,
FirIntersectionScopeOverrideChecker(session)
)
val candidates = mutableListOf<MemberWithBaseScope<T>>()
for (group in overridableGroups) {
val visibleCandidates = group.filter {
visibilityChecker.isVisible(it.member.fir, callInfo, dispatchReceiverValue)
@@ -183,7 +183,7 @@ class MemberScopeTowerLevel(
val visibleCandidatesFromSmartcast = visibleCandidates.filter { candidatesMapping.getValue(it) }
candidates += visibleCandidatesFromSmartcast.ifEmpty { group }
}
consumeCandidates(output, candidates)
consumeCandidates(output, candidates, candidatesMapping)
}
private fun <T : FirCallableSymbol<*>> FirTypeScope.collectCandidates(
@@ -206,20 +206,39 @@ class MemberScopeTowerLevel(
private fun <T : FirCallableSymbol<*>> consumeCandidates(
output: TowerScopeLevelProcessor<T>,
candidatesWithScope: List<MemberWithBaseScope<T>>
candidatesWithScope: List<MemberWithBaseScope<T>>,
// The map is not null only if there's a smart cast type on a dispatch receiver
// and candidates are present both in smart cast and original types.
// isFromSmartCast[candidate] == true iff exactly that member is present in smart cast type
isFromSmartCast: Map<MemberWithBaseScope<T>, Boolean>? = null
) {
for ((candidate, scope) in candidatesWithScope) {
for (candidateWithScope in candidatesWithScope) {
val (candidate, scope) = candidateWithScope
if (candidate.hasConsistentExtensionReceiver(givenExtensionReceiverOptions)) {
val isFromOriginalTypeInPresenceOfSmartCast = isFromSmartCast != null && !isFromSmartCast.getValue(candidateWithScope)
val dispatchReceiverToUse = when {
isFromOriginalTypeInPresenceOfSmartCast ->
getOriginalReceiverExpressionIfStableSmartCast()?.let(::ExpressionReceiverValue)
else -> dispatchReceiverValue
}
output.consumeCandidate(
candidate,
dispatchReceiverValue,
dispatchReceiverToUse,
givenExtensionReceiverOptions,
scope
scope,
isFromOriginalTypeInPresenceOfSmartCast = isFromOriginalTypeInPresenceOfSmartCast
)
}
}
}
private fun getOriginalReceiverExpressionIfStableSmartCast() =
(dispatchReceiverValue.receiverExpression as? FirSmartCastExpression)
?.takeIf { it.isStable }
?.originalExpression
override fun processFunctionsByName(
info: CallInfo,
processor: TowerScopeLevelProcessor<FirFunctionSymbol<*>>