From f485413cfd27a18614b1585c51db6a0c8dac8448 Mon Sep 17 00:00:00 2001 From: pyos Date: Mon, 14 Nov 2022 17:18:01 +0100 Subject: [PATCH] FIR DFA: x !is T? => x != null ^KT-22996 tag fixed-in-k2 --- .../fir/resolve/dfa/FirDataFlowAnalyzer.kt | 11 ++-- .../kotlin/fir/resolve/dfa/contracts.kt | 8 ++- .../diagnostics/tests/infos/SmartCasts.fir.kt | 6 +- .../ir/irText/expressions/bangbang.fir.ir.txt | 2 +- .../ir/irText/expressions/bangbang.fir.kt.txt | 2 +- .../when-expression/p-6/pos/1.3.fir.kt | 65 +++++++++++++++++++ .../when-expression/p-6/pos/1.3.kt | 1 - .../when-expression/p-6/pos/1.4.fir.kt | 2 +- .../contracts/analysis/common/neg/1.fir.kt | 6 +- .../analysis/smartcasts/neg/7.fir.kt | 6 +- .../analysis/smartcasts/neg/8.fir.kt | 8 +-- .../diagnostics/notLinked/dfa/neg/41.fir.kt | 12 ++-- .../diagnostics/notLinked/dfa/neg/45.fir.kt | 4 +- .../diagnostics/notLinked/dfa/neg/6.fir.kt | 14 ++-- .../diagnostics/notLinked/dfa/pos/22.fir.kt | 12 ++-- .../diagnostics/notLinked/dfa/pos/5.fir.kt | 4 +- .../diagnostics/notLinked/dfa/pos/6.fir.kt | 4 +- .../diagnostics/notLinked/dfa/pos/68.fir.kt | 4 +- 18 files changed, 118 insertions(+), 53 deletions(-) create mode 100644 compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-6/pos/1.3.fir.kt diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt index 4a39bf2311d..cb8f0282897 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt @@ -401,10 +401,10 @@ abstract class FirDataFlowAnalyzer( if (!type.canBeNull) { // x is (T & Any) => x != null flow.addImplication((expressionVariable eq isType) implies (operandVariable notEq null)) - } else { - // TODO? (KT-22996) x !is T? => x != null; don't forget to change `approveContractStatement` - // flow.addImplication((expressionVariable eq !isType) implies (operandVariable notEq null)) - } + } else if (type.isMarkedNullable) { + // x !is T? => x != null + flow.addImplication((expressionVariable eq !isType) implies (operandVariable notEq null)) + } // else probably a type parameter, so which implication is correct depends on instantiation } } } @@ -418,8 +418,7 @@ abstract class FirDataFlowAnalyzer( } else { val expressionVariable = variableStorage.createSynthetic(typeOperatorCall) flow.addImplication((expressionVariable notEq null) implies (operandVariable notEq null)) - // TODO? (x as T?) == null => x == null - // flow.addImplication((expressionVariable eq null) implies (operandVariable eq null)) + flow.addImplication((expressionVariable eq null) implies (operandVariable eq null)) } } diff --git a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/contracts.kt b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/contracts.kt index a1f9d6fff13..65bc31bef0b 100644 --- a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/contracts.kt +++ b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/contracts.kt @@ -45,9 +45,11 @@ fun LogicSystem.approveContractStatement( substitutedType.isAny -> it.processEqNull(!isType) substitutedType.isNullableNothing -> it.processEqNull(isType) else -> { - // x is (T & Any) => x != null - // TODO? (KT-22996) x !is T? => x != null: change `&&` to `==` - val fromNullability = if (isType && !type.canBeNull) it.processEqNull(false) else mapOf() + // x is (T & Any) || x !is T? => x != null + val fromNullability = if ((isType && !type.canBeNull) || (!isType && type.isMarkedNullable)) + it.processEqNull(false) + else + mapOf() if (isType && it is RealVariable) { andForTypeStatements(fromNullability, mapOf(it to (it typeEq substitutedType))) } else { diff --git a/compiler/testData/diagnostics/tests/infos/SmartCasts.fir.kt b/compiler/testData/diagnostics/tests/infos/SmartCasts.fir.kt index b32738470a8..2325155926e 100644 --- a/compiler/testData/diagnostics/tests/infos/SmartCasts.fir.kt +++ b/compiler/testData/diagnostics/tests/infos/SmartCasts.fir.kt @@ -58,7 +58,7 @@ fun f11(a : A?) { is A -> a.foo() is Any -> a.foo() is Any? -> a.bar() - else -> a?.foo() + else -> a?.foo() } } @@ -68,8 +68,8 @@ fun f12(a : A?) { is A -> a.foo() is Any -> a.foo(); is Any? -> a.bar() - is C -> a.bar() - else -> a?.foo() + is C -> a.bar() + else -> a?.foo() } if (a is Any?) { diff --git a/compiler/testData/ir/irText/expressions/bangbang.fir.ir.txt b/compiler/testData/ir/irText/expressions/bangbang.fir.ir.txt index 2eefcf73e51..5956c243297 100644 --- a/compiler/testData/ir/irText/expressions/bangbang.fir.ir.txt +++ b/compiler/testData/ir/irText/expressions/bangbang.fir.ir.txt @@ -56,5 +56,5 @@ FILE fqName: fileName:/bangbang.kt then: CALL 'public final fun useString (s: kotlin.String): kotlin.Unit declared in ' type=kotlin.Unit origin=null s: CALL 'public final fun CHECK_NOT_NULL (arg0: T0 of kotlin.internal.ir.CHECK_NOT_NULL?): {T0 of kotlin.internal.ir.CHECK_NOT_NULL & Any} declared in kotlin.internal.ir' type=kotlin.String origin=EXCLEXCL : kotlin.String - arg0: TYPE_OP type=kotlin.String? origin=IMPLICIT_CAST typeOperand=kotlin.String? + arg0: TYPE_OP type=kotlin.String origin=IMPLICIT_CAST typeOperand=kotlin.String GET_VAR 'a: X of .test4 declared in .test4' type=X of .test4 origin=null diff --git a/compiler/testData/ir/irText/expressions/bangbang.fir.kt.txt b/compiler/testData/ir/irText/expressions/bangbang.fir.kt.txt index e484c8e9ccd..242cc56348b 100644 --- a/compiler/testData/ir/irText/expressions/bangbang.fir.kt.txt +++ b/compiler/testData/ir/irText/expressions/bangbang.fir.kt.txt @@ -24,6 +24,6 @@ fun test4(a: X) { a is String? -> CHECK_NOT_NULL(arg0 = a /*as String? */) /*~> Unit */ } when { - a is String? -> useString(s = CHECK_NOT_NULL(arg0 = a /*as String? */)) + a is String? -> useString(s = CHECK_NOT_NULL(arg0 = a /*as String */)) } } diff --git a/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-6/pos/1.3.fir.kt b/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-6/pos/1.3.fir.kt new file mode 100644 index 00000000000..a4137b9fdd5 --- /dev/null +++ b/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-6/pos/1.3.fir.kt @@ -0,0 +1,65 @@ +// SKIP_TXT + +// TESTCASE NUMBER: 1 +fun case_1(value_1: Any) = when (value_1) { + is Int -> {} + is Float, is Char, is Boolean -> {} + is String -> {} + else -> {} +} + +// TESTCASE NUMBER: 2 +fun case_2(value_1: Any?) = when (value_1) { + is Float, is Char, is SealedClass? -> "" // if value is null then this branch will be executed + is Double, is Boolean, is ClassWithCompanionObject.Companion -> "" + else -> "" +} + +/* + * TESTCASE NUMBER: 3 + * UNEXPECTED BEHAVIOUR + * ISSUES: KT-22996 + */ +fun case_3(value_1: Any?) = when (value_1) { + is Float, is Char, is Int? -> "" // if value is null then this branch will be executed + is SealedChild2, is Boolean?, is String -> "" // redundant nullable type check + else -> "" +} + +/* + * TESTCASE NUMBER: 4 + * UNEXPECTED BEHAVIOUR + * ISSUES: KT-22996 + */ +fun case_4(value_1: Any?) = when (value_1) { + is Float, is Char?, is Int? -> "" // double nullable type check in the one branch + is SealedChild1, is Boolean, is String -> "" + else -> "" +} + +/* + * TESTCASE NUMBER: 5 + * UNEXPECTED BEHAVIOUR + * ISSUES: KT-22996 + */ +fun case_5(value_1: Any?): String { + when (value_1) { + is Float, is Char?, is Int -> return "" + is Double, is EmptyObject, is String -> return "" + null -> return "" // null-check redundant + else -> return "" + } +} + +/* + * TESTCASE NUMBER: 6 + * UNEXPECTED BEHAVIOUR + * ISSUES: KT-22996 + */ +fun case_6(value_1: Any?): String { + when (value_1) { + is Float, is Char?, null, is Int -> return "" // double nullable type check in the one branch + is Double, is EmptyObject, is String -> return "" + else -> return "" + } +} diff --git a/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-6/pos/1.3.kt b/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-6/pos/1.3.kt index 05be0d2e097..8420af56f3f 100644 --- a/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-6/pos/1.3.kt +++ b/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-6/pos/1.3.kt @@ -1,4 +1,3 @@ -// FIR_IDENTICAL // SKIP_TXT /* diff --git a/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-6/pos/1.4.fir.kt b/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-6/pos/1.4.fir.kt index 3b9c1973a4e..cea1d73a122 100644 --- a/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-6/pos/1.4.fir.kt +++ b/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-6/pos/1.4.fir.kt @@ -44,6 +44,6 @@ fun case_6(value_1: Any?): String = when (value_1) { * ISSUES: KT-22996 */ fun case_7(value_1: Any?): String = when (value_1) { - is Number?, null, !is SealedChild3 -> "" // triple nullable type check in the one branch + is Number?, null, !is SealedChild3 -> "" // triple nullable type check in the one branch else -> "" } diff --git a/compiler/tests-spec/testData/diagnostics/notLinked/contracts/analysis/common/neg/1.fir.kt b/compiler/tests-spec/testData/diagnostics/notLinked/contracts/analysis/common/neg/1.fir.kt index 26650b4bd44..52e388d5187 100644 --- a/compiler/tests-spec/testData/diagnostics/notLinked/contracts/analysis/common/neg/1.fir.kt +++ b/compiler/tests-spec/testData/diagnostics/notLinked/contracts/analysis/common/neg/1.fir.kt @@ -20,7 +20,7 @@ inline fun case_1(value_1: Int?, block: () -> Unit): Boolean { // TESTCASE NUMBER: 2 inline fun T?.case_2(value_1: Int?, value_2: Any?, block: () -> Unit): Boolean? { - contract { + contract { callsInPlace(block, InvocationKind.AT_MOST_ONCE) returns(true) implies (value_1 == null && this@case_2 == null && value_2 !is Boolean?) returns(false) implies (value_2 is Boolean?) @@ -55,7 +55,7 @@ fun case_2(value_1: Int?, value_2: Int?, value_3: Any?) { val value_4: Int when (value_1.case_2(value_2, value_3) { value_4 = 10 }) { true -> { - println(value_3?.xor(true)) + println(value_3?.xor(true)) println(value_4) println(value_1.inv()) println(value_2.inv()) @@ -66,7 +66,7 @@ fun case_2(value_1: Int?, value_2: Int?, value_3: Any?) { println(value_2) } null -> { - println(value_3?.xor(true)) + println(value_3?.xor(true)) println(value_4) println(value_1) println(value_2) diff --git a/compiler/tests-spec/testData/diagnostics/notLinked/contracts/analysis/smartcasts/neg/7.fir.kt b/compiler/tests-spec/testData/diagnostics/notLinked/contracts/analysis/smartcasts/neg/7.fir.kt index b55edf20020..c76663c1b43 100644 --- a/compiler/tests-spec/testData/diagnostics/notLinked/contracts/analysis/smartcasts/neg/7.fir.kt +++ b/compiler/tests-spec/testData/diagnostics/notLinked/contracts/analysis/smartcasts/neg/7.fir.kt @@ -235,7 +235,7 @@ fun case_3(value_1: Any?) { // TESTCASE NUMBER: 4 fun case_4(value_1: Any?) { case_4_1(value_1) - value_1?.toByte() + value_1?.toByte() case_4_2(value_1) value_1.toByte() case_4_3(value_1) @@ -329,14 +329,14 @@ fun case_7(value_1: Any?) { // TESTCASE NUMBER: 8 fun case_8(value_1: Any?) { if (case_8_1(value_1)) { - value_1?.toByte() + value_1?.toByte() if (case_8_2(value_1)) { value_1.toByte() if (case_8_3(value_1)) value_1.inv() } } if (!case_8_4(value_1)) { - value_1?.toByte() + value_1?.toByte() if (!case_8_5(value_1)) { value_1.toByte() if (!case_8_6(value_1)) value_1.inv() diff --git a/compiler/tests-spec/testData/diagnostics/notLinked/contracts/analysis/smartcasts/neg/8.fir.kt b/compiler/tests-spec/testData/diagnostics/notLinked/contracts/analysis/smartcasts/neg/8.fir.kt index 2a9172c6d1f..bb61a0fac14 100644 --- a/compiler/tests-spec/testData/diagnostics/notLinked/contracts/analysis/smartcasts/neg/8.fir.kt +++ b/compiler/tests-spec/testData/diagnostics/notLinked/contracts/analysis/smartcasts/neg/8.fir.kt @@ -58,7 +58,7 @@ import contracts.* // TESTCASE NUMBER: 1 fun case_1(value_1: Any?) { funWithReturns(value_1 !is Number?) - println(value_1?.toByte()) + println(value_1?.toByte()) if (funWithReturnsTrue(value_1 !is Number)) { println(value_1.toByte()) if (funWithReturnsNotNull(value_1 is Int) == null) println(value_1.inv()) @@ -68,7 +68,7 @@ fun case_1(value_1: Any?) { // TESTCASE NUMBER: 2 fun case_2(value_1: Any?) { if (!funWithReturnsFalse(value_1 !is Number?)) { - println(value_1?.toByte()) + println(value_1?.toByte()) funWithReturns(value_1 !is Number) println(value_1.toByte()) if (funWithReturnsNull(value_1 !is Int) == null) println(value_1.inv()) @@ -78,12 +78,12 @@ fun case_2(value_1: Any?) { // TESTCASE NUMBER: 3 fun case_3(value_1: Int?, value_2: Any?) { if (!value_1.case_3(value_1, value_2 is Number?)) { - println(value_2?.toByte()) + println(value_2?.toByte()) println(value_1) } else if (value_1.case_3(value_1, value_2 is Number?)) { println(value_2?.toByte()) } else { - println(value_2?.toByte()) + println(value_2?.toByte()) } } diff --git a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/neg/41.fir.kt b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/neg/41.fir.kt index 5031459d00b..5e64af57716 100644 --- a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/neg/41.fir.kt +++ b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/neg/41.fir.kt @@ -36,12 +36,12 @@ inline fun case_3(x: Any?) { when (x) { is T? -> return else -> { - x - x.equals(10) + x + x.equals(10) } } - x - x.equals(10) + x + x.equals(10) } // TESTCASE NUMBER: 4 @@ -70,8 +70,8 @@ inline fun case_5(x: Any?) { // TESTCASE NUMBER: 6 inline fun case_6(x: Any?) { if (x is T?) return - x - x.equals(10) + x + x.equals(10) } // TESTCASE NUMBER: 7 diff --git a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/neg/45.fir.kt b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/neg/45.fir.kt index afdac810dfc..f776ca252e2 100644 --- a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/neg/45.fir.kt +++ b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/neg/45.fir.kt @@ -7,8 +7,8 @@ */ fun case_1(x: Number?): Long? { if (x is Long?) return x - x - return x.toLong() + x + return x.toLong() } /* diff --git a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/neg/6.fir.kt b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/neg/6.fir.kt index fc1cd1cbb32..374ffce1a5c 100644 --- a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/neg/6.fir.kt +++ b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/neg/6.fir.kt @@ -38,8 +38,8 @@ fun case_4(x: Any) { fun case_5(x: Any?) { if (!(x !is Class.NestedClass?) || x is Class.NestedClass? || x !is Class.NestedClass?) { if (!!(x !is Class.NestedClass?)) { - x - x.prop_4 + x + x.prop_4 } } } @@ -72,8 +72,8 @@ fun case_8(x: Any?) { fun case_9(x: Any?) { if (!!!(x !is TypealiasNullableStringIndirect?)) else { if (!(x !is TypealiasNullableStringIndirect?)) else { - x - x?.get(0) + x + x?.get(0) } } } @@ -93,9 +93,9 @@ fun case_10(x: Any?) { fun case_11(x: Any?) { if (x is SealedMixedChildObject1?) else { if (x is SealedMixedChildObject1?) else { - x - x?.prop_1 - x?.prop_2 + x + x?.prop_1 + x?.prop_2 } } } diff --git a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/22.fir.kt b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/22.fir.kt index a6e80d8b9ce..ffa59a58faf 100644 --- a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/22.fir.kt +++ b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/22.fir.kt @@ -46,8 +46,8 @@ fun case_4(x: Any) { fun case_5(x: Any?) { if (!(x !is Class.NestedClass?)) { if (!!(x !is Class.NestedClass?)) { - x - x?.prop_4 + x + x?.prop_4 } } } @@ -76,8 +76,8 @@ fun case_7(x: Any) { fun case_8(x: Any?) { if (!!!!(x is Int?)) else { if (!(x is Int?)) else { - x - x?.inv() + x + x?.inv() } } } @@ -86,8 +86,8 @@ fun case_8(x: Any?) { fun case_9(x: Any?) { if (!!!(x !is TypealiasNullableStringIndirect?)) else { if (!!(x !is TypealiasNullableStringIndirect?)) else { - x - x?.get(0) + x + x?.get(0) } } } diff --git a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/5.fir.kt b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/5.fir.kt index 03af1ff8858..4714ba01bc3 100644 --- a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/5.fir.kt +++ b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/5.fir.kt @@ -207,7 +207,7 @@ fun case_10(x: Any?) { if (x is ClassLevel1? && x is ClassLevel2? && x is ClassLevel3?) { if (x is ClassLevel4?) { - } else if (x is ClassLevel5? && x != null) { + } else if (x is ClassLevel5? && x != null) { x x.equals(null) x.propT @@ -224,7 +224,7 @@ fun case_10(x: Any?) { x.test4() x.test5() } - } else if (x is ClassLevel4? && x != null && x is ClassLevel5?) { + } else if (x is ClassLevel4? && x != null && x is ClassLevel5?) { x x.equals(null) x.propT diff --git a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/6.fir.kt b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/6.fir.kt index f37dde6069f..abb86aa7a81 100644 --- a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/6.fir.kt +++ b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/6.fir.kt @@ -1255,7 +1255,7 @@ fun case_70(x: Any?) { if (x is ClassLevel1? && x is ClassLevel2? && x is ClassLevel3?) { if (x is ClassLevel4?) { - } else if (x is ClassLevel5? && x != nullableNothingProperty || x != implicitNullableNothingProperty) { + } else if (x is ClassLevel5? && x != nullableNothingProperty || x != implicitNullableNothingProperty) { x x.equals(null) x.propT @@ -1267,7 +1267,7 @@ fun case_70(x: Any?) { x.funNullableT() x.funNullableAny() } - } else if (x is ClassLevel4? && x !== nullableNothingProperty && x is ClassLevel5?) { + } else if (x is ClassLevel4? && x !== nullableNothingProperty && x is ClassLevel5?) { x x.equals(null) x.propT diff --git a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/68.fir.kt b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/68.fir.kt index 050d1cbaad3..c30e110293a 100644 --- a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/68.fir.kt +++ b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/68.fir.kt @@ -59,7 +59,7 @@ fun case_7(x: Any?) { // TESTCASE NUMBER: 8 fun case_8(x: Any?) { if (x as String? == null) { - x - x.length + x + x.length } }