From 480313210a510a7d65b9d431b53bccd8f73db0dd Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Tue, 6 Aug 2019 09:50:21 +0200 Subject: [PATCH] Use Intrinsics.checkNotNullExpressionValue to throw NPE in Java null checks Similarly to previous commit, this method was unused since its introduction before 1.0, so we're changing its semantics to throw NPE and starting to use it with API version >= 1.4. #KT-22275 In Progress --- .../org/jetbrains/kotlin/codegen/AsmUtil.java | 8 ++++--- .../RedundantNullCheckMethodTransformer.kt | 4 ++-- .../backend/jvm/codegen/ExpressionCodegen.kt | 9 +++---- .../javaNullCheckThrowsNpe_1_4.kt | 23 ++++++++++++++++++ .../notNullAssertions/arrayListGet.kt | 3 ++- .../assertionForNotNullCaptured.kt | 1 + .../javaMultipleSubstitutions.kt | 1 + .../deterministicNotNullChecks.kt | 1 + .../expressionValueIsNotNullTwice.kt | 3 ++- .../notNullExpressionValueTwice_1_4.kt | 24 +++++++++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 5 ++++ .../codegen/BytecodeTextTestGenerated.java | 5 ++++ .../LightAnalysisModeTestGenerated.java | 5 ++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 5 ++++ .../ir/IrBytecodeTextTestGenerated.java | 5 ++++ .../kotlin/jvm/internal/Intrinsics.java | 4 ++-- 16 files changed, 93 insertions(+), 13 deletions(-) create mode 100644 compiler/testData/codegen/box/nullCheckOptimization/javaNullCheckThrowsNpe_1_4.kt create mode 100644 compiler/testData/codegen/bytecodeText/nullCheckOptimization/notNullExpressionValueTwice_1_4.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/AsmUtil.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/AsmUtil.java index 344aa4a4793..11b37fcdd8e 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/AsmUtil.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/AsmUtil.java @@ -22,6 +22,7 @@ import org.jetbrains.kotlin.codegen.intrinsics.HashCode; import org.jetbrains.kotlin.codegen.intrinsics.IntrinsicMethods; import org.jetbrains.kotlin.codegen.state.GenerationState; import org.jetbrains.kotlin.codegen.state.KotlinTypeMapper; +import org.jetbrains.kotlin.config.ApiVersion; import org.jetbrains.kotlin.config.JvmTarget; import org.jetbrains.kotlin.config.LanguageFeature; import org.jetbrains.kotlin.config.LanguageVersionSettings; @@ -1013,7 +1014,6 @@ public class AsmUtil { if (runtimeAssertionInfo == null || !runtimeAssertionInfo.getNeedNotNullAssertion()) return stackValue; return new StackValue(stackValue.type, stackValue.kotlinType) { - @Override public void putSelector(@NotNull Type type, @Nullable KotlinType kotlinType, @NotNull InstructionAdapter v) { Type innerType = stackValue.type; @@ -1022,8 +1022,10 @@ public class AsmUtil { if (innerType.getSort() == Type.OBJECT || innerType.getSort() == Type.ARRAY) { v.dup(); v.visitLdcInsn(runtimeAssertionInfo.getMessage()); - v.invokestatic(IntrinsicMethods.INTRINSICS_CLASS_NAME, "checkExpressionValueIsNotNull", - "(Ljava/lang/Object;Ljava/lang/String;)V", false); + String methodName = state.getLanguageVersionSettings().getApiVersion().compareTo(ApiVersion.KOTLIN_1_4) >= 0 + ? "checkNotNullExpressionValue" + : "checkExpressionValueIsNotNull"; + v.invokestatic(IntrinsicMethods.INTRINSICS_CLASS_NAME, methodName, "(Ljava/lang/Object;Ljava/lang/String;)V", false); } StackValue.coerce(innerType, innerKotlinType, type, kotlinType, v); } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/nullCheck/RedundantNullCheckMethodTransformer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/nullCheck/RedundantNullCheckMethodTransformer.kt index e169dba2e7a..537804b4441 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/nullCheck/RedundantNullCheckMethodTransformer.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/nullCheck/RedundantNullCheckMethodTransformer.kt @@ -311,7 +311,7 @@ class RedundantNullCheckMethodTransformer(private val generationState: Generatio // ALOAD v // DUP // LDC * - // INVOKESTATIC checkExpressionValueIsNotNull + // INVOKESTATIC checkExpressionValueIsNotNull/checkNotNullExpressionValue // <...> -- v is not null here (otherwise an exception was thrown) methodNode.instructions.insert(insn, listOfSynthetics { @@ -440,7 +440,7 @@ internal fun AbstractInsnNode.isCheckParameterIsNotNull() = internal fun AbstractInsnNode.isCheckExpressionValueIsNotNull() = isInsn(Opcodes.INVOKESTATIC) { owner == IntrinsicMethods.INTRINSICS_CLASS_NAME && - name == "checkExpressionValueIsNotNull" && + (name == "checkExpressionValueIsNotNull" || name == "checkNotNullExpressionValue") && desc == "(Ljava/lang/Object;Ljava/lang/String;)V" } diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt index 203fd423d67..0f471bedef0 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt @@ -23,6 +23,7 @@ import org.jetbrains.kotlin.codegen.inline.ReifiedTypeInliner.OperationKind.SAFE import org.jetbrains.kotlin.codegen.pseudoInsns.fakeAlwaysFalseIfeq import org.jetbrains.kotlin.codegen.pseudoInsns.fixStackAndJump import org.jetbrains.kotlin.codegen.signature.BothSignatureWriter +import org.jetbrains.kotlin.config.ApiVersion import org.jetbrains.kotlin.config.isReleaseCoroutines import org.jetbrains.kotlin.config.languageVersionSettings import org.jetbrains.kotlin.descriptors.ClassDescriptor @@ -805,10 +806,10 @@ class ExpressionCodegen( val value = expression.argument.accept(this, data).materialized mv.dup() mv.visitLdcInsn("TODO provide message for IMPLICIT_NOTNULL") /*TODO*/ - mv.invokestatic( - "kotlin/jvm/internal/Intrinsics", "checkExpressionValueIsNotNull", - "(Ljava/lang/Object;Ljava/lang/String;)V", false - ) + val methodName = + if (state.languageVersionSettings.apiVersion >= ApiVersion.KOTLIN_1_4) "checkNotNullExpressionValue" + else "checkExpressionValueIsNotNull" + mv.invokestatic("kotlin/jvm/internal/Intrinsics", methodName, "(Ljava/lang/Object;Ljava/lang/String;)V", false) // Unbox primitives. value.coerce(expression.type) } diff --git a/compiler/testData/codegen/box/nullCheckOptimization/javaNullCheckThrowsNpe_1_4.kt b/compiler/testData/codegen/box/nullCheckOptimization/javaNullCheckThrowsNpe_1_4.kt new file mode 100644 index 00000000000..33602dfb875 --- /dev/null +++ b/compiler/testData/codegen/box/nullCheckOptimization/javaNullCheckThrowsNpe_1_4.kt @@ -0,0 +1,23 @@ +// !API_VERSION: LATEST +// TARGET_BACKEND: JVM +// IGNORE_BACKEND: JVM_IR +// FILE: A.java + +import org.jetbrains.annotations.NotNull; + +public class A { + @NotNull + public static String foo() { return null; } +} + +// FILE: test.kt + +fun box(): String { + try { + val s: String = A.foo() + return "Fail: NPE should have been thrown" + } catch (e: Throwable) { + if (e::class != NullPointerException::class) return "Fail: exception class should be NPE: ${e::class}" + return "OK" + } +} diff --git a/compiler/testData/codegen/bytecodeText/notNullAssertions/arrayListGet.kt b/compiler/testData/codegen/bytecodeText/notNullAssertions/arrayListGet.kt index a053458abc3..a131d2e2fd9 100644 --- a/compiler/testData/codegen/bytecodeText/notNullAssertions/arrayListGet.kt +++ b/compiler/testData/codegen/bytecodeText/notNullAssertions/arrayListGet.kt @@ -11,4 +11,5 @@ fun bar(a: ArrayList) { } // 1 checkExpressionValueIsNotNull -// 1 checkParameterIsNotNull \ No newline at end of file +// 0 checkNotNullExpressionValue +// 1 checkParameterIsNotNull diff --git a/compiler/testData/codegen/bytecodeText/notNullAssertions/assertionForNotNullCaptured.kt b/compiler/testData/codegen/bytecodeText/notNullAssertions/assertionForNotNullCaptured.kt index ac8b3bb13a3..8de96d2baad 100644 --- a/compiler/testData/codegen/bytecodeText/notNullAssertions/assertionForNotNullCaptured.kt +++ b/compiler/testData/codegen/bytecodeText/notNullAssertions/assertionForNotNullCaptured.kt @@ -10,3 +10,4 @@ public fun foo(x: MutableCollection, block: java.util.AbstractLi } // 1 checkExpressionValueIsNotNull +// 0 checkNotNullExpressionValue diff --git a/compiler/testData/codegen/bytecodeText/notNullAssertions/javaMultipleSubstitutions.kt b/compiler/testData/codegen/bytecodeText/notNullAssertions/javaMultipleSubstitutions.kt index a43487739e4..24bd3c5231d 100644 --- a/compiler/testData/codegen/bytecodeText/notNullAssertions/javaMultipleSubstitutions.kt +++ b/compiler/testData/codegen/bytecodeText/notNullAssertions/javaMultipleSubstitutions.kt @@ -35,4 +35,5 @@ internal fun bar(a: A, b: B, c: C) { // @JavaMultipleSubstitutionsKt.class // 3 checkExpressionValueIsNotNull +// 0 checkNotNullExpressionValue // 3 checkParameterIsNotNull diff --git a/compiler/testData/codegen/bytecodeText/nullCheckOptimization/deterministicNotNullChecks.kt b/compiler/testData/codegen/bytecodeText/nullCheckOptimization/deterministicNotNullChecks.kt index 9ace878dc25..76bfb612852 100644 --- a/compiler/testData/codegen/bytecodeText/nullCheckOptimization/deterministicNotNullChecks.kt +++ b/compiler/testData/codegen/bytecodeText/nullCheckOptimization/deterministicNotNullChecks.kt @@ -18,3 +18,4 @@ fun D.overriddenTreeUniqueAsSequenceA(): Boolean { // TODO: in fact, there should be an assertion, but it's missing because of https://youtrack.jetbrains.com/issue/KT-24210. // (This test's aim is not to check whether or not the assertion is generated, but to ensure that the behavior is deterministic.) // 0 checkExpressionValueIsNotNull +// 0 checkNotNullExpressionValue diff --git a/compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNullTwice.kt b/compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNullTwice.kt index e2bd9ec5c99..cae43053ca9 100644 --- a/compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNullTwice.kt +++ b/compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNullTwice.kt @@ -21,4 +21,5 @@ fun test() { // @KKt.class: // 1 LDC "a" -// 1 checkExpressionValueIsNotNull \ No newline at end of file +// 1 checkExpressionValueIsNotNull +// 0 checkNotNullExpressionValue diff --git a/compiler/testData/codegen/bytecodeText/nullCheckOptimization/notNullExpressionValueTwice_1_4.kt b/compiler/testData/codegen/bytecodeText/nullCheckOptimization/notNullExpressionValueTwice_1_4.kt new file mode 100644 index 00000000000..7daa421d473 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/nullCheckOptimization/notNullExpressionValueTwice_1_4.kt @@ -0,0 +1,24 @@ +// !API_VERSION: LATEST +// IGNORE_BACKEND: JVM_IR +// FILE: j/J.java + +package j; + +public class J { + public static final String ok() { return "OK"; } +} + +// FILE: foo.kt +fun foo(a: Any) {} + +// FILE: k.kt +import j.J + +fun test() { + val a = J.ok() + foo(a) + foo(a) +} + +// @KKt.class: +// 1 checkNotNullExpressionValue diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index a0f68d4e142..48e0a53b40d 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -16414,6 +16414,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/nullCheckOptimization/isNullable.kt"); } + @TestMetadata("javaNullCheckThrowsNpe_1_4.kt") + public void testJavaNullCheckThrowsNpe_1_4() throws Exception { + runTest("compiler/testData/codegen/box/nullCheckOptimization/javaNullCheckThrowsNpe_1_4.kt"); + } + @TestMetadata("kt22410.kt") public void testKt22410() throws Exception { runTest("compiler/testData/codegen/box/nullCheckOptimization/kt22410.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index a82c9e29936..3094339ae47 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -3276,6 +3276,11 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/nullCheckOptimization/notNullAsNotNullable.kt"); } + @TestMetadata("notNullExpressionValueTwice_1_4.kt") + public void testNotNullExpressionValueTwice_1_4() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/nullCheckOptimization/notNullExpressionValueTwice_1_4.kt"); + } + @TestMetadata("nullCheckAfterExclExcl_1_4.kt") public void testNullCheckAfterExclExcl_1_4() throws Exception { runTest("compiler/testData/codegen/bytecodeText/nullCheckOptimization/nullCheckAfterExclExcl_1_4.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index e380850bf6d..42ae5338e93 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -16414,6 +16414,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/nullCheckOptimization/isNullable.kt"); } + @TestMetadata("javaNullCheckThrowsNpe_1_4.kt") + public void testJavaNullCheckThrowsNpe_1_4() throws Exception { + runTest("compiler/testData/codegen/box/nullCheckOptimization/javaNullCheckThrowsNpe_1_4.kt"); + } + @TestMetadata("kt22410.kt") public void testKt22410() throws Exception { runTest("compiler/testData/codegen/box/nullCheckOptimization/kt22410.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index f8ddc5cb5ec..81c3ff195e6 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -15299,6 +15299,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/nullCheckOptimization/isNullable.kt"); } + @TestMetadata("javaNullCheckThrowsNpe_1_4.kt") + public void testJavaNullCheckThrowsNpe_1_4() throws Exception { + runTest("compiler/testData/codegen/box/nullCheckOptimization/javaNullCheckThrowsNpe_1_4.kt"); + } + @TestMetadata("kt22410.kt") public void testKt22410() throws Exception { runTest("compiler/testData/codegen/box/nullCheckOptimization/kt22410.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java index 24252c08781..a671370b3d1 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java @@ -3246,6 +3246,11 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/nullCheckOptimization/notNullAsNotNullable.kt"); } + @TestMetadata("notNullExpressionValueTwice_1_4.kt") + public void testNotNullExpressionValueTwice_1_4() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/nullCheckOptimization/notNullExpressionValueTwice_1_4.kt"); + } + @TestMetadata("nullCheckAfterExclExcl_1_4.kt") public void testNullCheckAfterExclExcl_1_4() throws Exception { runTest("compiler/testData/codegen/bytecodeText/nullCheckOptimization/nullCheckAfterExclExcl_1_4.kt"); diff --git a/libraries/stdlib/jvm/runtime/kotlin/jvm/internal/Intrinsics.java b/libraries/stdlib/jvm/runtime/kotlin/jvm/internal/Intrinsics.java index 84432b4d7f4..fe02d6fe5e5 100644 --- a/libraries/stdlib/jvm/runtime/kotlin/jvm/internal/Intrinsics.java +++ b/libraries/stdlib/jvm/runtime/kotlin/jvm/internal/Intrinsics.java @@ -88,9 +88,9 @@ public class Intrinsics { } } - public static void checkNotNullExpressionValue(Object value, String message) { + public static void checkNotNullExpressionValue(Object value, String expression) { if (value == null) { - throw sanitizeStackTrace(new IllegalStateException(message)); + throw sanitizeStackTrace(new NullPointerException(expression + " must not be null")); } }