Refine redundant null checks interpreter
The main change here is addition of a check that NotNullBasicValue instances
are not being created for non-reference types
Exactly this change should be used instead of f25f0db10e
The latter commit lead to problem described in the KT-14242 issue:
v.getType().getSort() == w.getType().getSort() && (v.getType().getSort() != Type.OBJECT || v.equals(w))
Problem is that the condition above returns true without calling `v.equals(w)`,
because the sort of type is ARRAY, not OBJECT, so testArray was being treated
as NotNullable erroneously
So the second part of this change is effectively revering mentioned commit
#KT-14242 Fixed
This commit is contained in:
+9
-4
@@ -16,12 +16,12 @@
|
|||||||
|
|
||||||
package org.jetbrains.kotlin.codegen.optimization.boxing
|
package org.jetbrains.kotlin.codegen.optimization.boxing
|
||||||
|
|
||||||
import org.jetbrains.org.objectweb.asm.tree.InsnList
|
|
||||||
import org.jetbrains.org.objectweb.asm.tree.analysis.BasicValue
|
|
||||||
import org.jetbrains.kotlin.codegen.optimization.common.BasicValueWrapper
|
import org.jetbrains.kotlin.codegen.optimization.common.BasicValueWrapper
|
||||||
|
import org.jetbrains.org.objectweb.asm.Opcodes
|
||||||
import org.jetbrains.org.objectweb.asm.Type
|
import org.jetbrains.org.objectweb.asm.Type
|
||||||
import org.jetbrains.org.objectweb.asm.tree.AbstractInsnNode
|
import org.jetbrains.org.objectweb.asm.tree.AbstractInsnNode
|
||||||
import org.jetbrains.org.objectweb.asm.Opcodes
|
import org.jetbrains.org.objectweb.asm.tree.InsnList
|
||||||
|
import org.jetbrains.org.objectweb.asm.tree.analysis.BasicValue
|
||||||
|
|
||||||
class NullabilityInterpreter(insns: InsnList) : BoxingInterpreter(insns) {
|
class NullabilityInterpreter(insns: InsnList) : BoxingInterpreter(insns) {
|
||||||
override fun unaryOperation(insn: AbstractInsnNode, value: BasicValue) = makeNotNullIfNeeded(insn, super.unaryOperation(insn, value))
|
override fun unaryOperation(insn: AbstractInsnNode, value: BasicValue) = makeNotNullIfNeeded(insn, super.unaryOperation(insn, value))
|
||||||
@@ -36,7 +36,12 @@ class NullabilityInterpreter(insns: InsnList) : BoxingInterpreter(insns) {
|
|||||||
|
|
||||||
private fun makeNotNullIfNeeded(insn: AbstractInsnNode, value: BasicValue?): BasicValue? =
|
private fun makeNotNullIfNeeded(insn: AbstractInsnNode, value: BasicValue?): BasicValue? =
|
||||||
when (insn.opcode) {
|
when (insn.opcode) {
|
||||||
Opcodes.ANEWARRAY, Opcodes.NEWARRAY, Opcodes.LDC, Opcodes.NEW -> NotNullBasicValue(value)
|
Opcodes.ANEWARRAY, Opcodes.NEWARRAY, Opcodes.LDC, Opcodes.NEW ->
|
||||||
|
if (value?.type?.sort == Type.OBJECT || value?.type?.sort == Type.ARRAY)
|
||||||
|
NotNullBasicValue(value)
|
||||||
|
else
|
||||||
|
value
|
||||||
|
|
||||||
else -> value
|
else -> value
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+18
-17
@@ -86,26 +86,27 @@ public class OptimizationBasicInterpreter extends BasicInterpreter {
|
|||||||
public BasicValue merge(
|
public BasicValue merge(
|
||||||
@NotNull BasicValue v, @NotNull BasicValue w
|
@NotNull BasicValue v, @NotNull BasicValue w
|
||||||
) {
|
) {
|
||||||
if (v == BasicValue.UNINITIALIZED_VALUE || w == BasicValue.UNINITIALIZED_VALUE) {
|
if (!v.equals(w)) {
|
||||||
|
if (v == BasicValue.UNINITIALIZED_VALUE || w == BasicValue.UNINITIALIZED_VALUE) {
|
||||||
|
return BasicValue.UNINITIALIZED_VALUE;
|
||||||
|
}
|
||||||
|
|
||||||
|
// if merge of two references then `lub` is java/lang/Object
|
||||||
|
// arrays also are BasicValues with reference type's
|
||||||
|
if (isReference(v) && isReference(w)) {
|
||||||
|
return BasicValue.REFERENCE_VALUE;
|
||||||
|
}
|
||||||
|
|
||||||
|
// if merge of something can be stored in int var (int, char, boolean, byte, character)
|
||||||
|
if (v.getType().getOpcode(Opcodes.ISTORE) == Opcodes.ISTORE &&
|
||||||
|
w.getType().getOpcode(Opcodes.ISTORE) == Opcodes.ISTORE) {
|
||||||
|
return BasicValue.INT_VALUE;
|
||||||
|
}
|
||||||
|
|
||||||
return BasicValue.UNINITIALIZED_VALUE;
|
return BasicValue.UNINITIALIZED_VALUE;
|
||||||
}
|
}
|
||||||
// Objects must be equal, others can just have the same sort
|
|
||||||
if (v.getType().getSort() == w.getType().getSort() && (v.getType().getSort() != Type.OBJECT || v.equals(w))) {
|
|
||||||
return v;
|
|
||||||
}
|
|
||||||
|
|
||||||
// if merge of two references then `lub` is java/lang/Object
|
return v;
|
||||||
if (isReference(v) && isReference(w)) {
|
|
||||||
return BasicValue.REFERENCE_VALUE;
|
|
||||||
}
|
|
||||||
|
|
||||||
// if merge of something can be stored in int var (int, char, boolean, byte, character)
|
|
||||||
if (v.getType().getOpcode(Opcodes.ISTORE) == Opcodes.ISTORE &&
|
|
||||||
w.getType().getOpcode(Opcodes.ISTORE) == Opcodes.ISTORE) {
|
|
||||||
return BasicValue.INT_VALUE;
|
|
||||||
}
|
|
||||||
|
|
||||||
return BasicValue.UNINITIALIZED_VALUE;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private static boolean isReference(@NotNull BasicValue v) {
|
private static boolean isReference(@NotNull BasicValue v) {
|
||||||
|
|||||||
@@ -0,0 +1,13 @@
|
|||||||
|
// See KT-14242
|
||||||
|
var x = 1
|
||||||
|
fun box(): String {
|
||||||
|
val testArray: Array<String?>? = when (1) {
|
||||||
|
x -> null
|
||||||
|
else -> arrayOfNulls<String>(0)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Must not be NPE here
|
||||||
|
val size = testArray?.size
|
||||||
|
|
||||||
|
return size?.toString() ?: "OK"
|
||||||
|
}
|
||||||
@@ -13639,6 +13639,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
|
|||||||
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/regressions"), Pattern.compile("^(.+)\\.kt$"), true);
|
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/regressions"), Pattern.compile("^(.+)\\.kt$"), true);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@TestMetadata("arrayLengthNPE.kt")
|
||||||
|
public void testArrayLengthNPE() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/regressions/arrayLengthNPE.kt");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
|
|
||||||
@TestMetadata("collections.kt")
|
@TestMetadata("collections.kt")
|
||||||
public void testCollections() throws Exception {
|
public void testCollections() throws Exception {
|
||||||
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/regressions/collections.kt");
|
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/regressions/collections.kt");
|
||||||
|
|||||||
Reference in New Issue
Block a user