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 bb2f2456658..745d905a80b 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 @@ -21,6 +21,7 @@ import org.jetbrains.kotlin.codegen.inline.ReifiedTypeInliner import org.jetbrains.kotlin.codegen.optimization.DeadCodeEliminationMethodTransformer import org.jetbrains.kotlin.codegen.optimization.common.OptimizationBasicInterpreter import org.jetbrains.kotlin.codegen.optimization.common.StrictBasicValue +import org.jetbrains.kotlin.codegen.optimization.common.debugText import org.jetbrains.kotlin.codegen.optimization.common.isInsn import org.jetbrains.kotlin.codegen.optimization.fixStack.peek import org.jetbrains.kotlin.codegen.optimization.fixStack.top @@ -61,7 +62,7 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() { if (insn.isInstanceOfOrNullCheck()) { checkedReferenceTypes[insn] = frame?.top()?.type ?: continue } - else if (insn.isCheckParameterNotNull()) { + else if (insn.isCheckParameterIsNotNull() || insn.isCheckExpressionValueIsNotNull()) { checkedReferenceTypes[insn] = frame?.peek(1)?.type ?: continue } } @@ -92,14 +93,18 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() { val insns = methodNode.instructions.toArray() val nullabilityMap = LinkedHashMap() for (i in insns.indices) { - val top = frames[i]?.top() as? StrictBasicValue ?: continue - val nullability = top.getNullability() - if (nullability == Nullability.NULLABLE) continue - + val frame = frames[i] ?: continue val insn = insns[i] - if (insn.isInstanceOfOrNullCheck()) { - nullabilityMap[insn] = top - } + + val value = when { + insn.isInstanceOfOrNullCheck() -> frame.top() + insn.isCheckExpressionValueIsNotNull() -> frame.peek(1) + else -> null + } as? StrictBasicValue ?: continue + + val nullability = value.getNullability() + if (nullability == Nullability.NULLABLE) continue + nullabilityMap[insn] = value } return nullabilityMap } @@ -111,6 +116,12 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() { Opcodes.IFNULL -> transformTrivialNullJump(insn as JumpInsnNode, nullability == Nullability.NULL) Opcodes.IFNONNULL -> transformTrivialNullJump(insn as JumpInsnNode, nullability == Nullability.NOT_NULL) Opcodes.INSTANCEOF -> transformInstanceOf(insn as TypeInsnNode, nullability, value) + + Opcodes.INVOKESTATIC -> { + if (insn.isCheckExpressionValueIsNotNull()) { + transformTrivialCheckExpressionValueIsNotNull(insn, nullability) + } + } } } } @@ -148,6 +159,16 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() { } } + private fun transformTrivialCheckExpressionValueIsNotNull(insn: AbstractInsnNode, nullability: Nullability) { + if (nullability != Nullability.NOT_NULL) return + val ldcInsn = insn.previous?.takeIf { it.opcode == Opcodes.LDC } ?: return + methodNode.instructions.run { + popReferenceValueBefore(ldcInsn) + remove(ldcInsn) + remove(insn) + } + } + private inner class NullabilityAssumptionsBuilder(val checkedReferenceTypes: Map) { private val checksDependingOnVariable = HashMap>() @@ -158,25 +179,47 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() { } private fun collectVariableDependentChecks() { - for (insn in methodNode.instructions) { - if (insn.isInstanceOfOrNullCheck()) { - val previous = insn.previous ?: continue - if (previous.opcode == Opcodes.ALOAD) { - addDependentCheck(insn, previous as VarInsnNode) - } - else if (previous.opcode == Opcodes.DUP) { - val previous2 = previous.previous ?: continue - if (previous2.opcode == Opcodes.ALOAD) { - addDependentCheck(insn, previous2 as VarInsnNode) + insnLoop@ for (insn in methodNode.instructions) { + when { + insn.isInstanceOfOrNullCheck() -> { + val previous = insn.previous ?: continue@insnLoop + if (previous.opcode == Opcodes.ALOAD) { + addDependentCheck(insn, previous as VarInsnNode) + } + else if (previous.opcode == Opcodes.DUP) { + val previous2 = previous.previous ?: continue@insnLoop + if (previous2.opcode == Opcodes.ALOAD) { + addDependentCheck(insn, previous2 as VarInsnNode) + } } } - } - else if (insn.isCheckParameterNotNull()) { - val ldcInsn = insn.previous ?: continue - if (ldcInsn.opcode != Opcodes.LDC) continue - val aLoadInsn = ldcInsn.previous ?: continue - if (aLoadInsn.opcode != Opcodes.ALOAD) continue - addDependentCheck(insn, aLoadInsn as VarInsnNode) + + insn.isCheckParameterIsNotNull() -> { + val ldcInsn = insn.previous ?: continue@insnLoop + if (ldcInsn.opcode != Opcodes.LDC) continue@insnLoop + val aLoadInsn = ldcInsn.previous ?: continue@insnLoop + if (aLoadInsn.opcode != Opcodes.ALOAD) continue@insnLoop + addDependentCheck(insn, aLoadInsn as VarInsnNode) + } + + insn.isCheckExpressionValueIsNotNull() -> { + val ldcInsn = insn.previous ?: continue@insnLoop + if (ldcInsn.opcode != Opcodes.LDC) continue@insnLoop + var aLoadInsn: VarInsnNode? = null + val insn1 = ldcInsn.previous ?: continue@insnLoop + if (insn1.opcode == Opcodes.ALOAD) { + aLoadInsn = insn1 as VarInsnNode + } + else if (insn1.opcode == Opcodes.DUP) { + val insn2 = insn1.previous ?: continue@insnLoop + if (insn2.opcode == Opcodes.ALOAD) { + aLoadInsn = insn2 as VarInsnNode + } + } + if (aLoadInsn == null) continue@insnLoop + addDependentCheck(insn, aLoadInsn) + } + } } } @@ -196,6 +239,11 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() { nullabilityAssumptions.injectAssumptionsForCheck(varIndex, checkInsn, varType) } } + for (insn in methodNode.instructions) { + if (insn.isThrowNpeIntrinsic()) { + nullabilityAssumptions.injectCodeForThrowNpe(insn) + } + } return nullabilityAssumptions } @@ -205,8 +253,10 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() { Opcodes.IFNONNULL -> injectAssumptionsForNullCheck(varIndex, insn as JumpInsnNode, varType) Opcodes.INVOKESTATIC -> { - assert(insn.isCheckParameterNotNull()) { "Expected non-null parameter check @${insn.getIndex()}"} - injectAssumptionsForParameterNotNullCheck(varIndex, insn, varType) + assert(insn.isCheckParameterIsNotNull() || insn.isCheckExpressionValueIsNotNull()) { + "Expected non-null assertion: ${insn.debugText}" + } + injectAssumptionsForNotNullAssertion(varIndex, insn, varType) } Opcodes.INSTANCEOF -> injectAssumptionsForInstanceOfCheck(varIndex, insn, varType) @@ -249,12 +299,18 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() { } } - private fun NullabilityAssumptions.injectAssumptionsForParameterNotNullCheck(varIndex: Int, insn: AbstractInsnNode, varType: Type) { + private fun NullabilityAssumptions.injectAssumptionsForNotNullAssertion(varIndex: Int, insn: AbstractInsnNode, varType: Type) { // ALOAD v - // LDC param_name + // LDC * // INVOKESTATIC checkParameterIsNotNull // <...> -- v is not null here (otherwise an exception was thrown) + // ALOAD v + // DUP + // LDC * + // INVOKESTATIC checkExpressionValueIsNotNull + // <...> -- v is not null here (otherwise an exception was thrown) + methodNode.instructions.insert(insn, listOfSynthetics { anew(varType) store(varIndex, varType) @@ -300,6 +356,15 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() { } } + private fun NullabilityAssumptions.injectCodeForThrowNpe(insn: AbstractInsnNode) { + methodNode.instructions.run { + insert(insn, listOfSynthetics { + aconst(null) + athrow() + }) + } + } + } inner class NullabilityAssumptions { @@ -336,13 +401,27 @@ internal fun AbstractInsnNode.isInstanceOfOrNullCheck() = opcode == Opcodes.IFNULL || opcode == Opcodes.IFNONNULL -internal fun AbstractInsnNode.isCheckParameterNotNull() = +internal fun AbstractInsnNode.isCheckParameterIsNotNull() = isInsn(Opcodes.INVOKESTATIC) { owner == "kotlin/jvm/internal/Intrinsics" && name == "checkParameterIsNotNull" && desc == "(Ljava/lang/Object;Ljava/lang/String;)V" } +internal fun AbstractInsnNode.isCheckExpressionValueIsNotNull() = + isInsn(Opcodes.INVOKESTATIC) { + owner == "kotlin/jvm/internal/Intrinsics" && + name == "checkExpressionValueIsNotNull" && + desc == "(Ljava/lang/Object;Ljava/lang/String;)V" + } + +internal fun AbstractInsnNode.isThrowNpeIntrinsic() = + isInsn(Opcodes.INVOKESTATIC) { + owner == "kotlin/jvm/internal/Intrinsics" && + name == "throwNpe" && + desc == "()V" + } + internal fun InsnList.popReferenceValueBefore(insn: AbstractInsnNode) { val prev = insn.previous when (prev?.opcode) { diff --git a/compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNull.kt b/compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNull.kt new file mode 100644 index 00000000000..758216e51e3 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNull.kt @@ -0,0 +1,23 @@ +// FILE: j/J.java + +package j; + +public class J { + public static final String ok() { return "OK"; } +} + +// FILE: k.kt +import j.J + +fun foo(a: Any) {} + +fun test() { + val a = J.ok() + foo(a) + if (a == null) foo("NULL-1") +} + +// @KKt.class: +// 0 IFNULL +// 0 IFNONNULL +// 0 NULL-1 \ No newline at end of file diff --git a/compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNullAfterExclExcl.kt b/compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNullAfterExclExcl.kt new file mode 100644 index 00000000000..c751a9fe5b7 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNullAfterExclExcl.kt @@ -0,0 +1,24 @@ +// FILE: j/J.java + +package j; + +public class J { + public static final String ok() { return "OK"; } +} + +// FILE: k.kt +import j.J + +fun foo(a: Any) {} + +fun test() { + val a = J.ok() + a!! + foo(a) + if (a == null) foo("NULL-1") +} + +// @KKt.class: +// 0 IFNULL +// 1 IFNONNULL +// 0 NULL-1 \ No newline at end of file diff --git a/compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNullTwice.kt b/compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNullTwice.kt new file mode 100644 index 00000000000..c69e95a81d6 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNullTwice.kt @@ -0,0 +1,23 @@ +// 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 LDC "a" +// 1 checkExpressionValueIsNotNull \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index 2d796711387..79b503dd224 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -1739,6 +1739,24 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { doTest(fileName); } + @TestMetadata("expressionValueIsNotNull.kt") + public void testExpressionValueIsNotNull() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNull.kt"); + doTest(fileName); + } + + @TestMetadata("expressionValueIsNotNullAfterExclExcl.kt") + public void testExpressionValueIsNotNullAfterExclExcl() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNullAfterExclExcl.kt"); + doTest(fileName); + } + + @TestMetadata("expressionValueIsNotNullTwice.kt") + public void testExpressionValueIsNotNullTwice() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNullTwice.kt"); + doTest(fileName); + } + @TestMetadata("ifNullEqualsNull.kt") public void testIfNullEqualsNull() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/nullCheckOptimization/ifNullEqualsNull.kt");