azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

K2: Fix false-positive OVERLOAD_RESOLUTION_AMBIGUITY

Browse Source 
It's been introduced in the previous commit
("K2: Simplify handling mixed smartcast vs. original candidates")

Because previously, it was assumed wrongly that each next level of
ConeCallConflictResolver filter out the candidates that are 100% less
applicable/specific, but the main one (ConeOverloadConflictResolver)
either leaves the single candidate or the whole same set, thus at
FilteringOutOriginalInPresenceOfSmartCastConeCallConflictResolver
we've got 4 candidates only two of which we might filter out.
This commit is contained in:
Denis.Zharkov
2023-02-10 17:30:03 +01:00
committed by Space Team
parent 1e1d122dd3
commit b4b443034f
11 changed files with 169 additions and 88 deletions
Download Patch File Download Diff File Expand all files Collapse all files
analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java
Generated
+12
View File
@@ -23422,6 +23422,18 @@ public class DiagnosisCompilerTestFE10TestdataTestGenerated extends AbstractDiag
runTest("compiler/testData/diagnostics/tests/overload/OverloadVarAndFunInClass.kt");
}
@Test
@TestMetadata("overloadsFromCurrentAndSuperClass.kt")
public void testOverloadsFromCurrentAndSuperClass() throws Exception {
runTest("compiler/testData/diagnostics/tests/overload/overloadsFromCurrentAndSuperClass.kt");
}
@Test
@TestMetadata("overloadsFromCurrentAndSuperClassWithReturnType.kt")
public void testOverloadsFromCurrentAndSuperClassWithReturnType() throws Exception {
runTest("compiler/testData/diagnostics/tests/overload/overloadsFromCurrentAndSuperClassWithReturnType.kt");
}
@Test
@TestMetadata("SyntheticAndNotSynthetic.kt")
public void testSyntheticAndNotSynthetic() throws Exception {
compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java
Generated
+12
View File
@@ -23428,6 +23428,18 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirDiagnosti
runTest("compiler/testData/diagnostics/tests/overload/OverloadVarAndFunInClass.kt");
}
@Test
@TestMetadata("overloadsFromCurrentAndSuperClass.kt")
public void testOverloadsFromCurrentAndSuperClass() throws Exception {
runTest("compiler/testData/diagnostics/tests/overload/overloadsFromCurrentAndSuperClass.kt");
}
@Test
@TestMetadata("overloadsFromCurrentAndSuperClassWithReturnType.kt")
public void testOverloadsFromCurrentAndSuperClassWithReturnType() throws Exception {
runTest("compiler/testData/diagnostics/tests/overload/overloadsFromCurrentAndSuperClassWithReturnType.kt");
}
@Test
@TestMetadata("SyntheticAndNotSynthetic.kt")
public void testSyntheticAndNotSynthetic() throws Exception {
compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java
Generated
+12
View File
@@ -23422,6 +23422,18 @@ public class FirOldFrontendDiagnosticsWithLightTreeTestGenerated extends Abstrac
runTest("compiler/testData/diagnostics/tests/overload/OverloadVarAndFunInClass.kt");
}
@Test
@TestMetadata("overloadsFromCurrentAndSuperClass.kt")
public void testOverloadsFromCurrentAndSuperClass() throws Exception {
runTest("compiler/testData/diagnostics/tests/overload/overloadsFromCurrentAndSuperClass.kt");
}
@Test
@TestMetadata("overloadsFromCurrentAndSuperClassWithReturnType.kt")
public void testOverloadsFromCurrentAndSuperClassWithReturnType() throws Exception {
runTest("compiler/testData/diagnostics/tests/overload/overloadsFromCurrentAndSuperClassWithReturnType.kt");
}
@Test
@TestMetadata("SyntheticAndNotSynthetic.kt")
public void testSyntheticAndNotSynthetic() throws Exception {
compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/JsConeConflictResolverFactory.kt
+7 -2
View File
@@ -7,7 +7,10 @@ package org.jetbrains.kotlin.fir.session
import org.jetbrains.kotlin.fir.NoMutableState
import org.jetbrains.kotlin.fir.resolve.BodyResolveComponents
import org.jetbrains.kotlin.fir.resolve.calls.*
import org.jetbrains.kotlin.fir.resolve.calls.ConeCallConflictResolverFactory
import org.jetbrains.kotlin.fir.resolve.calls.ConeCompositeConflictResolver
import org.jetbrains.kotlin.fir.resolve.calls.ConeIntegerOperatorConflictResolver
import org.jetbrains.kotlin.fir.resolve.calls.ConeOverloadConflictResolver
import org.jetbrains.kotlin.fir.resolve.calls.jvm.ConeEquivalentCallConflictResolver
import org.jetbrains.kotlin.fir.resolve.inference.InferenceComponents
import org.jetbrains.kotlin.fir.types.typeContext
@@ -22,11 +25,13 @@ object JsCallConflictResolverFactory : ConeCallConflictResolverFactory() {
transformerComponents: BodyResolveComponents
): ConeCompositeConflictResolver {
val specificityComparator = JsTypeSpecificityComparatorWithoutDelegate(components.session.typeContext)
// NB: Please, be aware that adding might not necessarily help you because ConeOverloadConflictResolver doesn't just filter out
// less specific candidates, but leave the set the same if there are more than one same-specifity candidates.
// Thus, in that case, your new ConeCallConflictResolver might get all the candidates in that case.
return ConeCompositeConflictResolver(
ConeOverloadConflictResolver(specificityComparator, components, transformerComponents),
ConeEquivalentCallConflictResolver(specificityComparator, components, transformerComponents),
ConeIntegerOperatorConflictResolver,
FilteringOutOriginalInPresenceOfSmartCastConeCallConflictResolver
)
}
}
compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/NativeCallConflictResolverFactory.kt
+7 -2
View File
@@ -6,7 +6,10 @@
package org.jetbrains.kotlin.fir.session
import org.jetbrains.kotlin.fir.resolve.BodyResolveComponents
import org.jetbrains.kotlin.fir.resolve.calls.*
import org.jetbrains.kotlin.fir.resolve.calls.ConeCallConflictResolverFactory
import org.jetbrains.kotlin.fir.resolve.calls.ConeCompositeConflictResolver
import org.jetbrains.kotlin.fir.resolve.calls.ConeIntegerOperatorConflictResolver
import org.jetbrains.kotlin.fir.resolve.calls.ConeOverloadConflictResolver
import org.jetbrains.kotlin.fir.resolve.calls.jvm.ConeEquivalentCallConflictResolver
import org.jetbrains.kotlin.fir.resolve.inference.InferenceComponents
import org.jetbrains.kotlin.resolve.calls.results.TypeSpecificityComparator
@@ -18,11 +21,13 @@ object NativeCallConflictResolverFactory : ConeCallConflictResolverFactory() {
transformerComponents: BodyResolveComponents
): ConeCompositeConflictResolver {
val specificityComparator = TypeSpecificityComparator.NONE
// NB: Please, be aware that adding might not necessarily help you because ConeOverloadConflictResolver doesn't just filter out
// less specific candidates, but leave the set the same if there are more than one same-specifity candidates.
// Thus, in that case, your new ConeCallConflictResolver might get all the candidates in that case.
return ConeCompositeConflictResolver(
ConeOverloadConflictResolver(specificityComparator, components, transformerComponents),
ConeEquivalentCallConflictResolver(specificityComparator, components, transformerComponents),
ConeIntegerOperatorConflictResolver,
FilteringOutOriginalInPresenceOfSmartCastConeCallConflictResolver
)
}
}
compiler/fir/java/src/org/jetbrains/kotlin/fir/resolve/calls/jvm/JvmCallConflictResolverFactory.kt
+7 -2
View File
@@ -7,7 +7,10 @@ package org.jetbrains.kotlin.fir.resolve.calls.jvm
import org.jetbrains.kotlin.fir.NoMutableState
import org.jetbrains.kotlin.fir.resolve.BodyResolveComponents
import org.jetbrains.kotlin.fir.resolve.calls.*
import org.jetbrains.kotlin.fir.resolve.calls.ConeCallConflictResolverFactory
import org.jetbrains.kotlin.fir.resolve.calls.ConeCompositeConflictResolver
import org.jetbrains.kotlin.fir.resolve.calls.ConeIntegerOperatorConflictResolver
import org.jetbrains.kotlin.fir.resolve.calls.ConeOverloadConflictResolver
import org.jetbrains.kotlin.fir.resolve.inference.InferenceComponents
import org.jetbrains.kotlin.fir.types.typeContext
import org.jetbrains.kotlin.resolve.calls.results.TypeSpecificityComparator
@@ -21,12 +24,14 @@ object JvmCallConflictResolverFactory : ConeCallConflictResolverFactory() {
transformerComponents: BodyResolveComponents
): ConeCompositeConflictResolver {
val specificityComparator = JvmTypeSpecificityComparator(components.session.typeContext)
// NB: Please, be aware that adding might not necessarily help you because ConeOverloadConflictResolver doesn't just filter out
// less specific candidates, but leave the set the same if there are more than one same-specifity candidates.
// Thus, in that case, your new ConeCallConflictResolver might get all the candidates in that case.
return ConeCompositeConflictResolver(
ConeOverloadConflictResolver(specificityComparator, components, transformerComponents),
ConeEquivalentCallConflictResolver(specificityComparator, components, transformerComponents),
JvmPlatformOverloadsConflictResolver(components.session),
ConeIntegerOperatorConflictResolver,
FilteringOutOriginalInPresenceOfSmartCastConeCallConflictResolver
)
}
}
compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ConeOverloadConflictResolver.kt
+59 -6
View File
@@ -57,7 +57,8 @@ class ConeOverloadConflictResolver(
discriminateGenerics,
discriminateAbstracts,
discriminateSAMs = true,
discriminateSuspendConversions = true
discriminateSuspendConversions = true,
discriminateByUnwrappedSmartCastOrigin = true,
)
}
@@ -78,8 +79,10 @@ class ConeOverloadConflictResolver(
candidates: Set<Candidate>,
discriminateGenerics: Boolean,
discriminateAbstracts: Boolean,
// Only set to 'false' by recursive calls when the relevant discrimination kind has been already applied
discriminateSAMs: Boolean,
discriminateSuspendConversions: Boolean,
discriminateByUnwrappedSmartCastOrigin: Boolean,
): Set<Candidate> {
findMaximallySpecificCall(candidates, false)?.let { return setOf(it) }
@@ -94,7 +97,11 @@ class ConeOverloadConflictResolver(
0, candidates.size -> {
}
else -> return chooseMaximallySpecificCandidates(
filtered, discriminateGenerics, discriminateAbstracts, discriminateSAMs = false, discriminateSuspendConversions
filtered, discriminateGenerics,
discriminateAbstracts,
discriminateSAMs = false,
discriminateSuspendConversions,
discriminateByUnwrappedSmartCastOrigin,
)
}
}
@@ -109,8 +116,9 @@ class ConeOverloadConflictResolver(
filtered,
discriminateGenerics,
discriminateAbstracts,
discriminateSAMs = false,
discriminateSuspendConversions = false
discriminateSAMs,
discriminateSuspendConversions = false,
discriminateByUnwrappedSmartCastOrigin,
)
}
}
@@ -125,8 +133,53 @@ class ConeOverloadConflictResolver(
filtered,
discriminateGenerics,
discriminateAbstracts = false,
discriminateSAMs = false,
discriminateSuspendConversions = false
discriminateSAMs,
discriminateSuspendConversions,
discriminateByUnwrappedSmartCastOrigin,
)
}
}
if (discriminateByUnwrappedSmartCastOrigin) {
// 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
val filtered = candidates.filterTo(mutableSetOf()) { !it.isFromOriginalTypeInPresenceOfSmartCast }
when (filtered.size) {
1 -> return filtered
0, candidates.size -> {
}
else -> return chooseMaximallySpecificCandidates(
filtered,
discriminateGenerics,
discriminateAbstracts,
discriminateSAMs,
discriminateSuspendConversions,
discriminateByUnwrappedSmartCastOrigin = false,
)
}
}
compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/FilteringOutOriginalInPresenceOfSmartCastConeCallConflictResolver.kt
-76
View File
@@ -1,76 +0,0 @@
/*
* 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
}
}
compiler/testData/diagnostics/tests/overload/overloadsFromCurrentAndSuperClass.kt
Vendored
+23
View File
@@ -0,0 +1,23 @@
// SKIP_TXT
// FIR_IDENTICAL
abstract class A {
open public fun foo(x: Any) {}
open public fun foo(x: String) {}
}
class B : A() {
override fun foo(x: Any) {
super.foo(x)
}
override fun foo(x: String) {
super.foo(x)
}
}
fun bar(a: A) {
if (a is B) {
a.foo("")
}
}
compiler/testData/diagnostics/tests/overload/overloadsFromCurrentAndSuperClassWithReturnType.kt
Vendored
+18
View File
@@ -0,0 +1,18 @@
// SKIP_TXT
// FIR_IDENTICAL
abstract class A {
open public fun foo(x: Any): Any = x
open public fun foo(x: String): String = x
}
class B : A() {
override fun foo(x: Any): Any = x
override fun foo(x: String): String = x
}
fun bar(a: A) {
if (a is B) {
a.foo("").length
}
}
compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java
Generated
+12
View File
@@ -23428,6 +23428,18 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest {
runTest("compiler/testData/diagnostics/tests/overload/OverloadVarAndFunInClass.kt");
}
@Test
@TestMetadata("overloadsFromCurrentAndSuperClass.kt")
public void testOverloadsFromCurrentAndSuperClass() throws Exception {
runTest("compiler/testData/diagnostics/tests/overload/overloadsFromCurrentAndSuperClass.kt");
}
@Test
@TestMetadata("overloadsFromCurrentAndSuperClassWithReturnType.kt")
public void testOverloadsFromCurrentAndSuperClassWithReturnType() throws Exception {
runTest("compiler/testData/diagnostics/tests/overload/overloadsFromCurrentAndSuperClassWithReturnType.kt");
}
@Test
@TestMetadata("SyntheticAndNotSynthetic.kt")
public void testSyntheticAndNotSynthetic() throws Exception {