From 1a05df204e3a34495f7af831aeebec9fef05b90b Mon Sep 17 00:00:00 2001 From: Mikhael Bogdanov Date: Tue, 22 Aug 2017 12:20:33 +0200 Subject: [PATCH] Properly check expression nullability on attempt to apply == optimizations #Fix KT-19767 --- .../kotlin/codegen/ExpressionCodegen.java | 15 ++++++-- .../codegen/box/boxingOptimization/kt19767.kt | 21 ++++++++++++ .../box/boxingOptimization/kt19767_2.kt | 6 ++++ .../box/boxingOptimization/kt19767_3.kt | 34 +++++++++++++++++++ .../box/boxingOptimization/kt19767_chain.kt | 23 +++++++++++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 24 +++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 24 +++++++++++++ .../LightAnalysisModeTestGenerated.java | 24 +++++++++++++ .../semantics/JsCodegenBoxTestGenerated.java | 24 +++++++++++++ 9 files changed, 193 insertions(+), 2 deletions(-) create mode 100644 compiler/testData/codegen/box/boxingOptimization/kt19767.kt create mode 100644 compiler/testData/codegen/box/boxingOptimization/kt19767_2.kt create mode 100644 compiler/testData/codegen/box/boxingOptimization/kt19767_3.kt create mode 100644 compiler/testData/codegen/box/boxingOptimization/kt19767_chain.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java index 4a268507ec5..f8278471226 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java @@ -2926,11 +2926,13 @@ public class ExpressionCodegen extends KtVisitor impleme return genCmpWithZero(left, opToken, pregeneratedLeft); } - if (pregeneratedLeft == null && left instanceof KtSafeQualifiedExpression && isPrimitive(rightType)) { + if (pregeneratedLeft == null && left instanceof KtSafeQualifiedExpression && + isSelectorPureNonNullType((KtSafeQualifiedExpression) left) && isPrimitive(rightType)) { return genCmpSafeCallToPrimitive((KtSafeQualifiedExpression) left, right, rightType, opToken); } - if (isPrimitive(leftType) && right instanceof KtSafeQualifiedExpression) { + if (isPrimitive(leftType) && right instanceof KtSafeQualifiedExpression && + isSelectorPureNonNullType(((KtSafeQualifiedExpression) right))) { return genCmpPrimitiveToSafeCall(left, leftType, (KtSafeQualifiedExpression) right, opToken, pregeneratedLeft); } @@ -2979,6 +2981,15 @@ public class ExpressionCodegen extends KtVisitor impleme return genEqualsForExpressionsPreferIEEE754Arithmetic(left, right, opToken, leftType, rightType, pregeneratedLeft); } + private boolean isSelectorPureNonNullType(@NotNull KtSafeQualifiedExpression safeExpression) { + KtExpression expression = safeExpression.getSelectorExpression(); + if (expression == null) return false; + ResolvedCall resolvedCall = CallUtilKt.getResolvedCall(expression, bindingContext); + if (resolvedCall == null) return false; + KotlinType returnType = resolvedCall.getResultingDescriptor().getReturnType(); + return returnType != null && !TypeUtils.isNullableType(returnType); + } + private StackValue genCmpPrimitiveToSafeCall( @NotNull KtExpression left, @NotNull Type leftType, diff --git a/compiler/testData/codegen/box/boxingOptimization/kt19767.kt b/compiler/testData/codegen/box/boxingOptimization/kt19767.kt new file mode 100644 index 00000000000..bb00ac53fa6 --- /dev/null +++ b/compiler/testData/codegen/box/boxingOptimization/kt19767.kt @@ -0,0 +1,21 @@ +fun foo(p: Int?): Boolean { + return M(p)?.nulled() == 1 +} + +fun foo2(p: Int?): Boolean { + return 1 == M(p)?.nulled() +} + +class M(val z: T?) { + fun nulled(): T? = z +} + + +fun box(): String { + if (foo(null)) return "fail 1" + if (!foo(1)) return "fail 2" + + if (foo2(null)) return "fail 1" + if (!foo2(1)) return "fail 2" + return "OK" +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/boxingOptimization/kt19767_2.kt b/compiler/testData/codegen/box/boxingOptimization/kt19767_2.kt new file mode 100644 index 00000000000..8f0913523f9 --- /dev/null +++ b/compiler/testData/codegen/box/boxingOptimization/kt19767_2.kt @@ -0,0 +1,6 @@ +//WITH_RUNTIME + +fun box(): String { + val map: Map? = mapOf() + return if (map?.get("") == true) "fail" else "OK" +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/boxingOptimization/kt19767_3.kt b/compiler/testData/codegen/box/boxingOptimization/kt19767_3.kt new file mode 100644 index 00000000000..cf8f6d9063e --- /dev/null +++ b/compiler/testData/codegen/box/boxingOptimization/kt19767_3.kt @@ -0,0 +1,34 @@ +// IGNORE_BACKEND: JS, NATIVE + +// FILE: M.java + +public class M { + private final Integer value; + + public M(Integer value) { + this.value = value; + } + + public Integer nulled() { + return value; + } +} + + +// FILE: Kotlin.kt +fun foo(p: Int?): Boolean { + return M(p)?.nulled() == 1 +} + +fun foo2(p: Int?): Boolean { + return 1 == M(p)?.nulled() +} + +fun box(): String { + if (foo(null)) return "fail 1" + if (!foo(1)) return "fail 2" + + if (foo2(null)) return "fail 1" + if (!foo2(1)) return "fail 2" + return "OK" +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/boxingOptimization/kt19767_chain.kt b/compiler/testData/codegen/box/boxingOptimization/kt19767_chain.kt new file mode 100644 index 00000000000..441714d90b2 --- /dev/null +++ b/compiler/testData/codegen/box/boxingOptimization/kt19767_chain.kt @@ -0,0 +1,23 @@ +fun foo(p: Int?): Boolean { + return M(p)?.chain()?.nulled() == 1 +} + +fun foo2(p: Int?): Boolean { + return 1 == M(p)?.chain()?.nulled() +} + +class M(val z: T?) { + fun nulled(): T? = z + + fun chain(): M? = this +} + + +fun box(): String { + if (foo(null)) return "fail 1" + if (!foo(1)) return "fail 2" + + if (foo2(null)) return "fail 1" + if (!foo2(1)) return "fail 2" + return "OK" +} \ No newline at end of file diff --git a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 810442954fa..259ace5ab46 100644 --- a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -956,6 +956,30 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes doTest(fileName); } + @TestMetadata("kt19767.kt") + public void testKt19767() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt19767.kt"); + doTest(fileName); + } + + @TestMetadata("kt19767_2.kt") + public void testKt19767_2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt19767_2.kt"); + doTest(fileName); + } + + @TestMetadata("kt19767_3.kt") + public void testKt19767_3() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt19767_3.kt"); + doTest(fileName); + } + + @TestMetadata("kt19767_chain.kt") + public void testKt19767_chain() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt19767_chain.kt"); + doTest(fileName); + } + @TestMetadata("kt5493.kt") public void testKt5493() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt5493.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index ec6bd92cd46..07b5c25d63d 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -956,6 +956,30 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } + @TestMetadata("kt19767.kt") + public void testKt19767() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt19767.kt"); + doTest(fileName); + } + + @TestMetadata("kt19767_2.kt") + public void testKt19767_2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt19767_2.kt"); + doTest(fileName); + } + + @TestMetadata("kt19767_3.kt") + public void testKt19767_3() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt19767_3.kt"); + doTest(fileName); + } + + @TestMetadata("kt19767_chain.kt") + public void testKt19767_chain() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt19767_chain.kt"); + doTest(fileName); + } + @TestMetadata("kt5493.kt") public void testKt5493() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt5493.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index ba614909224..eae3a9185ba 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -956,6 +956,30 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes doTest(fileName); } + @TestMetadata("kt19767.kt") + public void testKt19767() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt19767.kt"); + doTest(fileName); + } + + @TestMetadata("kt19767_2.kt") + public void testKt19767_2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt19767_2.kt"); + doTest(fileName); + } + + @TestMetadata("kt19767_3.kt") + public void testKt19767_3() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt19767_3.kt"); + doTest(fileName); + } + + @TestMetadata("kt19767_chain.kt") + public void testKt19767_chain() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt19767_chain.kt"); + doTest(fileName); + } + @TestMetadata("kt5493.kt") public void testKt5493() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt5493.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java index 714548953aa..a5db30ec6ea 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java @@ -1154,6 +1154,30 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { doTest(fileName); } + @TestMetadata("kt19767.kt") + public void testKt19767() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt19767.kt"); + doTest(fileName); + } + + @TestMetadata("kt19767_2.kt") + public void testKt19767_2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt19767_2.kt"); + doTest(fileName); + } + + @TestMetadata("kt19767_3.kt") + public void testKt19767_3() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt19767_3.kt"); + doTest(fileName); + } + + @TestMetadata("kt19767_chain.kt") + public void testKt19767_chain() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt19767_chain.kt"); + doTest(fileName); + } + @TestMetadata("kt5493.kt") public void testKt5493() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/boxingOptimization/kt5493.kt");