From 58685be4e2fcae3e8463e21462b125146c544e6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steven=20Sch=C3=A4fer?= Date: Thu, 26 Mar 2020 13:15:24 +0100 Subject: [PATCH] IR: Don't use IrStringConcatenation for ordinary toString calls We can only use IrStrinConcatentation to represent calls to Any?.toString and toString calls on primitive types. Otherwise, x.toString() and "$x" are observably different when x is a non-null type with null value (e.g., an @NotNull value coming from Java). --- .../ir/FirBlackBoxCodegenTestGenerated.java | 5 +++++ .../FlattenStringConcatenationLowering.kt | 17 +++++++------- .../codegen/box/strings/javaToStringNPE.kt | 22 +++++++++++++++++++ .../stringOperations/stringBuilderToString.kt | 12 ++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 5 +++++ .../codegen/BytecodeTextTestGenerated.java | 5 +++++ .../LightAnalysisModeTestGenerated.java | 5 +++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 5 +++++ .../ir/IrBytecodeTextTestGenerated.java | 5 +++++ 9 files changed, 73 insertions(+), 8 deletions(-) create mode 100644 compiler/testData/codegen/box/strings/javaToStringNPE.kt create mode 100644 compiler/testData/codegen/bytecodeText/stringOperations/stringBuilderToString.kt diff --git a/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java index 64b2283c761..06f0377e533 100644 --- a/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java @@ -26325,6 +26325,11 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/strings/interpolation.kt"); } + @TestMetadata("javaToStringNPE.kt") + public void testJavaToStringNPE() throws Exception { + runTest("compiler/testData/codegen/box/strings/javaToStringNPE.kt"); + } + @TestMetadata("kt2592.kt") public void testKt2592() throws Exception { runTest("compiler/testData/codegen/box/strings/kt2592.kt"); diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/FlattenStringConcatenationLowering.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/FlattenStringConcatenationLowering.kt index 3ddef3c595f..37bcbce91f1 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/FlattenStringConcatenationLowering.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/lower/FlattenStringConcatenationLowering.kt @@ -16,10 +16,7 @@ import org.jetbrains.kotlin.ir.expressions.IrCall import org.jetbrains.kotlin.ir.expressions.IrExpression import org.jetbrains.kotlin.ir.expressions.IrStringConcatenation import org.jetbrains.kotlin.ir.expressions.impl.IrStringConcatenationImpl -import org.jetbrains.kotlin.ir.types.isAny -import org.jetbrains.kotlin.ir.types.isNullableAny -import org.jetbrains.kotlin.ir.types.isString -import org.jetbrains.kotlin.ir.types.isStringClassType +import org.jetbrains.kotlin.ir.types.* import org.jetbrains.kotlin.ir.util.fqNameWhenAvailable import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid @@ -126,9 +123,13 @@ class FlattenStringConcatenationLowering(val context: CommonBackendContext) : Fi return function.isToString || function.isNullableToString } - /** @return true if the given expression is a [IrStringConcatenation], or an [IrCall] to [toString] or [String.plus]. */ + /** @return true if the given expression is a call to [Any?.toString] or a call of [toString] on a primitive type. */ + private val IrCall.isSpecialToStringCall: Boolean + get() = isToStringCall && dispatchReceiver?.type?.isPrimitiveType() != false + + /** @return true if the given expression is a [IrStringConcatenation], or an [IrCall] to [String.plus]. */ private fun isStringConcatenationExpression(expression: IrExpression): Boolean = - (expression is IrStringConcatenation) || (expression is IrCall) && (expression.isStringPlusCall || expression.isToStringCall) + (expression is IrStringConcatenation) || (expression is IrCall) && expression.isStringPlusCall /** Recursively collects string concatenation arguments from the given expression. */ private fun collectStringConcatenationArguments(expression: IrExpression): List { @@ -141,7 +142,7 @@ class FlattenStringConcatenationLowering(val context: CommonBackendContext) : Fi } override fun visitCall(expression: IrCall) { - if (isStringConcatenationExpression(expression)) { + if (isStringConcatenationExpression(expression) || expression.isToStringCall) { // Recursively collect from call arguments. expression.acceptChildrenVoid(this) } else { @@ -172,7 +173,7 @@ class FlattenStringConcatenationLowering(val context: CommonBackendContext) : Fi override fun visitExpression(expression: IrExpression): IrExpression { // Only modify/flatten string concatenation expressions. val transformedExpression = - if (isStringConcatenationExpression(expression)) + if (isStringConcatenationExpression(expression) || expression is IrCall && expression.isSpecialToStringCall) expression.run { IrStringConcatenationImpl( startOffset, diff --git a/compiler/testData/codegen/box/strings/javaToStringNPE.kt b/compiler/testData/codegen/box/strings/javaToStringNPE.kt new file mode 100644 index 00000000000..ed73f5d3cc3 --- /dev/null +++ b/compiler/testData/codegen/box/strings/javaToStringNPE.kt @@ -0,0 +1,22 @@ +// TARGET_BACKEND: JVM +// WITH_RUNTIME + +// FILE: J.java +import org.jetbrains.annotations.NotNull; + +public class J { + @NotNull + public static String notNullStringIsNull() { + return null; + } +} + +// FILE: test.kt +fun box(): String { + try { + J.notNullStringIsNull().toString() + } catch (e: java.lang.NullPointerException) { + return "OK" + } + return "Fail" +} diff --git a/compiler/testData/codegen/bytecodeText/stringOperations/stringBuilderToString.kt b/compiler/testData/codegen/bytecodeText/stringOperations/stringBuilderToString.kt new file mode 100644 index 00000000000..4b3ff2d6130 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/stringOperations/stringBuilderToString.kt @@ -0,0 +1,12 @@ +val chars = listOf('O', 'K') + +fun box(): String { + val b = StringBuilder() + for (c in chars) { + b.append(c) + } + return b.toString() +} + +// 0 INVOKESTATIC java/lang/String.valueOf +// 1 INVOKEVIRTUAL java/lang/StringBuilder.toString \ 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 c63557368db..f363c4b8ba4 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -27846,6 +27846,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/strings/interpolation.kt"); } + @TestMetadata("javaToStringNPE.kt") + public void testJavaToStringNPE() throws Exception { + runTest("compiler/testData/codegen/box/strings/javaToStringNPE.kt"); + } + @TestMetadata("kt2592.kt") public void testKt2592() throws Exception { runTest("compiler/testData/codegen/box/strings/kt2592.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index f6eaa187b59..80c7092df18 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -4245,6 +4245,11 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/stringOperations/singleConcat.kt"); } + @TestMetadata("stringBuilderToString.kt") + public void testStringBuilderToString() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/stringOperations/stringBuilderToString.kt"); + } + @TestMetadata("stringPlus.kt") public void testStringPlus() throws Exception { runTest("compiler/testData/codegen/bytecodeText/stringOperations/stringPlus.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 945b1cd0c1d..19a1d637078 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -26663,6 +26663,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/strings/interpolation.kt"); } + @TestMetadata("javaToStringNPE.kt") + public void testJavaToStringNPE() throws Exception { + runTest("compiler/testData/codegen/box/strings/javaToStringNPE.kt"); + } + @TestMetadata("kt2592.kt") public void testKt2592() throws Exception { runTest("compiler/testData/codegen/box/strings/kt2592.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 5f18cec046d..197696ee233 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -26325,6 +26325,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/strings/interpolation.kt"); } + @TestMetadata("javaToStringNPE.kt") + public void testJavaToStringNPE() throws Exception { + runTest("compiler/testData/codegen/box/strings/javaToStringNPE.kt"); + } + @TestMetadata("kt2592.kt") public void testKt2592() throws Exception { runTest("compiler/testData/codegen/box/strings/kt2592.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java index 56cecb0f278..6ef7b66cc28 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java @@ -4163,6 +4163,11 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/stringOperations/singleConcat.kt"); } + @TestMetadata("stringBuilderToString.kt") + public void testStringBuilderToString() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/stringOperations/stringBuilderToString.kt"); + } + @TestMetadata("stringPlus.kt") public void testStringPlus() throws Exception { runTest("compiler/testData/codegen/bytecodeText/stringOperations/stringPlus.kt");