[JVM IR] Maintain KT-36625 bug compatibility between non-IR and IR
backends by removing IMPLICIT_NOTNULL casts from IrStringConcatenation
arguments.
Also fixed an issue where IrStringConcatenation can be lowered into
a null String instead of a literal "null" String if the lone argument
was a platform type String or String with enhanced nullability and the
value was null (e.g., "${FromJava.nullPlatformString()}").
This commit is contained in:
Mark Punzalan
committed by
Dmitry Petrov
Dmitry Petrov
parent
18a3d7ee08
commit
ba606147c9
+5
-1
@@ -7,6 +7,8 @@ package org.jetbrains.kotlin.backend.jvm.lower
|
||||
|
||||
import org.jetbrains.kotlin.backend.common.FileLoweringPass
|
||||
import org.jetbrains.kotlin.backend.common.lower.SpecialBridgeMethods
|
||||
import org.jetbrains.kotlin.backend.common.lower.flattenStringConcatenationPhase
|
||||
import org.jetbrains.kotlin.backend.common.lower.loops.forLoopsPhase
|
||||
import org.jetbrains.kotlin.backend.common.phaser.makeIrFilePhase
|
||||
import org.jetbrains.kotlin.backend.jvm.JvmBackendContext
|
||||
import org.jetbrains.kotlin.config.ApiVersion
|
||||
@@ -22,7 +24,9 @@ import org.jetbrains.kotlin.utils.addToStdlib.safeAs
|
||||
val jvmArgumentNullabilityAssertions = makeIrFilePhase(
|
||||
::JvmArgumentNullabilityAssertionsLowering,
|
||||
name = "ArgumentNullabilityAssertions",
|
||||
description = "Transform nullability assertions on arguments according to the compiler settings"
|
||||
description = "Transform nullability assertions on arguments according to the compiler settings",
|
||||
// jvmStringConcatenationLowering may remove IMPLICIT_NOTNULL casts.
|
||||
prerequisite = setOf(jvmStringConcatenationLowering)
|
||||
)
|
||||
|
||||
private enum class AssertionScope {
|
||||
|
||||
+22
-6
@@ -24,6 +24,8 @@ import org.jetbrains.kotlin.ir.declarations.IrFile
|
||||
import org.jetbrains.kotlin.ir.declarations.IrSimpleFunction
|
||||
import org.jetbrains.kotlin.ir.expressions.IrExpression
|
||||
import org.jetbrains.kotlin.ir.expressions.IrStringConcatenation
|
||||
import org.jetbrains.kotlin.ir.expressions.IrTypeOperator
|
||||
import org.jetbrains.kotlin.ir.expressions.IrTypeOperatorCall
|
||||
import org.jetbrains.kotlin.ir.types.*
|
||||
import org.jetbrains.kotlin.ir.util.constructors
|
||||
import org.jetbrains.kotlin.ir.util.functions
|
||||
@@ -86,9 +88,6 @@ private class JvmStringConcatenationLowering(val context: JvmBackendContext) : F
|
||||
}
|
||||
|
||||
private fun JvmIrBuilder.callToString(expression: IrExpression): IrExpression {
|
||||
if (expression.type.isString())
|
||||
return expression
|
||||
|
||||
val argument = widenIntegerType(expression)
|
||||
val argumentType = if (argument.type.isPrimitiveType()) argument.type else context.irBuiltIns.anyNType
|
||||
|
||||
@@ -115,6 +114,18 @@ private class JvmStringConcatenationLowering(val context: JvmBackendContext) : F
|
||||
override fun visitStringConcatenation(expression: IrStringConcatenation): IrExpression {
|
||||
expression.transformChildrenVoid(this)
|
||||
return context.createJvmIrBuilder(currentScope!!.scope.scopeOwnerSymbol, expression.startOffset, expression.endOffset).run {
|
||||
// When `String.plus(Any?)` is invoked with receiver of platform type String or String with enhanced nullability, this SHOULD
|
||||
// fail a nullability check (NullPointerException) on the receiver. However, the non-IR backend has a bug (KT-36625) where this
|
||||
// check is not inserted (see KT-36625). To maintain bug compatibility with the non-IR backend, we remove IMPLICIT_NOTNULL casts
|
||||
// (which generate the nullability checks in JvmArgumentNullabilityAssertionsLowering) from all arguments.
|
||||
|
||||
fun IrExpression.unwrapImplicitNotNull() =
|
||||
if (this is IrTypeOperatorCall && operator == IrTypeOperator.IMPLICIT_NOTNULL)
|
||||
argument
|
||||
else
|
||||
this
|
||||
|
||||
|
||||
val arguments = expression.arguments.map { lowerInlineClassArgument(it) }
|
||||
|
||||
when {
|
||||
@@ -122,11 +133,13 @@ private class JvmStringConcatenationLowering(val context: JvmBackendContext) : F
|
||||
irString("")
|
||||
|
||||
arguments.size == 1 ->
|
||||
callToString(arguments.single())
|
||||
callToString(arguments.single().unwrapImplicitNotNull())
|
||||
|
||||
arguments.size == 2 && arguments[0].type.isStringClassType() ->
|
||||
irCall(backendContext.ir.symbols.intrinsicStringPlus).apply {
|
||||
putValueArgument(0, arguments[0])
|
||||
putValueArgument(0, arguments[0].unwrapImplicitNotNull())
|
||||
|
||||
// Unwrapping IMPLICIT_NOTNULL is not strictly necessary on 2nd argument (parameter type is `Any?`)
|
||||
putValueArgument(1, arguments[1])
|
||||
}
|
||||
|
||||
@@ -137,7 +150,10 @@ private class JvmStringConcatenationLowering(val context: JvmBackendContext) : F
|
||||
val appendFunction = typeToAppendFunction(argument.type)
|
||||
stringBuilder = irCall(appendFunction).apply {
|
||||
dispatchReceiver = stringBuilder
|
||||
putValueArgument(0, argument)
|
||||
|
||||
// Unwrapping IMPLICIT_NOTNULL is necessary for ALL arguments. There could be a call to `String.plus(Any?)`
|
||||
// anywhere in the flattened IrStringConcatenation expression, e.g., `"foo" + (Java.platformString() + 123)`.
|
||||
putValueArgument(0, argument.unwrapImplicitNotNull())
|
||||
}
|
||||
}
|
||||
irCall(toStringFunction).apply {
|
||||
|
||||
@@ -0,0 +1,138 @@
|
||||
// IGNORE_BACKEND_FIR: JVM_IR
|
||||
// TARGET_BACKEND: JVM
|
||||
// WITH_RUNTIME
|
||||
|
||||
// FILE: J.java
|
||||
import org.jetbrains.annotations.NotNull;
|
||||
import org.jetbrains.annotations.Nullable;
|
||||
|
||||
public class J {
|
||||
public static String platformStringIsNull() {
|
||||
return null;
|
||||
}
|
||||
|
||||
@NotNull
|
||||
public static String notNullStringIsNull() {
|
||||
return null;
|
||||
}
|
||||
|
||||
@Nullable
|
||||
public static String nullableStringIsNull() {
|
||||
return null;
|
||||
}
|
||||
|
||||
@NotNull
|
||||
public static String notNullStringIsNotNull() {
|
||||
return "foo";
|
||||
}
|
||||
|
||||
public static String platformStringIsNotNull() {
|
||||
return "foo";
|
||||
}
|
||||
|
||||
@Nullable
|
||||
public static String nullableStringIsNotNull() {
|
||||
return "foo";
|
||||
}
|
||||
}
|
||||
|
||||
// FILE: main.kt
|
||||
import kotlin.test.assertEquals
|
||||
|
||||
fun box(): String {
|
||||
val n = 123
|
||||
|
||||
// Due to KT-36625, certain concatenation calls below (marked with a comment) SHOULD fail a nullability check but do not.
|
||||
|
||||
assertEquals("null", "${J.platformStringIsNull()}")
|
||||
assertEquals("nullBAR", J.platformStringIsNull() + "BAR") // KT-36625
|
||||
assertEquals("nullBAR", "${J.platformStringIsNull() + "BAR"}") // KT-36625
|
||||
assertEquals("nullBAR", "${J.platformStringIsNull()}BAR")
|
||||
assertEquals("BARnull", "BAR" + J.platformStringIsNull())
|
||||
assertEquals("BARnull", "BAR${J.platformStringIsNull()}")
|
||||
assertEquals("123null", "$n${J.platformStringIsNull()}")
|
||||
assertEquals("nullBAR123", J.platformStringIsNull() + "BAR" + n) // KT-36625
|
||||
assertEquals("nullBAR123", "${J.platformStringIsNull() + "BAR" + n}") // KT-36625
|
||||
assertEquals("nullBAR123", "${J.platformStringIsNull()}BAR$n")
|
||||
assertEquals("BARnull123", "BAR" + J.platformStringIsNull() + n)
|
||||
assertEquals("BARnull123", "BAR${J.platformStringIsNull()}$n")
|
||||
assertEquals("BARnull123", "BAR" + (J.platformStringIsNull() + n)) // KT-36625
|
||||
assertEquals("123nullBAR", "$n${J.platformStringIsNull() + "BAR"}") // KT-36625
|
||||
|
||||
assertEquals("null", "${J.notNullStringIsNull()}")
|
||||
assertEquals("nullBAR", J.notNullStringIsNull() + "BAR") // KT-36625
|
||||
assertEquals("nullBAR", "${J.notNullStringIsNull() + "BAR"}") // KT-36625
|
||||
assertEquals("nullBAR", "${J.notNullStringIsNull()}BAR")
|
||||
assertEquals("BARnull", "BAR" + J.notNullStringIsNull())
|
||||
assertEquals("BARnull", "BAR${J.notNullStringIsNull()}")
|
||||
assertEquals("123null", "$n${J.notNullStringIsNull()}")
|
||||
assertEquals("nullBAR123", J.notNullStringIsNull() + "BAR" + n) // KT-36625
|
||||
assertEquals("nullBAR123", "${J.notNullStringIsNull() + "BAR" + n}") // KT-36625
|
||||
assertEquals("nullBAR123", "${J.notNullStringIsNull()}BAR$n")
|
||||
assertEquals("BARnull123", "BAR" + J.notNullStringIsNull() + n)
|
||||
assertEquals("BARnull123", "BAR${J.notNullStringIsNull()}$n")
|
||||
assertEquals("BARnull123", "BAR" + (J.notNullStringIsNull() + n)) // KT-36625
|
||||
assertEquals("123nullBAR", "$n${J.notNullStringIsNull() + "BAR"}") // KT-36625
|
||||
|
||||
assertEquals("null", "${J.nullableStringIsNull()}")
|
||||
assertEquals("nullBAR", J.nullableStringIsNull() + "BAR")
|
||||
assertEquals("nullBAR", "${J.nullableStringIsNull() + "BAR"}")
|
||||
assertEquals("nullBAR", "${J.nullableStringIsNull()}BAR")
|
||||
assertEquals("BARnull", "BAR" + J.nullableStringIsNull())
|
||||
assertEquals("BARnull", "BAR${J.nullableStringIsNull()}")
|
||||
assertEquals("123null", "$n${J.nullableStringIsNull()}")
|
||||
assertEquals("nullBAR123", J.nullableStringIsNull() + "BAR" + n)
|
||||
assertEquals("nullBAR123", "${J.nullableStringIsNull() + "BAR" + n}")
|
||||
assertEquals("nullBAR123", "${J.nullableStringIsNull()}BAR$n")
|
||||
assertEquals("BARnull123", "BAR" + J.nullableStringIsNull() + n)
|
||||
assertEquals("BARnull123", "BAR${J.nullableStringIsNull()}$n")
|
||||
assertEquals("BARnull123", "BAR" + (J.nullableStringIsNull() + n))
|
||||
assertEquals("123nullBAR", "$n${J.nullableStringIsNull() + "BAR"}")
|
||||
|
||||
assertEquals("foo", "${J.platformStringIsNotNull()}")
|
||||
assertEquals("fooBAR", J.platformStringIsNotNull() + "BAR")
|
||||
assertEquals("fooBAR", "${J.platformStringIsNotNull() + "BAR"}")
|
||||
assertEquals("fooBAR", "${J.platformStringIsNotNull()}BAR")
|
||||
assertEquals("BARfoo", "BAR" + J.platformStringIsNotNull())
|
||||
assertEquals("BARfoo", "BAR${J.platformStringIsNotNull()}")
|
||||
assertEquals("123foo", "$n${J.platformStringIsNotNull()}")
|
||||
assertEquals("fooBAR123", J.platformStringIsNotNull() + "BAR" + n)
|
||||
assertEquals("fooBAR123", "${J.platformStringIsNotNull() + "BAR" + n}")
|
||||
assertEquals("fooBAR123", "${J.platformStringIsNotNull()}BAR$n")
|
||||
assertEquals("BARfoo123", "BAR" + J.platformStringIsNotNull() + n)
|
||||
assertEquals("BARfoo123", "BAR${J.platformStringIsNotNull()}$n")
|
||||
assertEquals("BARfoo123", "BAR" + (J.platformStringIsNotNull() + n))
|
||||
assertEquals("123fooBAR", "$n${J.platformStringIsNotNull() + "BAR"}")
|
||||
|
||||
assertEquals("foo", "${J.notNullStringIsNotNull()}")
|
||||
assertEquals("fooBAR", J.notNullStringIsNotNull() + "BAR")
|
||||
assertEquals("fooBAR", "${J.notNullStringIsNotNull() + "BAR"}")
|
||||
assertEquals("fooBAR", "${J.notNullStringIsNotNull()}BAR")
|
||||
assertEquals("BARfoo", "BAR" + J.notNullStringIsNotNull())
|
||||
assertEquals("BARfoo", "BAR${J.notNullStringIsNotNull()}")
|
||||
assertEquals("123foo", "$n${J.notNullStringIsNotNull()}")
|
||||
assertEquals("fooBAR123", J.notNullStringIsNotNull() + "BAR" + n)
|
||||
assertEquals("fooBAR123", "${J.notNullStringIsNotNull() + "BAR" + n}")
|
||||
assertEquals("fooBAR123", "${J.notNullStringIsNotNull()}BAR$n")
|
||||
assertEquals("BARfoo123", "BAR" + J.notNullStringIsNotNull() + n)
|
||||
assertEquals("BARfoo123", "BAR${J.notNullStringIsNotNull()}$n")
|
||||
assertEquals("BARfoo123", "BAR" + (J.notNullStringIsNotNull() + n))
|
||||
assertEquals("123fooBAR", "$n${J.notNullStringIsNotNull() + "BAR"}")
|
||||
|
||||
assertEquals("foo", "${J.nullableStringIsNotNull()}")
|
||||
assertEquals("fooBAR", J.nullableStringIsNotNull() + "BAR")
|
||||
assertEquals("fooBAR", "${J.nullableStringIsNotNull() + "BAR"}")
|
||||
assertEquals("fooBAR", "${J.nullableStringIsNotNull()}BAR")
|
||||
assertEquals("BARfoo", "BAR" + J.nullableStringIsNotNull())
|
||||
assertEquals("BARfoo", "BAR${J.nullableStringIsNotNull()}")
|
||||
assertEquals("123foo", "$n${J.nullableStringIsNotNull()}")
|
||||
assertEquals("fooBAR123", J.nullableStringIsNotNull() + "BAR" + n)
|
||||
assertEquals("fooBAR123", "${J.nullableStringIsNotNull() + "BAR" + n}")
|
||||
assertEquals("fooBAR123", "${J.nullableStringIsNotNull()}BAR$n")
|
||||
assertEquals("BARfoo123", "BAR" + J.nullableStringIsNotNull() + n)
|
||||
assertEquals("BARfoo123", "BAR${J.nullableStringIsNotNull()}$n")
|
||||
assertEquals("BARfoo123", "BAR" + (J.nullableStringIsNotNull() + n))
|
||||
assertEquals("123fooBAR", "$n${J.nullableStringIsNotNull() + "BAR"}")
|
||||
|
||||
return "OK"
|
||||
}
|
||||
@@ -1,8 +1,6 @@
|
||||
// IGNORE_BACKEND: JVM
|
||||
|
||||
fun f(s: String) = "$s"
|
||||
|
||||
fun g(s: String?) = "$s"
|
||||
|
||||
// 1 valueOf
|
||||
// 2 valueOf
|
||||
// 0 NEW java/lang/StringBuilder
|
||||
+5
@@ -27490,6 +27490,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
|
||||
runTest("compiler/testData/codegen/box/strings/stringBuilderAppend.kt");
|
||||
}
|
||||
|
||||
@TestMetadata("stringFromJavaPlus.kt")
|
||||
public void testStringFromJavaPlus() throws Exception {
|
||||
runTest("compiler/testData/codegen/box/strings/stringFromJavaPlus.kt");
|
||||
}
|
||||
|
||||
@TestMetadata("stringPlusOnlyWorksOnString.kt")
|
||||
public void testStringPlusOnlyWorksOnString() throws Exception {
|
||||
runTest("compiler/testData/codegen/box/strings/stringPlusOnlyWorksOnString.kt");
|
||||
|
||||
+5
@@ -26307,6 +26307,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes
|
||||
runTest("compiler/testData/codegen/box/strings/stringBuilderAppend.kt");
|
||||
}
|
||||
|
||||
@TestMetadata("stringFromJavaPlus.kt")
|
||||
public void testStringFromJavaPlus() throws Exception {
|
||||
runTest("compiler/testData/codegen/box/strings/stringFromJavaPlus.kt");
|
||||
}
|
||||
|
||||
@TestMetadata("stringPlusOnlyWorksOnString.kt")
|
||||
public void testStringPlusOnlyWorksOnString() throws Exception {
|
||||
runTest("compiler/testData/codegen/box/strings/stringPlusOnlyWorksOnString.kt");
|
||||
|
||||
+5
@@ -25994,6 +25994,11 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT
|
||||
runTest("compiler/testData/codegen/box/strings/stringBuilderAppend.kt");
|
||||
}
|
||||
|
||||
@TestMetadata("stringFromJavaPlus.kt")
|
||||
public void testStringFromJavaPlus() throws Exception {
|
||||
runTest("compiler/testData/codegen/box/strings/stringFromJavaPlus.kt");
|
||||
}
|
||||
|
||||
@TestMetadata("stringPlusOnlyWorksOnString.kt")
|
||||
public void testStringPlusOnlyWorksOnString() throws Exception {
|
||||
runTest("compiler/testData/codegen/box/strings/stringPlusOnlyWorksOnString.kt");
|
||||
|
||||
+5
@@ -25994,6 +25994,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
|
||||
runTest("compiler/testData/codegen/box/strings/stringBuilderAppend.kt");
|
||||
}
|
||||
|
||||
@TestMetadata("stringFromJavaPlus.kt")
|
||||
public void testStringFromJavaPlus() throws Exception {
|
||||
runTest("compiler/testData/codegen/box/strings/stringFromJavaPlus.kt");
|
||||
}
|
||||
|
||||
@TestMetadata("stringPlusOnlyWorksOnString.kt")
|
||||
public void testStringPlusOnlyWorksOnString() throws Exception {
|
||||
runTest("compiler/testData/codegen/box/strings/stringPlusOnlyWorksOnString.kt");
|
||||
|
||||
Reference in New Issue
Block a user