From e7d090997918f9e65c21352fba667adcc0b750c0 Mon Sep 17 00:00:00 2001 From: Kristoffer Andersen Date: Thu, 5 Sep 2019 11:25:17 +0200 Subject: [PATCH] JVM_IR: Fix null-constant comparison with primitive types. --- .../lower/JvmBuiltinOptimizationLowering.kt | 109 +++++++++++------- .../equalityWithObject/intEqualsNull.kt | 2 + .../conditions/nullCompareConst.kt | 20 ++++ .../codegen/BlackBoxCodegenTestGenerated.java | 5 + .../codegen/BytecodeTextTestGenerated.java | 5 + .../LightAnalysisModeTestGenerated.java | 5 + .../ir/IrBlackBoxCodegenTestGenerated.java | 5 + .../ir/IrBytecodeTextTestGenerated.java | 5 + .../IrJsCodegenBoxTestGenerated.java | 5 + .../semantics/JsCodegenBoxTestGenerated.java | 5 + 10 files changed, 122 insertions(+), 44 deletions(-) create mode 100644 compiler/testData/codegen/box/primitiveTypes/equalityWithObject/intEqualsNull.kt create mode 100644 compiler/testData/codegen/bytecodeText/conditions/nullCompareConst.kt diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmBuiltinOptimizationLowering.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmBuiltinOptimizationLowering.kt index 8ccd0caecc9..3536ff743d9 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmBuiltinOptimizationLowering.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmBuiltinOptimizationLowering.kt @@ -13,12 +13,14 @@ import org.jetbrains.kotlin.codegen.intrinsics.Not import org.jetbrains.kotlin.ir.IrStatement import org.jetbrains.kotlin.ir.declarations.IrDeclarationOrigin import org.jetbrains.kotlin.ir.declarations.IrFile +import org.jetbrains.kotlin.ir.declarations.IrFunction import org.jetbrains.kotlin.ir.declarations.IrVariable import org.jetbrains.kotlin.ir.expressions.* import org.jetbrains.kotlin.ir.expressions.impl.IrBlockImpl import org.jetbrains.kotlin.ir.expressions.impl.IrCallImpl import org.jetbrains.kotlin.ir.expressions.impl.IrConstImpl import org.jetbrains.kotlin.ir.types.isBoolean +import org.jetbrains.kotlin.ir.types.isNullableAny import org.jetbrains.kotlin.ir.types.isPrimitiveType import org.jetbrains.kotlin.ir.types.toKotlinType import org.jetbrains.kotlin.ir.util.* @@ -43,56 +45,71 @@ class JvmBuiltinOptimizationLowering(val context: JvmBackendContext) : FileLower return expression.type.isPrimitiveType() && (expression is IrConst<*> || expression is IrGetValue) } - private fun isNullCheckOfPrimitiveTypeValue(call: IrCall, context: JvmBackendContext): Boolean { - if (call.symbol == context.irBuiltIns.eqeqSymbol) { - val left = call.getValueArgument(0)!! - val right = call.getValueArgument(1)!! - // When used for null checks, it is safe to eliminate constants and local variable loads. - // Even if a local variable of simple type is updated via the debugger it still cannot - // be null. - return (right.isNullConst() && left.type.unboxInlineClass().isPrimitiveType()) - || (left.isNullConst() && right.type.unboxInlineClass().isPrimitiveType()) - } - return false - } + private val IrFunction.isObjectEquals + get() = name.asString() == "equals" && + valueParameters.count() == 1 && + valueParameters[0].type.isNullableAny() && + extensionReceiverParameter == null && + dispatchReceiverParameter != null - private fun isNullCheckOfConstant(call: IrCall, context: JvmBackendContext): Boolean { + + private fun getOperandsIfCallToEqeqOrEquals(call: IrCall): Pair? { if (call.symbol == context.irBuiltIns.eqeqSymbol) { val left = call.getValueArgument(0)!! val right = call.getValueArgument(1)!! - return (right.isNullConst() && left is IrConst<*>) - || (left.isNullConst() && right is IrConst<*>) + return left to right + } else if (call.symbol.owner.isObjectEquals) { + val left = call.dispatchReceiver!! + val right = call.getValueArgument(0)!! + return left to right + } else { + return null; } - return false } override fun lower(irFile: IrFile) { irFile.transformChildrenVoid(object : IrElementTransformerVoid() { override fun visitCall(expression: IrCall): IrExpression { expression.transformChildrenVoid(this) - return if (isNegation(expression, context) && isNegation(expression.dispatchReceiver!!, context)) { - (expression.dispatchReceiver as IrCall).dispatchReceiver!! - } else if (isNullCheckOfPrimitiveTypeValue(expression, context)) { - val left = expression.getValueArgument(0)!! - val nonNullArgument = if (left.isNullConst()) expression.getValueArgument(1)!! else left - val constFalse = IrConstImpl.constFalse(expression.startOffset, expression.endOffset, context.irBuiltIns.booleanType) - if (hasNoSideEffectsForNullCompare(nonNullArgument)) { - constFalse - } else { - IrBlockImpl(expression.startOffset, expression.endOffset, expression.type, expression.origin).apply { - statements.add(nonNullArgument.coerceToUnitIfNeeded(nonNullArgument.type.toKotlinType(), context.irBuiltIns)) - statements.add(constFalse) - } - } - } else if (isNullCheckOfConstant(expression, context)) { - if (expression.getValueArgument(0)!!.isNullConst() && expression.getValueArgument(1)!!.isNullConst()) { - IrConstImpl.constTrue(expression.startOffset, expression.endOffset, context.irBuiltIns.booleanType) - } else { - IrConstImpl.constFalse(expression.startOffset, expression.endOffset, context.irBuiltIns.booleanType) - } - } else { - expression + + if (isNegation(expression, context) && isNegation(expression.dispatchReceiver!!, context)) { + return (expression.dispatchReceiver as IrCall).dispatchReceiver!! } + + getOperandsIfCallToEqeqOrEquals(expression)?.let { (left, right) -> + val constFalse = + IrConstImpl.constFalse(expression.startOffset, expression.endOffset, context.irBuiltIns.booleanType) + + return when { + left.isNullConst() && right.isNullConst() -> + IrConstImpl.constTrue(expression.startOffset, expression.endOffset, context.irBuiltIns.booleanType) + + left.isNullConst() && right is IrConst<*> || right.isNullConst() && left is IrConst<*> -> + constFalse + + right.isNullConst() && left.type.unboxInlineClass().isPrimitiveType() || + left.isNullConst() && right.type.unboxInlineClass().isPrimitiveType() -> { + val nonNullArgument = if (left.isNullConst()) right else left + if (hasNoSideEffectsForNullCompare(nonNullArgument)) { + constFalse + } else { + IrBlockImpl(expression.startOffset, expression.endOffset, expression.type, expression.origin).apply { + statements.add( + nonNullArgument.coerceToUnitIfNeeded( + nonNullArgument.type.toKotlinType(), + context.irBuiltIns + ) + ) + statements.add(constFalse) + } + } + } + + else -> expression + } + } + + return expression } override fun visitWhen(expression: IrWhen): IrExpression { @@ -103,10 +120,12 @@ class JvmBuiltinOptimizationLowering(val context: JvmBackendContext) : FileLower it.condition.isFalseConst() && isCompilerGenerated } if (expression.origin == IrStatementOrigin.ANDAND) { - assert(expression.type.isBoolean() - && expression.branches.size == 2 - && expression.branches[1].condition.isTrueConst() - && expression.branches[1].result.isFalseConst()) { + assert( + expression.type.isBoolean() + && expression.branches.size == 2 + && expression.branches[1].condition.isTrueConst() + && expression.branches[1].result.isFalseConst() + ) { "ANDAND condition should have an 'if true then false' body on its second branch. " + "Failing expression: ${expression.dump()}" } @@ -126,7 +145,8 @@ class JvmBuiltinOptimizationLowering(val context: JvmBackendContext) : FileLower expression.type.isBoolean() && expression.branches.size == 2 && expression.branches[0].result.isTrueConst() - && expression.branches[1].condition.isTrueConst()) { + && expression.branches[1].condition.isTrueConst() + ) { "OROR condition should have an 'if a then true' body on its first branch, " + "and an 'if true then b' body on its second branch. " + "Failing expression: ${expression.dump()}" @@ -228,7 +248,8 @@ class JvmBuiltinOptimizationLowering(val context: JvmBackendContext) : FileLower if (first is IrVariable && first.origin == IrDeclarationOrigin.IR_TEMPORARY_VARIABLE && second is IrGetValue - && first.symbol == second.symbol) { + && first.symbol == second.symbol + ) { expression.statements.clear() first.initializer?.let { expression.statements.add(it) } } diff --git a/compiler/testData/codegen/box/primitiveTypes/equalityWithObject/intEqualsNull.kt b/compiler/testData/codegen/box/primitiveTypes/equalityWithObject/intEqualsNull.kt new file mode 100644 index 00000000000..5d2e0061c75 --- /dev/null +++ b/compiler/testData/codegen/box/primitiveTypes/equalityWithObject/intEqualsNull.kt @@ -0,0 +1,2 @@ +fun box(): String = + if (1.equals(null)) "FAIL" else "OK" \ No newline at end of file diff --git a/compiler/testData/codegen/bytecodeText/conditions/nullCompareConst.kt b/compiler/testData/codegen/bytecodeText/conditions/nullCompareConst.kt new file mode 100644 index 00000000000..ccfd804d961 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/conditions/nullCompareConst.kt @@ -0,0 +1,20 @@ +fun f(): Boolean = "non-primitive" == null +fun g(): Boolean = null == "non-primitive" +fun h(): Boolean = "non-primitive".equals(null) +//fun i(): Boolean = null.equals("non-primitive") +//See KT-33757 + +// JVM does not optimize h() to constant false + +// 0 IF + +// JVM_TEMPLATES +// 2 ICONST_0 +// 1 ACONST_NULL +// 1 INVOKEVIRTUAL + +// JVM_IR_TEMPLATES +// 0 ACONST_NULL +// 0 INVOKESTATIC +// 0 INVOKEVIRTUAL +// 3 ICONST_0 \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 2206737c928..eece3b49929 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -17830,6 +17830,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/primitiveTypes/equalityWithObject/boxedLongEqualsLong.kt"); } + @TestMetadata("intEqualsNull.kt") + public void testIntEqualsNull() throws Exception { + runTest("compiler/testData/codegen/box/primitiveTypes/equalityWithObject/intEqualsNull.kt"); + } + @TestMetadata("intEqualsNullableInt.kt") public void testIntEqualsNullableInt() throws Exception { runTest("compiler/testData/codegen/box/primitiveTypes/equalityWithObject/intEqualsNullableInt.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index 1b6a4fbf1bf..6b0f7d36190 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -1034,6 +1034,11 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/conditions/nonZeroCompareInWhile.kt"); } + @TestMetadata("nullCompareConst.kt") + public void testNullCompareConst() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/conditions/nullCompareConst.kt"); + } + @TestMetadata("nullCompareInDoWhile.kt") public void testNullCompareInDoWhile() throws Exception { runTest("compiler/testData/codegen/bytecodeText/conditions/nullCompareInDoWhile.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 0ac872311ca..5cc5a669c8e 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -17830,6 +17830,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/primitiveTypes/equalityWithObject/boxedLongEqualsLong.kt"); } + @TestMetadata("intEqualsNull.kt") + public void testIntEqualsNull() throws Exception { + runTest("compiler/testData/codegen/box/primitiveTypes/equalityWithObject/intEqualsNull.kt"); + } + @TestMetadata("intEqualsNullableInt.kt") public void testIntEqualsNullableInt() throws Exception { runTest("compiler/testData/codegen/box/primitiveTypes/equalityWithObject/intEqualsNullableInt.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index a651dfc0c03..d3a619c9605 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -16715,6 +16715,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/primitiveTypes/equalityWithObject/boxedLongEqualsLong.kt"); } + @TestMetadata("intEqualsNull.kt") + public void testIntEqualsNull() throws Exception { + runTest("compiler/testData/codegen/box/primitiveTypes/equalityWithObject/intEqualsNull.kt"); + } + @TestMetadata("intEqualsNullableInt.kt") public void testIntEqualsNullableInt() throws Exception { runTest("compiler/testData/codegen/box/primitiveTypes/equalityWithObject/intEqualsNullableInt.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java index 1cf375adf4c..663fb8968a0 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java @@ -1034,6 +1034,11 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/conditions/nonZeroCompareInWhile.kt"); } + @TestMetadata("nullCompareConst.kt") + public void testNullCompareConst() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/conditions/nullCompareConst.kt"); + } + @TestMetadata("nullCompareInDoWhile.kt") public void testNullCompareInDoWhile() throws Exception { runTest("compiler/testData/codegen/bytecodeText/conditions/nullCompareInDoWhile.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java index a136bde78b1..bbb5261f590 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java @@ -13775,6 +13775,11 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { runTest("compiler/testData/codegen/box/primitiveTypes/equalityWithObject/boxedLongEqualsLong.kt"); } + @TestMetadata("intEqualsNull.kt") + public void testIntEqualsNull() throws Exception { + runTest("compiler/testData/codegen/box/primitiveTypes/equalityWithObject/intEqualsNull.kt"); + } + @TestMetadata("intEqualsNullableInt.kt") public void testIntEqualsNullableInt() throws Exception { runTest("compiler/testData/codegen/box/primitiveTypes/equalityWithObject/intEqualsNullableInt.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 53952e77007..d623be891f6 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 @@ -14930,6 +14930,11 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { runTest("compiler/testData/codegen/box/primitiveTypes/equalityWithObject/boxedLongEqualsLong.kt"); } + @TestMetadata("intEqualsNull.kt") + public void testIntEqualsNull() throws Exception { + runTest("compiler/testData/codegen/box/primitiveTypes/equalityWithObject/intEqualsNull.kt"); + } + @TestMetadata("intEqualsNullableInt.kt") public void testIntEqualsNullableInt() throws Exception { runTest("compiler/testData/codegen/box/primitiveTypes/equalityWithObject/intEqualsNullableInt.kt");