From 620be20ea6d0965fd3406a1d03078d450fd591e4 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 17 May 2018 12:33:50 +0700 Subject: [PATCH] Control transfer handling refactoring; responsibilities tuned --- .../dataFlow/DataFlowRunner.java | 4 +- .../dataFlow/InstructionVisitor.java | 7 +- .../codeInspection/dataFlow/LoopAnalyzer.java | 1 + .../dataFlow/controlTransfer.kt | 150 ++++++++---------- .../ControlTransferInstruction.kt | 24 +++ .../instructions/ReturnInstruction.java | 1 - 6 files changed, 97 insertions(+), 90 deletions(-) create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ControlTransferInstruction.kt diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowRunner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowRunner.java index 2f4983959660..91becb3d5334 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowRunner.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowRunner.java @@ -24,6 +24,7 @@ import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.MultiMap; import com.siyeh.ig.psiutils.VariableAccessUtils; import gnu.trove.THashSet; +import one.util.streamex.IntStreamEx; import one.util.streamex.StreamEx; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -271,7 +272,8 @@ public class DataFlowRunner { } else if (instruction instanceof ConditionalGotoInstruction) { joinInstructions.add(myInstructions[((ConditionalGotoInstruction)instruction).getOffset()]); } else if (instruction instanceof ControlTransferInstruction) { - joinInstructions.addAll(((ControlTransferInstruction)instruction).getPossibleTargetInstructions(myInstructions)); + IntStreamEx.of(((ControlTransferInstruction)instruction).getPossibleTargetIndices()).elements(myInstructions) + .into(joinInstructions); } else if (instruction instanceof MethodCallInstruction && !((MethodCallInstruction)instruction).getContracts().isEmpty()) { joinInstructions.add(myInstructions[index + 1]); } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java index a5a26494dc10..52cc2fa8131a 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java @@ -66,12 +66,7 @@ public abstract class InstructionVisitor { @NotNull public DfaInstructionState[] visitControlTransfer(@NotNull ControlTransferInstruction controlTransferInstruction, @NotNull DataFlowRunner runner, @NotNull DfaMemoryState state) { - DfaControlTransferValue transferValue = controlTransferInstruction.getTransfer(); - if (transferValue.getTarget() instanceof ExitFinallyTransfer) { - transferValue = (DfaControlTransferValue)state.pop(); - } - return new ControlTransferHandler(state, runner, transferValue.getTarget()).iteration(transferValue.getTraps()) - .toArray(DfaInstructionState.EMPTY_ARRAY); + return controlTransferInstruction.getTransfer().dispatch(state, runner).toArray(DfaInstructionState.EMPTY_ARRAY); } public DfaInstructionState[] visitEndOfInitializer(EndOfInitializerInstruction instruction, DataFlowRunner runner, DfaMemoryState state) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/LoopAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/LoopAnalyzer.java index 991fb7e1debe..3d78b3bdb6a9 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/LoopAnalyzer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/LoopAnalyzer.java @@ -16,6 +16,7 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInspection.dataFlow.instructions.ConditionalGotoInstruction; +import com.intellij.codeInspection.dataFlow.instructions.ControlTransferInstruction; import com.intellij.codeInspection.dataFlow.instructions.GotoInstruction; import com.intellij.codeInspection.dataFlow.instructions.Instruction; import com.intellij.util.ArrayUtil; diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/controlTransfer.kt b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/controlTransfer.kt index d7b7bf4570f0..17c0fa808114 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/controlTransfer.kt +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/controlTransfer.kt @@ -17,7 +17,7 @@ package com.intellij.codeInspection.dataFlow -import com.intellij.codeInspection.dataFlow.instructions.Instruction +import com.intellij.codeInspection.dataFlow.instructions.ControlTransferInstruction import com.intellij.codeInspection.dataFlow.value.DfaPsiType import com.intellij.codeInspection.dataFlow.value.DfaValue import com.intellij.codeInspection.dataFlow.value.DfaValueFactory @@ -33,52 +33,58 @@ import kotlin.collections.ArrayList class DfaControlTransferValue(factory: DfaValueFactory, val target: TransferTarget, val traps: FList) : DfaValue(factory) { + fun dispatch(state: DfaMemoryState, runner: DataFlowRunner) = ControlTransferHandler(state, runner, this).dispatch() override fun toString() = target.toString() + (if (traps.isEmpty()) "" else " $traps") } interface TransferTarget { + /** @return list of possible instruction offsets for given target */ fun getPossibleTargets() : Collection = emptyList() + /** @return next instruction states assuming no traps */ + fun dispatch(state: DfaMemoryState, runner: DataFlowRunner) : List = emptyList() } data class ExceptionTransfer(val throwable: DfaPsiType?) : TransferTarget { override fun toString() = "Exception($throwable)" } -data class InstructionTransfer(val offset: ControlFlow.ControlFlowOffset, val toFlush: List) : TransferTarget { - override fun getPossibleTargets() = listOf(offset.instructionOffset) +data class InstructionTransfer(val offset: ControlFlow.ControlFlowOffset, private val toFlush: List) : TransferTarget { + override fun dispatch(state: DfaMemoryState, runner: DataFlowRunner): List { + toFlush.forEach(state::flushVariable) + return listOf(DfaInstructionState(runner.getInstruction(offset.instructionOffset), state)) + } + override fun getPossibleTargets() = listOf(offset.instructionOffset) override fun toString() = "-> $offset" + (if (toFlush.isEmpty()) "" else "; flushing $toFlush") } data class ExitFinallyTransfer(private val enterFinally: Trap.EnterFinally) : TransferTarget { override fun getPossibleTargets() = enterFinally.backLinks.asIterable().flatMap { it.getPossibleTargetIndices() } .filter { index -> index != enterFinally.jumpOffset.instructionOffset }.toSet() + override fun dispatch(state: DfaMemoryState, runner: DataFlowRunner): List { + return (state.pop() as DfaControlTransferValue).dispatch(state, runner) + } + override fun toString() = "ExitFinally" } object ReturnTransfer : TransferTarget { override fun toString(): String = "Return" } -open class ControlTransferInstruction(val transfer: DfaControlTransferValue) : Instruction() { - init { - transfer.traps.forEach { trap -> trap.link(this) } - } - - override fun accept(runner: DataFlowRunner, state: DfaMemoryState, visitor: InstructionVisitor): Array { - return visitor.visitControlTransfer(this, runner, state) - } - - fun getPossibleTargetIndices() = transfer.traps.flatMap(Trap::getPossibleTargets) + transfer.target.getPossibleTargets() - - fun getPossibleTargetInstructions(allInstructions: Array) = getPossibleTargetIndices().map { allInstructions[it] } - - override fun toString() = "TRANSFER $transfer [targets: ${getPossibleTargetIndices()}]" -} - sealed class Trap(val anchor: PsiElement) { open fun link(instruction: ControlTransferInstruction) {} + internal abstract fun dispatch(handler: ControlTransferHandler): List + internal open fun getPossibleTargets(): Collection = emptyList() + override fun toString() = javaClass.simpleName!! + class TryCatch(tryStatement: PsiTryStatement, val clauses: LinkedHashMap) : Trap(tryStatement) { - override fun toString() = "TryCatch -> ${clauses.values}" + override fun dispatch(handler: ControlTransferHandler): List { + return if (handler.target is ExceptionTransfer) handler.processCatches(handler.target.throwable, clauses) + else handler.dispatch() + } + + override fun getPossibleTargets() = clauses.values.map { it.instructionOffset } + override fun toString() = "${super.toString()} -> ${clauses.values}" } abstract class EnterFinally(anchor: PsiElement, val jumpOffset: ControlFlow.ControlFlowOffset): Trap(anchor) { internal val backLinks = ArrayList() @@ -87,81 +93,62 @@ sealed class Trap(val anchor: PsiElement) { backLinks.add(instruction) } - override fun toString() = "${javaClass.simpleName} -> $jumpOffset" + override fun dispatch(handler: ControlTransferHandler): List { + handler.state.push(handler.runner.factory.controlTransfer(handler.target, handler.traps)) + return listOf(DfaInstructionState(handler.runner.getInstruction(jumpOffset.instructionOffset), handler.state)) + } + + override fun getPossibleTargets() = listOf(jumpOffset.instructionOffset) + override fun toString() = "${super.toString()} -> $jumpOffset" } class TryFinally(finallyBlock: PsiCodeBlock, jumpOffset: ControlFlow.ControlFlowOffset): EnterFinally(finallyBlock, jumpOffset) - class TwrFinally(resourceList: PsiResourceList, jumpOffset: ControlFlow.ControlFlowOffset): EnterFinally(resourceList, jumpOffset) + class TwrFinally(resourceList: PsiResourceList, jumpOffset: ControlFlow.ControlFlowOffset) : EnterFinally(resourceList, jumpOffset) { + override fun dispatch(handler: ControlTransferHandler) = + if (handler.target is ExceptionTransfer) handler.dispatch() + else super.dispatch(handler) + } class InsideFinally(finallyBlock: PsiElement): Trap(finallyBlock) { - override fun toString() = "InsideFinally" + override fun dispatch(handler: ControlTransferHandler): List { + handler.state.pop() as DfaControlTransferValue + return handler.dispatch() + } } class InsideInlinedBlock(block: PsiCodeBlock): Trap(block) { - override fun toString() = "InsideInlinedBlock" - } - - internal fun getPossibleTargets(): Collection { - return when (this) { - is TryCatch -> clauses.values.map { it.instructionOffset } - is EnterFinally -> listOf(jumpOffset.instructionOffset) - else -> emptyList() + override fun dispatch(handler: ControlTransferHandler): List { + (handler.state.pop() as DfaControlTransferValue).target as ReturnTransfer + return handler.dispatch() } } } -private class ControlTransferHandler(val state: DfaMemoryState, val runner: DataFlowRunner, val target: TransferTarget) { - var throwableState: DfaVariableState? = null +internal class ControlTransferHandler(val state: DfaMemoryState, val runner: DataFlowRunner, transferValue: DfaControlTransferValue) { + private var throwableType: TypeConstraint? = null + val target = transferValue.target + var traps = transferValue.traps - fun iteration(traps: FList): List { - val (head, tail) = traps.head to traps.tail + fun dispatch(): List { + val head = traps.head + traps = traps.tail ?: FList.emptyList() state.emptyStack() - return when (head) { - null -> transferToTarget() - is Trap.TryCatch -> if (target is ExceptionTransfer) processCatches(head, target.throwable, tail) else iteration(tail) - is Trap.TryFinally -> goToFinally(head.jumpOffset.instructionOffset, tail) - is Trap.TwrFinally -> if (target is ExceptionTransfer) iteration(tail) else goToFinally(head.jumpOffset.instructionOffset, tail) - is Trap.InsideFinally -> leaveFinally(tail) - is Trap.InsideInlinedBlock -> { - assert((state.pop() as DfaControlTransferValue).target === ReturnTransfer) - iteration(tail) - } - else -> throw InternalError("Impossible") - } + return head?.dispatch(this) ?: target.dispatch(state, runner) } - private fun transferToTarget(): List { - return when (target) { - is InstructionTransfer -> { - target.toFlush.forEach { state.flushVariable(it) } - listOf(DfaInstructionState(runner.getInstruction(target.offset.instructionOffset), state)) - } - else -> emptyList() - } - } - - private fun goToFinally(offset: Int, traps: FList): List { - state.push(runner.factory.controlTransfer(target, traps)) - return listOf(DfaInstructionState(runner.getInstruction(offset), state)) - } - - private fun leaveFinally(traps: FList): List { - state.pop() as DfaControlTransferValue - return iteration(traps) - } - - private fun processCatches(tryCatch: Trap.TryCatch, thrownValue: DfaPsiType?, traps: FList): List { + internal fun processCatches(thrownValue: DfaPsiType?, + catches: Map): List { val result = arrayListOf() - for ((catchSection, jumpOffset) in tryCatch.clauses) { + for ((catchSection, jumpOffset) in catches) { val param = catchSection.parameter ?: continue - if (throwableState == null) throwableState = initVariableState(param, thrownValue) + if (throwableType == null) throwableType = createConstraint(thrownValue) for (caughtType in allCaughtTypes(param)) { - throwableState?.withInstanceofValue(caughtType)?.let { varState -> - result.add(DfaInstructionState(runner.getInstruction(jumpOffset.instructionOffset), stateForCatchClause(param, varState))) + throwableType?.withInstanceofValue(caughtType)?.let { constraint -> + result.add(DfaInstructionState(runner.getInstruction(jumpOffset.instructionOffset), stateForCatchClause(param, constraint))) } - throwableState = throwableState?.withNotInstanceofValue(caughtType) ?: return result + throwableType = throwableType?.withNotInstanceofValue(caughtType) ?: return result } } - return result + iteration(traps) + return result + dispatch() } private fun allCaughtTypes(param: PsiParameter): List { @@ -169,16 +156,15 @@ private class ControlTransferHandler(val state: DfaMemoryState, val runner: Data return psiTypes.map { runner.factory.createDfaType(it) } } - private fun stateForCatchClause(param: PsiParameter, varState: DfaVariableState): DfaMemoryState { - val catchingCopy = state.createCopy() as DfaMemoryStateImpl - catchingCopy.setVariableState(catchingCopy.factory.varFactory.createVariableValue(param), varState) + private fun stateForCatchClause(param: PsiParameter, constraint: TypeConstraint): DfaMemoryState { + val catchingCopy = state.createCopy() + val value = runner.factory.varFactory.createVariableValue(param) + catchingCopy.applyFact(value, DfaFactType.TYPE_CONSTRAINT, constraint) + catchingCopy.applyFact(value, DfaFactType.CAN_BE_NULL, false) return catchingCopy } - private fun initVariableState(param: PsiParameter, throwable: DfaPsiType?): DfaVariableState { - val sampleVar = (state as DfaMemoryStateImpl).factory.varFactory.createVariableValue(param) - val varState = state.createVariableState(sampleVar).withFact(DfaFactType.CAN_BE_NULL, false) - return if (throwable != null) varState.withInstanceofValue(throwable)!! else varState + private fun createConstraint(throwable: DfaPsiType?): TypeConstraint { + return if (throwable != null) TypeConstraint.EMPTY.withInstanceofValue(throwable)!! else TypeConstraint.EMPTY } - } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ControlTransferInstruction.kt b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ControlTransferInstruction.kt new file mode 100644 index 000000000000..d8668be90dba --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ControlTransferInstruction.kt @@ -0,0 +1,24 @@ +// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package com.intellij.codeInspection.dataFlow.instructions + +import com.intellij.codeInspection.dataFlow.* + +/** + * Instruction which performs complex control transfer (handling exception; processing finally blocks; exiting inlined lambda, etc.) + */ +open class ControlTransferInstruction(val transfer: DfaControlTransferValue) : Instruction() { + init { + transfer.traps.forEach { trap -> trap.link(this) } + } + + override fun accept(runner: DataFlowRunner, state: DfaMemoryState, visitor: InstructionVisitor): Array { + return visitor.visitControlTransfer(this, runner, state) + } + + /** + * Returns list of possible target instruction indices + */ + fun getPossibleTargetIndices() = transfer.traps.flatMap(Trap::getPossibleTargets) + transfer.target.getPossibleTargets() + + override fun toString() = "TRANSFER $transfer [targets: ${getPossibleTargetIndices()}]" +} \ No newline at end of file diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ReturnInstruction.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ReturnInstruction.java index 1c6f2dc9e3fd..9724246f71ff 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ReturnInstruction.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ReturnInstruction.java @@ -16,7 +16,6 @@ package com.intellij.codeInspection.dataFlow.instructions; -import com.intellij.codeInspection.dataFlow.ControlTransferInstruction; import com.intellij.codeInspection.dataFlow.DfaControlTransferValue; import com.intellij.codeInspection.dataFlow.ExceptionTransfer; import com.intellij.psi.PsiElement;