Skip to content

Commit

Permalink
KT-16245 Redundant null-check generated for a cast of already non-nul…
Browse files Browse the repository at this point in the history
…lable value

KT-16194 Code with unnecessary safe call contains redundant boxing/unboxing for primitive values
KT-12839 Two null checks are generated when manually null checking platform type

Recognize some additional cases of trivial null checks and trivial instance-of checks.

A variable is "checked for null", if it is:
- a function parameter checked with 'INVOKESTATIC kotlin/jvm/internal/Intrinsics.checkParameterIsNotNull'
- checked for nullability with 'IFNULL/IFNONNULL'
- checked for nullability with 'INSTANCEOF'
  (if objectref is instance-of T, then objectref is non-null)

Before analyzing nullability, introduce synthetic assumptions for execution branches
where a variable is guaranteed to be null or not null. For example, the following bytecode:

     ALOAD 1 // Ljava/lang/String;
     IFNULL L
     <non-null branch>
  L:
     <null branch>

is transformed to

     ALOAD 1
     IFNULL L1
     NEW java/lang/String
     ASTORE 1            // tells analyzer that variable #1 is non-null
     <non-null branch>
  L:
     <null branch>
  L1:
     ACONST_NULL
     ASTORE 1            // tells analyzer that variable #1 is null
     GOTO L

After the analysis is performed on a preprocessed method,
remember the results for "interesting" instructions
and revert the preprocessing transformations.

After that, perform bytecode transformations as usual.

Do not transform INSTANCEOF to-be-reified, because reification at call site
can introduce null checks. E.g.,

    inline fun <reified T> isNullable() = null is T
    ...
    assert(isNullable<String?>())
  • Loading branch information
dnpetrov committed Feb 15, 2017
1 parent b4e90a2 commit 2563376
Show file tree
Hide file tree
Showing 22 changed files with 522 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ private fun InstructionAdapter.generateResumeWithExceptionCheck() {

private fun Type.fieldNameForVar(index: Int) = descriptor.first() + "$" + index

private fun withInstructionAdapter(block: InstructionAdapter.() -> Unit): InsnList {
inline fun withInstructionAdapter(block: InstructionAdapter.() -> Unit): InsnList {
val tmpMethodNode = MethodNode()

InstructionAdapter(tmpMethodNode).apply(block)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class ReifiedTypeInliner(private val parametersMapping: TypeParameterMappings?)
const val REIFIED_OPERATION_MARKER_METHOD_NAME = "reifiedOperationMarker"
const val NEED_CLASS_REIFICATION_MARKER_METHOD_NAME = "needClassReification"

private fun isOperationReifiedMarker(insn: AbstractInsnNode) =
fun isOperationReifiedMarker(insn: AbstractInsnNode) =
isReifiedMarker(insn) { it == REIFIED_OPERATION_MARKER_METHOD_NAME }

private fun isReifiedMarker(insn: AbstractInsnNode, namePredicate: (String) -> Boolean): Boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,13 @@ class DeadCodeEliminationMethodTransformer : MethodTransformer() {
}

fun transformWithResult(internalClassName: String, methodNode: MethodNode): Result {
val frames = analyze(internalClassName, methodNode, OptimizationBasicInterpreter())
return removeDeadCodeByFrames(methodNode, frames)
}

fun removeDeadCodeByFrames(methodNode: MethodNode, frames: Array<out Any?>): Result {
val removedNodes = HashSet<AbstractInsnNode>()

val frames = analyze(internalClassName, methodNode, OptimizationBasicInterpreter())
val insnList = methodNode.instructions
val insnsArray = insnList.toArray()

Expand All @@ -51,6 +55,7 @@ class DeadCodeEliminationMethodTransformer : MethodTransformer() {
}

class Result(val removedNodes: Set<AbstractInsnNode>) {
fun hasRemovedAnything() = removedNodes.isNotEmpty()
fun isRemoved(node: AbstractInsnNode) = removedNodes.contains(node)
fun isAlive(node: AbstractInsnNode) = !isRemoved(node)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.jetbrains.kotlin.codegen.optimization.captured.CapturedVarsOptimizationMethodTransformer;
import org.jetbrains.kotlin.codegen.optimization.common.UtilKt;
import org.jetbrains.kotlin.codegen.optimization.nullCheck.RedundantNullCheckMethodTransformer;
import org.jetbrains.kotlin.codegen.optimization.nullCheck.RedundantNullCheckV2MethodTransformer;
import org.jetbrains.kotlin.codegen.optimization.transformer.MethodTransformer;
import org.jetbrains.org.objectweb.asm.MethodVisitor;
import org.jetbrains.org.objectweb.asm.tree.MethodNode;
Expand All @@ -35,7 +36,7 @@ public class OptimizationMethodVisitor extends TransformationMethodVisitor {

private static final MethodTransformer[] OPTIMIZATION_TRANSFORMERS = new MethodTransformer[] {
new CapturedVarsOptimizationMethodTransformer(),
new RedundantNullCheckMethodTransformer(),
new RedundantNullCheckV2MethodTransformer(),
new RedundantBoxingMethodTransformer(),
new RedundantCoercionToUnitTransformer(),
new DeadCodeEliminationMethodTransformer(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ fun <V : Value> Frame<V>.top(): V? =
peek(0)

fun <V : Value> Frame<V>.peek(offset: Int): V? =
if (stackSize >= offset) getStack(stackSize - offset - 1) else null
if (stackSize > offset) getStack(stackSize - offset - 1) else null

class SavedStackDescriptor(
val savedValues: List<BasicValue>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import org.jetbrains.org.objectweb.asm.tree.AbstractInsnNode
import org.jetbrains.org.objectweb.asm.tree.TypeInsnNode
import org.jetbrains.org.objectweb.asm.tree.analysis.BasicValue

class NullabilityV2Interpreter : OptimizationBasicInterpreter() {
class NullabilityInterpreter : OptimizationBasicInterpreter() {
override fun newOperation(insn: AbstractInsnNode): BasicValue? {
val defaultResult = super.newOperation(insn)
val resultType = defaultResult?.type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,6 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() {
}
}

private enum class Nullability {
NULL, NOT_NULL, NULLABLE
}

private fun BasicValue.getNullability(): Nullability =
when (this) {
is NullBasicValue -> Nullability.NULL
is NotNullBasicValue -> Nullability.NOT_NULL
is ProgressionIteratorBasicValue -> Nullability.NOT_NULL
else -> Nullability.NULLABLE
}

private fun isAlwaysFalse(opcode: Int, nullability: Nullability) =
(opcode == Opcodes.IFNULL && nullability == Nullability.NOT_NULL) ||
(opcode == Opcodes.IFNONNULL && nullability == Nullability.NULL)
Expand All @@ -70,7 +58,7 @@ class RedundantNullCheckMethodTransformer : MethodTransformer() {
}
if (nullCheckIfs.isEmpty()) return false

val frames = analyze(internalClassName, methodNode, NullabilityV2Interpreter())
val frames = analyze(internalClassName, methodNode, NullabilityInterpreter())

val redundantNullCheckIfs = nullCheckIfs.mapNotNull { insn ->
frames[instructions.indexOf(insn)]?.top()?.let { top ->
Expand Down
Loading

0 comments on commit 2563376

Please sign in to comment.