From cff88a3f8b0fe7207fa0aaf770c4bdb6e441269f Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Mon, 12 Mar 2018 14:57:16 +0300 Subject: [PATCH] Fix false positive unreachable code in case of Nothing!-typed calls Note, that this change potentially has some other effects in corner cases (like the changed test data that is rather sensible because `bar` in the example is not effectively projected out and can be called with nulls) Probably, we need to consider rewriting all other isSomeType methods in KotlinBuiltins, but now it seems to be a rather dangerous change #KT-16424 Fixed --- .../tests/generics/starProjections/invalid.kt | 3 ++- .../diagnostics/tests/j+k/flexibleNothing.kt | 17 +++++++++++++++++ .../diagnostics/tests/j+k/flexibleNothing.txt | 11 +++++++++++ .../checkers/DiagnosticsTestGenerated.java | 6 ++++++ .../DiagnosticsUsingJavacTestGenerated.java | 6 ++++++ .../kotlin/builtins/KotlinBuiltIns.java | 4 ++-- 6 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/j+k/flexibleNothing.kt create mode 100644 compiler/testData/diagnostics/tests/j+k/flexibleNothing.txt diff --git a/compiler/testData/diagnostics/tests/generics/starProjections/invalid.kt b/compiler/testData/diagnostics/tests/generics/starProjections/invalid.kt index cf29f51d3a9..55910af194f 100644 --- a/compiler/testData/diagnostics/tests/generics/starProjections/invalid.kt +++ b/compiler/testData/diagnostics/tests/generics/starProjections/invalid.kt @@ -17,7 +17,8 @@ fun main(a: A<*>, j: JavaClass<*>, i2: Inv2<*>) { j.foo() checkType { _() } i2.x checkType { _() } - j.bar(1, 2, Any()) + j.bar(1, 2, Any()) + j.bar(null) } // FILE: JavaClass.java diff --git a/compiler/testData/diagnostics/tests/j+k/flexibleNothing.kt b/compiler/testData/diagnostics/tests/j+k/flexibleNothing.kt new file mode 100644 index 00000000000..f5596d04f2e --- /dev/null +++ b/compiler/testData/diagnostics/tests/j+k/flexibleNothing.kt @@ -0,0 +1,17 @@ +// FILE: TestClass.java +import org.jetbrains.annotations.Nullable; +public class TestClass { + public T set(@Nullable String key, @Nullable T t) { + return t; + } +} + +// FILE: main.kt +fun run() { + val testClass = TestClass() + // inferred as `set()`, return type is Nothing! + testClass.set("test", null) + + // Should not be unreachable + run() +} diff --git a/compiler/testData/diagnostics/tests/j+k/flexibleNothing.txt b/compiler/testData/diagnostics/tests/j+k/flexibleNothing.txt new file mode 100644 index 00000000000..c387c1715fd --- /dev/null +++ b/compiler/testData/diagnostics/tests/j+k/flexibleNothing.txt @@ -0,0 +1,11 @@ +package + +public fun run(): kotlin.Unit + +public open class TestClass { + public constructor TestClass() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open operator fun set(/*0*/ @org.jetbrains.annotations.Nullable key: kotlin.String?, /*1*/ @org.jetbrains.annotations.Nullable t: T?): T! + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 7cf880c9895..b4ac5a32e40 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -12578,6 +12578,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("flexibleNothing.kt") + public void testFlexibleNothing() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/j+k/flexibleNothing.kt"); + doTest(fileName); + } + @TestMetadata("genericConstructorWithMultipleBounds.kt") public void testGenericConstructorWithMultipleBounds() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/j+k/genericConstructorWithMultipleBounds.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java index 7ee70dc9738..0834d174a21 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java @@ -12578,6 +12578,12 @@ public class DiagnosticsUsingJavacTestGenerated extends AbstractDiagnosticsUsing doTest(fileName); } + @TestMetadata("flexibleNothing.kt") + public void testFlexibleNothing() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/j+k/flexibleNothing.kt"); + doTest(fileName); + } + @TestMetadata("genericConstructorWithMultipleBounds.kt") public void testGenericConstructorWithMultipleBounds() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/j+k/genericConstructorWithMultipleBounds.kt"); diff --git a/core/descriptors/src/org/jetbrains/kotlin/builtins/KotlinBuiltIns.java b/core/descriptors/src/org/jetbrains/kotlin/builtins/KotlinBuiltIns.java index 1b934443a33..ba388478f59 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/builtins/KotlinBuiltIns.java +++ b/core/descriptors/src/org/jetbrains/kotlin/builtins/KotlinBuiltIns.java @@ -976,12 +976,12 @@ public abstract class KotlinBuiltIns { public static boolean isNothing(@NotNull KotlinType type) { return isNothingOrNullableNothing(type) - && !type.isMarkedNullable(); + && !TypeUtils.isNullableType(type); } public static boolean isNullableNothing(@NotNull KotlinType type) { return isNothingOrNullableNothing(type) - && type.isMarkedNullable(); + && TypeUtils.isNullableType(type); } public static boolean isNothingOrNullableNothing(@NotNull KotlinType type) {