From bc5180656d3e4dc9994bd9bf4afee89514ee13c1 Mon Sep 17 00:00:00 2001 From: Nikita Bobko Date: Fri, 13 Oct 2023 13:57:28 +0200 Subject: [PATCH] [FIR] ReturnType expect-actual incompatibility: convert from strong to weak It should have been WeakIncompatible from the beginning because it's not possible to overload by return type in Kotlin This commit is a step forward to fix KT-62591 Unfortunately, the test cannot demonstrate the problem because of another bug in K2 KT-59887 ^KT-62752 Fixed Review: https://jetbrains.team/p/kt/reviews/12750/timeline Motivation: It makes expect-actual matching-checking model more consistent. expect-actual "matching" is run before FirResolvePhase.BODY_RESOLVE. You can't know return types, until you run BODY_RESOLVE. That's why the return type can't be checked during expect-actual matching. But it's cursed: you have something that have to match by, but, at the same time, you can't do it. expect-actual "checking" is run after FirResolvePhase.BODY_RESOLVE. That's why if we convert ReturnType incompatibility to WeakIncompatible (which should have been called CheckingIncompatible), then expect-actual matching model becomes consistent. We will also be able to get rid of unnecessary FirActualCallableDeclarationChecker. Because it won't be necessary. Return types will be checked by common logic of expect-actual "checker" --- ...DiagnosticsWithLightTreeTestGenerated.java | 6 +++ ...endMPPDiagnosticsWithPsiTestGenerated.java | 6 +++ ...bstractExpectActualCompatibilityChecker.kt | 43 +++++++++++++------ ...VsGenericsUpperBoundIncompatibility.fir.kt | 11 +++++ ...TypeVsGenericsUpperBoundIncompatibility.kt | 11 +++++ .../test/runners/DiagnosticTestGenerated.java | 6 +++ .../ExpectActualCompatibility.kt | 2 +- 7 files changed, 70 insertions(+), 15 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/multiplatform/returnTypeVsGenericsUpperBoundIncompatibility.fir.kt create mode 100644 compiler/testData/diagnostics/tests/multiplatform/returnTypeVsGenericsUpperBoundIncompatibility.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 e67a6b1fe13..0d76f9de0f6 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 @@ -301,6 +301,12 @@ public class FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated extends Abst runTest("compiler/testData/diagnostics/tests/multiplatform/privateTopLevelDeclarations.kt"); } + @Test + @TestMetadata("returnTypeVsGenericsUpperBoundIncompatibility.kt") + public void testReturnTypeVsGenericsUpperBoundIncompatibility() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/returnTypeVsGenericsUpperBoundIncompatibility.kt"); + } + @Test @TestMetadata("sealedClassWithPrivateConstructor.kt") public void testSealedClassWithPrivateConstructor() 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 2f917c35f21..8c891e0404e 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 @@ -301,6 +301,12 @@ public class FirOldFrontendMPPDiagnosticsWithPsiTestGenerated extends AbstractFi runTest("compiler/testData/diagnostics/tests/multiplatform/privateTopLevelDeclarations.kt"); } + @Test + @TestMetadata("returnTypeVsGenericsUpperBoundIncompatibility.kt") + public void testReturnTypeVsGenericsUpperBoundIncompatibility() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/returnTypeVsGenericsUpperBoundIncompatibility.kt"); + } + @Test @TestMetadata("sealedClassWithPrivateConstructor.kt") public void testSealedClassWithPrivateConstructor() 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 18e65686a5e..d5b2d2d4911 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 @@ -132,7 +132,7 @@ object AbstractExpectActualCompatibilityChecker { val substitutor = createExpectActualTypeParameterSubstitutor( (expectTypeParameterSymbols zipIfSizesAreEqual actualTypeParameterSymbols) - ?: error("expect/actual type parameters sizes are checked above"), + ?: error("expect/actual type parameters sizes are checked earlier"), parentSubstitutor ) @@ -323,7 +323,14 @@ object AbstractExpectActualCompatibilityChecker { // We must prioritize to return STRONG incompatible over WEAK incompatible (because STRONG incompatibility allows to search for overloads) val annotationMode = expectContainingClass?.classKind == ClassKind.ANNOTATION_CLASS return getCallablesStrongIncompatibility(expectDeclaration, actualDeclaration, annotationMode, parentSubstitutor) - ?: getCallablesWeakIncompatibility(expectDeclaration, actualDeclaration, expectContainingClass, actualContainingClass) + ?: getCallablesWeakIncompatibility( + expectDeclaration, + actualDeclaration, + annotationMode, + parentSubstitutor, + expectContainingClass, + actualContainingClass + ) ?: ExpectActualCompatibility.Compatible } @@ -358,7 +365,7 @@ object AbstractExpectActualCompatibilityChecker { val substitutor = createExpectActualTypeParameterSubstitutor( (expectedTypeParameters zipIfSizesAreEqual actualTypeParameters) - ?: error("expect/actual type parameters sizes are checked above"), + ?: error("expect/actual type parameters sizes are checked earlier"), parentSubstitutor ) @@ -376,17 +383,6 @@ object AbstractExpectActualCompatibilityChecker { return Incompatible.ParameterTypes } - if (shouldCheckReturnTypesOfCallables) { - if (!areCompatibleExpectActualTypes( - substitutor.safeSubstitute(expectDeclaration.returnType), - actualDeclaration.returnType, - parameterOfAnnotationComparisonMode = insideAnnotationClass - ) - ) { - return Incompatible.ReturnType - } - } - if (!areCompatibleTypeParameterUpperBounds(expectedTypeParameters, actualTypeParameters, substitutor)) { return Incompatible.FunctionTypeParameterUpperBounds } @@ -398,6 +394,8 @@ object AbstractExpectActualCompatibilityChecker { private fun getCallablesWeakIncompatibility( expectDeclaration: CallableSymbolMarker, actualDeclaration: CallableSymbolMarker, + insideAnnotationClass: Boolean, + parentSubstitutor: TypeSubstitutorMarker?, expectContainingClass: RegularClassSymbolMarker?, actualContainingClass: RegularClassSymbolMarker?, ): Incompatible.WeakIncompatible<*>? { @@ -406,6 +404,23 @@ object AbstractExpectActualCompatibilityChecker { val expectedValueParameters = expectDeclaration.valueParameters val actualValueParameters = actualDeclaration.valueParameters + if (shouldCheckReturnTypesOfCallables) { + val substitutor = createExpectActualTypeParameterSubstitutor( + (expectedTypeParameters zipIfSizesAreEqual actualTypeParameters) + ?: error("expect/actual type parameters sizes are checked earlier"), + parentSubstitutor + ) + + if (!areCompatibleExpectActualTypes( + substitutor.safeSubstitute(expectDeclaration.returnType), + actualDeclaration.returnType, + parameterOfAnnotationComparisonMode = insideAnnotationClass + ) + ) { + return Incompatible.ReturnType + } + } + if (actualDeclaration.hasStableParameterNames && !equalsBy(expectedValueParameters, actualValueParameters) { it.name }) { return Incompatible.ParameterNames } diff --git a/compiler/testData/diagnostics/tests/multiplatform/returnTypeVsGenericsUpperBoundIncompatibility.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/returnTypeVsGenericsUpperBoundIncompatibility.fir.kt new file mode 100644 index 00000000000..824097c280c --- /dev/null +++ b/compiler/testData/diagnostics/tests/multiplatform/returnTypeVsGenericsUpperBoundIncompatibility.fir.kt @@ -0,0 +1,11 @@ +// MODULE: m1-common +// FILE: common.kt + +interface A +expect fun foo(t: T): String + +// MODULE: m2-jvm()()(m1-common) +// FILE: jvm.kt + +fun foo(t: T): T = TODO() +fun foo(t: T): String = TODO() diff --git a/compiler/testData/diagnostics/tests/multiplatform/returnTypeVsGenericsUpperBoundIncompatibility.kt b/compiler/testData/diagnostics/tests/multiplatform/returnTypeVsGenericsUpperBoundIncompatibility.kt new file mode 100644 index 00000000000..8a31cc56432 --- /dev/null +++ b/compiler/testData/diagnostics/tests/multiplatform/returnTypeVsGenericsUpperBoundIncompatibility.kt @@ -0,0 +1,11 @@ +// MODULE: m1-common +// FILE: common.kt + +interface A +expect fun foo(t: T): String + +// MODULE: m2-jvm()()(m1-common) +// FILE: jvm.kt + +fun foo(t: T): T = TODO() +fun foo(t: T): String = TODO() 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 c874ea135ef..018d5259efd 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 @@ -23668,6 +23668,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { runTest("compiler/testData/diagnostics/tests/multiplatform/privateTopLevelDeclarations.kt"); } + @Test + @TestMetadata("returnTypeVsGenericsUpperBoundIncompatibility.kt") + public void testReturnTypeVsGenericsUpperBoundIncompatibility() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/returnTypeVsGenericsUpperBoundIncompatibility.kt"); + } + @Test @TestMetadata("sealedClassWithPrivateConstructor.kt") public void testSealedClassWithPrivateConstructor() throws Exception { diff --git a/core/compiler.common/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectActualCompatibility.kt b/core/compiler.common/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectActualCompatibility.kt index b79e3163c62..876375a05ef 100644 --- a/core/compiler.common/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectActualCompatibility.kt +++ b/core/compiler.common/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectActualCompatibility.kt @@ -33,7 +33,7 @@ sealed class ExpectActualCompatibility { object ClassTypeParameterCount : WeakIncompatible(FunctionTypeParameterCount.reason) object ParameterTypes : StrongIncompatible("parameter types are different") - object ReturnType : StrongIncompatible("return type is different") + object ReturnType : WeakIncompatible("return type is different") object ParameterNames : WeakIncompatible("parameter names are different") object TypeParameterNames : WeakIncompatible("names of type parameters are different")