From f12a4e08cf0ce605095c00e562437f0ba46e36ab Mon Sep 17 00:00:00 2001 From: "Denis.Zharkov" Date: Fri, 2 Dec 2022 18:19:06 +0100 Subject: [PATCH] K2: Do not force completion for 'when' branches with expected type Otherwise, it leads to branches inference run fully independent, while there are cases when it's necessary to flow type information from one of the branch to another (see the new test). NB. In K1, it worked differently: if branches were inferred altogether only for Any/Any? expect types (otherwise they're analyzed independently) See foo2/foo4 in the test. To avoid breaking change we need to support foo1/foo3, but we're trying not to have some special rule for Any, so we've got a new resolution mode that provides expect type, but doesn't require full completion. ^KT-45989 Fixed ^KT-56563 Fixed ^KT-54709 Related For change in specialCallWithMaterializeAndExpectedType.kt At first, see at KT-36776 Long time ago, it's been decided that if/when resolution should look similar to similar "select()" calls, but it's a breaking change (see KT-36776), and we were ready for that back then. But then, there were too many broken cases found, thus we reverted it at 100a6f70ca8953bad2755cf438beade943221ea6 But probably, it would be better to try to infer `String?` instead of `Nothing?` (see next commits) Note that change in specialCallWithMaterializeAndExpectedType.kt will be addressed in later commits, too --- ...CompilerTestFE10TestdataTestGenerated.java | 6 ++ ...irOldFrontendDiagnosticsTestGenerated.java | 6 ++ ...DiagnosticsWithLightTreeTestGenerated.java | 6 ++ .../kotlin/fir/resolve/ResolutionMode.kt | 13 +++- ...ControlFlowStatementsResolveTransformer.kt | 11 ++-- .../FirExpressionsResolveTransformer.kt | 2 +- .../ifElseIntersection.fir.kt | 2 +- .../diagnostics/tests/generics/kt53656.fir.kt | 2 +- .../inference/ifWithDependentBranches.fir.kt | 62 +++++++++++++++++++ .../inference/ifWithDependentBranches.kt | 62 +++++++++++++++++++ ...lCallWithMaterializeAndExpectedType.fir.kt | 7 +++ ...ecialCallWithMaterializeAndExpectedType.kt | 1 - .../test/runners/DiagnosticTestGenerated.java | 6 ++ 13 files changed, 177 insertions(+), 9 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/inference/ifWithDependentBranches.fir.kt create mode 100644 compiler/testData/diagnostics/tests/inference/ifWithDependentBranches.kt create mode 100644 compiler/testData/diagnostics/tests/inference/nothingType/specialCallWithMaterializeAndExpectedType.fir.kt diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java index b231510df52..5a82bbd415d 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java @@ -14168,6 +14168,12 @@ public class DiagnosisCompilerTestFE10TestdataTestGenerated extends AbstractDiag runTest("compiler/testData/diagnostics/tests/inference/hasErrorInConstrainingTypes.kt"); } + @Test + @TestMetadata("ifWithDependentBranches.kt") + public void testIfWithDependentBranches() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/ifWithDependentBranches.kt"); + } + @Test @TestMetadata("illegalUnderscoredTypeArgument.kt") public void testIllegalUnderscoredTypeArgument() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java index 3ac14a19342..dfd79536f7e 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java @@ -14174,6 +14174,12 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirDiagnosti runTest("compiler/testData/diagnostics/tests/inference/hasErrorInConstrainingTypes.kt"); } + @Test + @TestMetadata("ifWithDependentBranches.kt") + public void testIfWithDependentBranches() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/ifWithDependentBranches.kt"); + } + @Test @TestMetadata("illegalUnderscoredTypeArgument.kt") public void testIllegalUnderscoredTypeArgument() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java index d062af2bba8..4502b796581 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java @@ -14168,6 +14168,12 @@ public class FirOldFrontendDiagnosticsWithLightTreeTestGenerated extends Abstrac runTest("compiler/testData/diagnostics/tests/inference/hasErrorInConstrainingTypes.kt"); } + @Test + @TestMetadata("ifWithDependentBranches.kt") + public void testIfWithDependentBranches() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/ifWithDependentBranches.kt"); + } + @Test @TestMetadata("illegalUnderscoredTypeArgument.kt") public void testIllegalUnderscoredTypeArgument() throws Exception { diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/ResolutionMode.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/ResolutionMode.kt index 4e1adaa3b77..af53a9e2fec 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/ResolutionMode.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/ResolutionMode.kt @@ -41,7 +41,18 @@ sealed class ResolutionMode(val forceFullCompletion: Boolean) { // In these examples we should try using the property type information while resolving the initializer, // but it's ok if it's not applicable val shouldBeStrictlyEnforced: Boolean = true, - ) : ResolutionMode(forceFullCompletion = true) { + // Currently the only case for expected type when we don't force completion are when's branches + forceFullCompletion: Boolean = true, + ) : ResolutionMode(forceFullCompletion) { + + fun copy( + mayBeCoercionToUnitApplied: Boolean = this.mayBeCoercionToUnitApplied, + forceFullCompletion: Boolean = this.forceFullCompletion + ): WithExpectedType = WithExpectedType( + expectedTypeRef, mayBeCoercionToUnitApplied, expectedTypeMismatchIsReportedInChecker, fromCast, shouldBeStrictlyEnforced, + forceFullCompletion + ) + override fun toString(): String { return "WithExpectedType: ${expectedTypeRef.prettyString()}, " + "mayBeCoercionToUnitApplied=${mayBeCoercionToUnitApplied}, " + diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirControlFlowStatementsResolveTransformer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirControlFlowStatementsResolveTransformer.kt index 00e28b6db61..655ea3eeb00 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirControlFlowStatementsResolveTransformer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirControlFlowStatementsResolveTransformer.kt @@ -74,12 +74,15 @@ class FirControlFlowStatementsResolveTransformer(transformer: FirAbstractBodyRes // when with one branch cannot be completed if it's not already complete in the first place } else -> { + val resolutionModeForBranches = + (data as? ResolutionMode.WithExpectedType) + // Currently we don't use information from cast, but probably we could have + ?.takeUnless { it.fromCast } + ?.copy(forceFullCompletion = false) + ?: ResolutionMode.ContextDependent whenExpression = whenExpression.transformBranches( transformer, - data.takeIf { - val expectedType = it.expectedType - expectedType != null && expectedType !is FirImplicitTypeRef - } ?: ResolutionMode.ContextDependent, + resolutionModeForBranches, ) whenExpression = syntheticCallGenerator.generateCalleeForWhenExpression(whenExpression, resolutionContext) ?: run { diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirExpressionsResolveTransformer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirExpressionsResolveTransformer.kt index c46c8eba5d8..b00ec8419df 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirExpressionsResolveTransformer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirExpressionsResolveTransformer.kt @@ -518,7 +518,7 @@ open class FirExpressionsResolveTransformer(transformer: FirAbstractBodyResolveT val value = if (index == numberOfStatements - 1) if (data is ResolutionMode.WithExpectedType) - ResolutionMode.WithExpectedType(data.expectedTypeRef, mayBeCoercionToUnitApplied = true) + data.copy(mayBeCoercionToUnitApplied = true) else data else diff --git a/compiler/testData/diagnostics/tests/controlStructures/ifElseIntersection.fir.kt b/compiler/testData/diagnostics/tests/controlStructures/ifElseIntersection.fir.kt index bc955bc78e2..ba8d25dc29a 100644 --- a/compiler/testData/diagnostics/tests/controlStructures/ifElseIntersection.fir.kt +++ b/compiler/testData/diagnostics/tests/controlStructures/ifElseIntersection.fir.kt @@ -26,7 +26,7 @@ fun bind2(r: Option): Option { fun bind3(r: Option): Option { return if (r is Some) { // Diagnoses an error correctly - if (true) None() else r + if (true) None() else r } else r } diff --git a/compiler/testData/diagnostics/tests/generics/kt53656.fir.kt b/compiler/testData/diagnostics/tests/generics/kt53656.fir.kt index d8858e99ac8..bb429822a01 100644 --- a/compiler/testData/diagnostics/tests/generics/kt53656.fir.kt +++ b/compiler/testData/diagnostics/tests/generics/kt53656.fir.kt @@ -5,7 +5,7 @@ interface Entity abstract class SecuredEntity(val entity: E) where E : Entity, E : SecurityCodeAware<*,*> interface SecurityCodeAware> where E : Entity, E : SecurityCodeAware fun > SecurityCodeAware.secured() : R where E : Entity, E : SecurityCodeAware = when(this) { - is Order -> SecuredOrder(this) + is Order -> SecuredOrder(this) else -> null!! } class Order : Entity diff --git a/compiler/testData/diagnostics/tests/inference/ifWithDependentBranches.fir.kt b/compiler/testData/diagnostics/tests/inference/ifWithDependentBranches.fir.kt new file mode 100644 index 00000000000..033680524ff --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/ifWithDependentBranches.fir.kt @@ -0,0 +1,62 @@ +// SKIP_TXT + +interface Additional +interface A : Additional + +fun aOf(): A = TODO() + +interface B + +fun B.convert(): A = TODO() + +fun foo1(x: B): Any { + return if (x.hashCode() == 0) aOf() else x.convert() +} + +fun foo2(x: B): Additional { + return if (x.hashCode() == 0) aOf() else x.convert() +} + +fun foo3(x: B): Any { + return when { + x.hashCode() == 0 -> aOf() + else -> x.convert() + } +} + +fun foo4(x: B): Additional { + return when { + x.hashCode() == 0 -> aOf() + else -> x.convert() + } +} + +fun foo5(x: B): Any { + return if (x.hashCode() == 0) { + aOf() + } else { + x.convert() + } +} + +fun foo6(x: B): Additional { + return if (x.hashCode() == 0) { + aOf() + } else { + x.convert() + } +} + +fun foo7(x: B): Any { + return when { + x.hashCode() == 0 -> { aOf() } + else -> { x.convert() } + } +} + +fun foo8(x: B): Additional { + return when { + x.hashCode() == 0 -> { aOf() } + else -> { x.convert() } + } +} diff --git a/compiler/testData/diagnostics/tests/inference/ifWithDependentBranches.kt b/compiler/testData/diagnostics/tests/inference/ifWithDependentBranches.kt new file mode 100644 index 00000000000..ffec5988699 --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/ifWithDependentBranches.kt @@ -0,0 +1,62 @@ +// SKIP_TXT + +interface Additional +interface A : Additional + +fun aOf(): A = TODO() + +interface B + +fun B.convert(): A = TODO() + +fun foo1(x: B): Any { + return if (x.hashCode() == 0) aOf() else x.convert() +} + +fun foo2(x: B): Additional { + return if (x.hashCode() == 0) aOf() else x.convert() +} + +fun foo3(x: B): Any { + return when { + x.hashCode() == 0 -> aOf() + else -> x.convert() + } +} + +fun foo4(x: B): Additional { + return when { + x.hashCode() == 0 -> aOf() + else -> x.convert() + } +} + +fun foo5(x: B): Any { + return if (x.hashCode() == 0) { + aOf() + } else { + x.convert() + } +} + +fun foo6(x: B): Additional { + return if (x.hashCode() == 0) { + aOf() + } else { + x.convert() + } +} + +fun foo7(x: B): Any { + return when { + x.hashCode() == 0 -> { aOf() } + else -> { x.convert() } + } +} + +fun foo8(x: B): Additional { + return when { + x.hashCode() == 0 -> { aOf() } + else -> { x.convert() } + } +} diff --git a/compiler/testData/diagnostics/tests/inference/nothingType/specialCallWithMaterializeAndExpectedType.fir.kt b/compiler/testData/diagnostics/tests/inference/nothingType/specialCallWithMaterializeAndExpectedType.fir.kt new file mode 100644 index 00000000000..21c46587a59 --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/nothingType/specialCallWithMaterializeAndExpectedType.fir.kt @@ -0,0 +1,7 @@ +// !DIAGNOSTICS: -UNUSED_VARIABLE + +fun foo() { + val s: String? = if (true) materialize() else null +} + +fun materialize(): K = TODO() \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/inference/nothingType/specialCallWithMaterializeAndExpectedType.kt b/compiler/testData/diagnostics/tests/inference/nothingType/specialCallWithMaterializeAndExpectedType.kt index 57c9839179b..e52674c522d 100644 --- a/compiler/testData/diagnostics/tests/inference/nothingType/specialCallWithMaterializeAndExpectedType.kt +++ b/compiler/testData/diagnostics/tests/inference/nothingType/specialCallWithMaterializeAndExpectedType.kt @@ -1,4 +1,3 @@ -// FIR_IDENTICAL // !DIAGNOSTICS: -UNUSED_VARIABLE fun 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 4072b774ad4..9ca733aabfa 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 @@ -14174,6 +14174,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { runTest("compiler/testData/diagnostics/tests/inference/hasErrorInConstrainingTypes.kt"); } + @Test + @TestMetadata("ifWithDependentBranches.kt") + public void testIfWithDependentBranches() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/ifWithDependentBranches.kt"); + } + @Test @TestMetadata("illegalUnderscoredTypeArgument.kt") public void testIllegalUnderscoredTypeArgument() throws Exception {