From 44920f42d8c229846f49fb367e119f5e8e9aaec3 Mon Sep 17 00:00:00 2001 From: Dmitry Savvinov Date: Wed, 17 Jan 2018 19:03:11 +0300 Subject: [PATCH] [NI] Fix unit coercion Consider following case: fun foo(): Unit = run { "hello" } Previously, NI would analyze lambda body without expected type, because it is a type variable 'R' from 'run', which hasn't been fixed yet. This leads to treating "hello" as lambda-return argument and adding bogus 'String' constraint on 'R', and, consequently, type mismatch. Now, we peek into current constraint system and check if return-type of lambda is type variable with upper-Unit constraint (which is exactly condition for its body to be Unit-coerced). If so, then we provide expected Unit-type for body explicitly, and the rest will be done automatically (in particular, in aforementioned example "hello" wouldn't be treated as lambda return argument). --- .../components/PostponedArgumentsAnalyzer.kt | 22 +++++++++-- .../model/NewConstraintSystemImpl.kt | 11 ++++++ .../coercionWithExpectedType.kt | 26 +++++++++++++ .../coercionWithExpectedType.txt | 6 +++ .../coercionWithExpectedTypeAndBound.kt | 13 +++++++ .../coercionWithExpectedTypeAndBound.txt | 5 +++ .../coercionWithoutExpectedType.kt | 19 +++++++++ .../coercionWithoutExpectedType.txt | 4 ++ .../indirectCoercionWithExpectedType.kt | 36 +++++++++++++++++ .../indirectCoercionWithExpectedType.txt | 8 ++++ .../inference/coercionToUnit/noCoercion.kt | 26 +++++++++++++ .../inference/coercionToUnit/noCoercion.txt | 6 +++ .../checkers/DiagnosticsTestGenerated.java | 39 +++++++++++++++++++ .../DiagnosticsUsingJavacTestGenerated.java | 39 +++++++++++++++++++ 14 files changed, 257 insertions(+), 3 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedType.kt create mode 100644 compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedType.txt create mode 100644 compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedTypeAndBound.kt create mode 100644 compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedTypeAndBound.txt create mode 100644 compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithoutExpectedType.kt create mode 100644 compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithoutExpectedType.txt create mode 100644 compiler/testData/diagnostics/tests/inference/coercionToUnit/indirectCoercionWithExpectedType.kt create mode 100644 compiler/testData/diagnostics/tests/inference/coercionToUnit/indirectCoercionWithExpectedType.txt create mode 100644 compiler/testData/diagnostics/tests/inference/coercionToUnit/noCoercion.kt create mode 100644 compiler/testData/diagnostics/tests/inference/coercionToUnit/noCoercion.txt diff --git a/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/components/PostponedArgumentsAnalyzer.kt b/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/components/PostponedArgumentsAnalyzer.kt index d8659bf4276..cb6d0674a5b 100644 --- a/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/components/PostponedArgumentsAnalyzer.kt +++ b/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/components/PostponedArgumentsAnalyzer.kt @@ -34,6 +34,8 @@ class PostponedArgumentsAnalyzer( // type can be proper if it not contains not fixed type variables fun canBeProper(type: UnwrappedType): Boolean + fun hasUpperUnitConstraint(type: UnwrappedType): Boolean + // mutable operations fun addOtherSystem(otherSystem: ConstraintStorage) @@ -73,10 +75,24 @@ class PostponedArgumentsAnalyzer( val receiver = lambda.receiver?.let(::substitute) val parameters = lambda.parameters.map(::substitute) - val expectedType = lambda.returnType.takeIf { c.canBeProper(it) }?.let(::substitute) + val rawReturnType = lambda.returnType - val returnArguments = - resolutionCallbacks.analyzeAndGetLambdaReturnArguments(lambda.atom, lambda.isSuspend, receiver, parameters, expectedType) + val expectedTypeForReturnArguments = when { + c.canBeProper(rawReturnType) -> substitute(rawReturnType) + + // For Unit-coercion + c.hasUpperUnitConstraint(rawReturnType) -> lambda.returnType.builtIns.unitType + + else -> null + } + + val returnArguments = resolutionCallbacks.analyzeAndGetLambdaReturnArguments( + lambda.atom, + lambda.isSuspend, + receiver, + parameters, + expectedTypeForReturnArguments + ) returnArguments.forEach { c.addSubsystemFromArgument(it) } diff --git a/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/inference/model/NewConstraintSystemImpl.kt b/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/inference/model/NewConstraintSystemImpl.kt index 2924214eba9..62690012c08 100644 --- a/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/inference/model/NewConstraintSystemImpl.kt +++ b/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/inference/model/NewConstraintSystemImpl.kt @@ -27,7 +27,9 @@ import org.jetbrains.kotlin.resolve.calls.model.KotlinCallDiagnostic import org.jetbrains.kotlin.resolve.calls.tower.isSuccess import org.jetbrains.kotlin.types.TypeConstructor import org.jetbrains.kotlin.types.UnwrappedType +import org.jetbrains.kotlin.types.checker.NewTypeVariableConstructor import org.jetbrains.kotlin.types.typeUtil.contains +import org.jetbrains.kotlin.types.typeUtil.isUnit import org.jetbrains.kotlin.utils.SmartList class NewConstraintSystemImpl( @@ -248,4 +250,13 @@ class NewConstraintSystemImpl( checkState(State.BUILDING, State.COMPLETION) return storage.buildCurrentSubstitutor() } + + // PostponedArgumentsAnalyzer.Context + override fun hasUpperUnitConstraint(type: UnwrappedType): Boolean { + checkState(State.BUILDING, State.COMPLETION, State.FREEZED) + + val constraints = storage.notFixedTypeVariables[type.constructor]?.constraints ?: return false + + return constraints.any { it.kind == ConstraintKind.UPPER && it.type.isUnit() } + } } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedType.kt b/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedType.kt new file mode 100644 index 00000000000..ee12000834a --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedType.kt @@ -0,0 +1,26 @@ +// !WITH_NEW_INFERENCE + +fun materialize(): T = TODO() + +val a: () -> Unit = l@{ + // Expected type 'Unit' is used here for inference + if (true) return@l materialize() + + // Expected type here is Unit, but it also implies coercion, + // so we can end lambda body with statement + if (true) 42 +} + +val b: () -> Unit = l@{ + // Error, coercion can't be applied at this position! + if (true) return@l "hello" + + // However, this is OK, because here coercion is applied + "hello" +} + +val c: () -> Unit = { + // Interesting enough, for such expessions we use expected type Unit + // (compare that with the previous case, where we didn't used expected type Unit for "hello") + materialize() +} diff --git a/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedType.txt b/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedType.txt new file mode 100644 index 00000000000..df51f7205a6 --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedType.txt @@ -0,0 +1,6 @@ +package + +public val a: () -> kotlin.Unit +public val b: () -> kotlin.Unit +public val c: () -> kotlin.Unit +public fun materialize(): T diff --git a/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedTypeAndBound.kt b/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedTypeAndBound.kt new file mode 100644 index 00000000000..c86551c0788 --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedTypeAndBound.kt @@ -0,0 +1,13 @@ +// !WITH_NEW_INFERENCE + +fun materializeNumber(): T = TODO() + +fun a(): Unit = run { + materializeNumber() +} + +fun b(): Unit = run { + run { + materializeNumber() + } +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedTypeAndBound.txt b/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedTypeAndBound.txt new file mode 100644 index 00000000000..b4ee681fc04 --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedTypeAndBound.txt @@ -0,0 +1,5 @@ +package + +public fun a(): kotlin.Unit +public fun b(): kotlin.Unit +public fun materializeNumber(): T diff --git a/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithoutExpectedType.kt b/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithoutExpectedType.kt new file mode 100644 index 00000000000..2125a3976f3 --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithoutExpectedType.kt @@ -0,0 +1,19 @@ +// !WITH_NEW_INFERENCE + +fun materialize(): T = TODO() + +fun implicitCoercion() { + val a = { + // Block is implicitly Unit-coerced, so it is allowed to place statement at the end of lambda + if (true) 42 + } + + val b = l@{ + return@l + } + + val c = l@{ + // Error: block doesn't have an expected type, so call can't be inferred! + return@l materialize() + } +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithoutExpectedType.txt b/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithoutExpectedType.txt new file mode 100644 index 00000000000..4ab9cd9fb19 --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithoutExpectedType.txt @@ -0,0 +1,4 @@ +package + +public fun implicitCoercion(): kotlin.Unit +public fun materialize(): T diff --git a/compiler/testData/diagnostics/tests/inference/coercionToUnit/indirectCoercionWithExpectedType.kt b/compiler/testData/diagnostics/tests/inference/coercionToUnit/indirectCoercionWithExpectedType.kt new file mode 100644 index 00000000000..ed127b74913 --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/coercionToUnit/indirectCoercionWithExpectedType.kt @@ -0,0 +1,36 @@ +// !WITH_NEW_INFERENCE + +fun materialize(): T = TODO() + +fun a(): Unit = run { + run { + // Ok, block is coerced, because it has (indirectly) Unit-expected type + "hello" + } +} + +fun b(): Unit = run { + // Ok, expected type is applied + materialize() +} + +fun c(): Unit = run { + run { + // Attention! + // In OI expected type 'Unit' isn't applied here because of implementation quirks (note that OI still applies Unit in case 'e') + // In NI, it is applied and call is correctly inferred, which is consistent with the previous case + materialize() + } +} + +fun d(): Unit = run outer@{ + run inner@{ + return@inner materialize() + } +} + +fun e(): Unit = run outer@{ + run inner@{ + return@outer materialize() + } +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/inference/coercionToUnit/indirectCoercionWithExpectedType.txt b/compiler/testData/diagnostics/tests/inference/coercionToUnit/indirectCoercionWithExpectedType.txt new file mode 100644 index 00000000000..65d9b94bc9c --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/coercionToUnit/indirectCoercionWithExpectedType.txt @@ -0,0 +1,8 @@ +package + +public fun a(): kotlin.Unit +public fun b(): kotlin.Unit +public fun c(): kotlin.Unit +public fun d(): kotlin.Unit +public fun e(): kotlin.Unit +public fun materialize(): T diff --git a/compiler/testData/diagnostics/tests/inference/coercionToUnit/noCoercion.kt b/compiler/testData/diagnostics/tests/inference/coercionToUnit/noCoercion.kt new file mode 100644 index 00000000000..1bd52c393b5 --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/coercionToUnit/noCoercion.kt @@ -0,0 +1,26 @@ +// !WITH_NEW_INFERENCE +// !CHECK_TYPE + +fun noCoercionLastExpressionUsedAsReturnArgument() { + val a = { + 42 + } + + a checkType { _<() -> Int>() } +} + +fun noCoercionBlockHasExplicitType() { + val b: () -> Int = { + if (true) 42 + } +} + +fun noCoercionBlockHasExplicitReturn() { + val c = l@{ + if (true) return@l 42 + + if (true) 239 + } +} + +fun noCoercionInExpressionBody(): Unit = "hello" \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/inference/coercionToUnit/noCoercion.txt b/compiler/testData/diagnostics/tests/inference/coercionToUnit/noCoercion.txt new file mode 100644 index 00000000000..c55109201f4 --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/coercionToUnit/noCoercion.txt @@ -0,0 +1,6 @@ +package + +public fun noCoercionBlockHasExplicitReturn(): kotlin.Unit +public fun noCoercionBlockHasExplicitType(): kotlin.Unit +public fun noCoercionInExpressionBody(): kotlin.Unit +public fun noCoercionLastExpressionUsedAsReturnArgument(): kotlin.Unit diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index dfb8ece0903..23b359b46c7 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -10703,6 +10703,45 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { } } + @TestMetadata("compiler/testData/diagnostics/tests/inference/coercionToUnit") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class CoercionToUnit extends AbstractDiagnosticsTest { + public void testAllFilesPresentInCoercionToUnit() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/tests/inference/coercionToUnit"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); + } + + @TestMetadata("coercionWithExpectedType.kt") + public void testCoercionWithExpectedType() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedType.kt"); + doTest(fileName); + } + + @TestMetadata("coercionWithExpectedTypeAndBound.kt") + public void testCoercionWithExpectedTypeAndBound() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedTypeAndBound.kt"); + doTest(fileName); + } + + @TestMetadata("coercionWithoutExpectedType.kt") + public void testCoercionWithoutExpectedType() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithoutExpectedType.kt"); + doTest(fileName); + } + + @TestMetadata("indirectCoercionWithExpectedType.kt") + public void testIndirectCoercionWithExpectedType() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/inference/coercionToUnit/indirectCoercionWithExpectedType.kt"); + doTest(fileName); + } + + @TestMetadata("noCoercion.kt") + public void testNoCoercion() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/inference/coercionToUnit/noCoercion.kt"); + doTest(fileName); + } + } + @TestMetadata("compiler/testData/diagnostics/tests/inference/commonSystem") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java index ccb0516f3b8..1e6c16789e5 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java @@ -10703,6 +10703,45 @@ public class DiagnosticsUsingJavacTestGenerated extends AbstractDiagnosticsUsing } } + @TestMetadata("compiler/testData/diagnostics/tests/inference/coercionToUnit") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class CoercionToUnit extends AbstractDiagnosticsUsingJavacTest { + public void testAllFilesPresentInCoercionToUnit() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/tests/inference/coercionToUnit"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); + } + + @TestMetadata("coercionWithExpectedType.kt") + public void testCoercionWithExpectedType() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedType.kt"); + doTest(fileName); + } + + @TestMetadata("coercionWithExpectedTypeAndBound.kt") + public void testCoercionWithExpectedTypeAndBound() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithExpectedTypeAndBound.kt"); + doTest(fileName); + } + + @TestMetadata("coercionWithoutExpectedType.kt") + public void testCoercionWithoutExpectedType() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/inference/coercionToUnit/coercionWithoutExpectedType.kt"); + doTest(fileName); + } + + @TestMetadata("indirectCoercionWithExpectedType.kt") + public void testIndirectCoercionWithExpectedType() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/inference/coercionToUnit/indirectCoercionWithExpectedType.kt"); + doTest(fileName); + } + + @TestMetadata("noCoercion.kt") + public void testNoCoercion() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/inference/coercionToUnit/noCoercion.kt"); + doTest(fileName); + } + } + @TestMetadata("compiler/testData/diagnostics/tests/inference/commonSystem") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)