From eac4b81b11b098bdb259c49ac91b1dcea9e7d268 Mon Sep 17 00:00:00 2001 From: Nikita Bobko Date: Wed, 2 Aug 2023 19:12:41 +0200 Subject: [PATCH] [FE] Fix TypeParameterUpperBounds incompatibility priority ^KT-60902 Fixed Review: https://jetbrains.team/p/kt/reviews/11039/timeline We should prioritize to return STRONG incompatibilities over WEAK incompatibilities. But this invariant broke in `areCompatibleCallables`, because `areCompatibleTypeParameters` returns incompatibilities of both types, and `areCompatibleTypeParameters` is called in WEAK incompatibilities section. The fix is to split `areCompatibleTypeParameters` into two functions: `areStrongIncompatibleTypeParameters` and `areWeakIncompatibleTypeParameters`. And call each of this function in appropriate `areCompatibleCallables` sections. --- ...DiagnosticsWithLightTreeTestGenerated.java | 6 ++++ ...endMPPDiagnosticsWithPsiTestGenerated.java | 6 ++++ ...bstractExpectActualCompatibilityChecker.kt | 35 +++++++++++++++---- .../tests/multiplatform/kt60902.kt | 13 +++++++ .../test/runners/DiagnosticTestGenerated.java | 6 ++++ 5 files changed, 59 insertions(+), 7 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/multiplatform/kt60902.kt diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java index d701ec40417..0e49236cb84 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java @@ -205,6 +205,12 @@ public class FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated extends Abst runTest("compiler/testData/diagnostics/tests/multiplatform/kt58153.kt"); } + @Test + @TestMetadata("kt60902.kt") + public void testKt60902() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/kt60902.kt"); + } + @Test @TestMetadata("manyImplMemberNotImplemented.kt") public void testManyImplMemberNotImplemented() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java index 062083fa793..415a89590bf 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java @@ -205,6 +205,12 @@ public class FirOldFrontendMPPDiagnosticsWithPsiTestGenerated extends AbstractFi runTest("compiler/testData/diagnostics/tests/multiplatform/kt58153.kt"); } + @Test + @TestMetadata("kt60902.kt") + public void testKt60902() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/kt60902.kt"); + } + @Test @TestMetadata("manyImplMemberNotImplemented.kt") public void testManyImplMemberNotImplemented() throws Exception { diff --git a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualCompatibilityChecker.kt b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualCompatibilityChecker.kt index 3001ce86e82..789bcc6ed31 100644 --- a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualCompatibilityChecker.kt +++ b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualCompatibilityChecker.kt @@ -298,6 +298,8 @@ object AbstractExpectActualCompatibilityChecker { return ExpectActualCompatibility.Compatible } + // ALL THE FOLLOWING ARE STRONG INCOMPATIBILITIES + if (expectDeclaration is FunctionSymbolMarker != actualDeclaration is FunctionSymbolMarker) { return Incompatible.CallableKind } @@ -345,6 +347,10 @@ object AbstractExpectActualCompatibilityChecker { } } + areStrongIncompatibleTypeParameters(expectedTypeParameters, actualTypeParameters, substitutor)?.let { return it } + + // ALL THE FOLLOWING ARE WEAK INCOMPATIBILITIES + if (actualDeclaration.hasStableParameterNames && !equalsBy(expectedValueParameters, actualValueParameters) { it.name }) { return Incompatible.ParameterNames } @@ -370,11 +376,7 @@ object AbstractExpectActualCompatibilityChecker { return Incompatible.Visibility } - areCompatibleTypeParameters(expectedTypeParameters, actualTypeParameters, substitutor).let { - if (it != ExpectActualCompatibility.Compatible) { - return it - } - } + areWeakIncompatibleTypeParameters(expectedTypeParameters, actualTypeParameters)?.let { return it } if (shouldCheckAbsenceOfDefaultParamsInActual) { // "Default parameters in actual" check is required only for functions, because only functions can have parameters @@ -535,7 +537,18 @@ object AbstractExpectActualCompatibilityChecker { expectTypeParameterSymbols: List, actualTypeParameterSymbols: List, substitutor: TypeSubstitutorMarker, - ): ExpectActualCompatibility<*> { + ): ExpectActualCompatibility<*> = + // We must prioritize to return STRONG incompatible over WEAK incompatible (because STRONG incompatibility allows to search for overloads) + areStrongIncompatibleTypeParameters(expectTypeParameterSymbols, actualTypeParameterSymbols, substitutor) + ?: areWeakIncompatibleTypeParameters(expectTypeParameterSymbols, actualTypeParameterSymbols) + ?: ExpectActualCompatibility.Compatible + + context(ExpectActualMatchingContext<*>) + private fun areStrongIncompatibleTypeParameters( + expectTypeParameterSymbols: List, + actualTypeParameterSymbols: List, + substitutor: TypeSubstitutorMarker, + ): Incompatible<*>? { for (i in expectTypeParameterSymbols.indices) { val expectBounds = expectTypeParameterSymbols[i].bounds val actualBounds = actualTypeParameterSymbols[i].bounds @@ -547,6 +560,14 @@ object AbstractExpectActualCompatibilityChecker { } } + return null + } + + context(ExpectActualMatchingContext<*>) + private fun areWeakIncompatibleTypeParameters( + expectTypeParameterSymbols: List, + actualTypeParameterSymbols: List, + ): Incompatible<*>? { if (!equalsBy(expectTypeParameterSymbols, actualTypeParameterSymbols) { it.variance }) { return Incompatible.TypeParameterVariance } @@ -560,7 +581,7 @@ object AbstractExpectActualCompatibilityChecker { return Incompatible.TypeParameterReified } - return ExpectActualCompatibility.Compatible + return null } context(ExpectActualMatchingContext<*>) diff --git a/compiler/testData/diagnostics/tests/multiplatform/kt60902.kt b/compiler/testData/diagnostics/tests/multiplatform/kt60902.kt new file mode 100644 index 00000000000..d75e9ccc922 --- /dev/null +++ b/compiler/testData/diagnostics/tests/multiplatform/kt60902.kt @@ -0,0 +1,13 @@ +// FIR_IDENTICAL +// MODULE: m1-common +// FILE: common.kt + +public expect fun > Array.foo() + +// MODULE: m2-jvm()()(m1-common) +// FILE: jvm.kt +public actual fun > Array.foo() { +} + +private fun Array.foo() { +} diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java index 9e311f34d97..10d9ae27c6e 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java @@ -22884,6 +22884,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { runTest("compiler/testData/diagnostics/tests/multiplatform/kt58153.kt"); } + @Test + @TestMetadata("kt60902.kt") + public void testKt60902() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/kt60902.kt"); + } + @Test @TestMetadata("manyImplMemberNotImplemented.kt") public void testManyImplMemberNotImplemented() throws Exception {