From 35b77e8d95127ed9d87df557be8209473d77e2f1 Mon Sep 17 00:00:00 2001 From: Vladimir Koshelev Date: Sat, 6 Jan 2018 02:53:44 +0700 Subject: [PATCH] replace DFAEngine implementation: use topological order - add createGraph(flow) to ControlFlowUtil; - use DFSTBuilder to get SCCs for a control flow; - dfa iterates all components once; - dfa iterates strongly-connected component before convergence; - deprecated method DfaInstance#isForward() is removed; - remove unused DFAEngine#performDfa(info); - remove unused ControlFlowUtil#postOrder. New dfa analysis does not have a convergence issue in merge points. (IDEA-CR-28352) --- .../controlflow/ControlFlowUtil.java | 52 ++- .../codeInsight/dataflow/DFAEngine.java | 154 ++++----- .../codeInsight/dataflow/DfaInstance.java | 5 - .../dataflow/PyReachingDefsDfaInstance.java | 4 - .../TooLargeToAnalyze.py | 297 ++++++++++++++++++ .../PyUnboundLocalVariableInspectionTest.java | 4 + 6 files changed, 403 insertions(+), 113 deletions(-) create mode 100644 python/testData/inspections/PyUnboundLocalVariableInspection/TooLargeToAnalyze.py diff --git a/platform/core-impl/src/com/intellij/codeInsight/controlflow/ControlFlowUtil.java b/platform/core-impl/src/com/intellij/codeInsight/controlflow/ControlFlowUtil.java index 3118ca348409..b9bfc71a5819 100644 --- a/platform/core-impl/src/com/intellij/codeInsight/controlflow/ControlFlowUtil.java +++ b/platform/core-impl/src/com/intellij/codeInsight/controlflow/ControlFlowUtil.java @@ -21,9 +21,13 @@ import com.intellij.psi.PsiElement; import com.intellij.util.Function; import com.intellij.util.Processor; import com.intellij.util.containers.IntStack; +import com.intellij.util.graph.Graph; import org.jetbrains.annotations.NotNull; import java.util.Arrays; +import java.util.Collection; +import java.util.Iterator; +import java.util.List; /** * @author oleg @@ -34,35 +38,27 @@ public class ControlFlowUtil { private ControlFlowUtil() { } - public static int[] postOrder(Instruction[] flow) { - final int length = flow.length; - int[] result = new int[length]; - boolean[] visited = new boolean[length]; - Arrays.fill(visited, false); - final IntStack stack = new IntStack(length); + @NotNull + public static Graph createGraph(@NotNull final Instruction[] flow) { + return new Graph() { + @NotNull + final private List myList = Arrays.asList(flow); - int N = 0; - for (int i = 0; i < length; i++) { //graph might not be connected - if (!visited[i]) { - visited[i] = true; - stack.clear(); - stack.push(i); - - while (!stack.empty()) { - final int num = stack.pop(); - result[N++] = num; - for (Instruction succ : flow[num].allSucc()) { - final int succNum = succ.num(); - if (!visited[succNum]) { - visited[succNum] = true; - stack.push(succNum); - } - } - } + @Override + public Collection getNodes() { + return myList; } - } - LOG.assertTrue(N == length); - return result; + + @Override + public Iterator getIn(Instruction n) { + return n.allPred().iterator(); + } + + @Override + public Iterator getOut(Instruction n) { + return n.allSucc().iterator(); + } + }; } public static int findInstructionNumberByElement(final Instruction[] flow, final PsiElement element){ @@ -141,7 +137,7 @@ public class ControlFlowUtil { } } - public static enum Operation { + public enum Operation { /** * CONTINUE is used to ignore previous elements processing for the node, however it doesn't stop the iteration process */ diff --git a/platform/lang-impl/src/com/intellij/codeInsight/dataflow/DFAEngine.java b/platform/lang-impl/src/com/intellij/codeInsight/dataflow/DFAEngine.java index dd038de42352..9c8fe43159ec 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/dataflow/DFAEngine.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/dataflow/DFAEngine.java @@ -18,12 +18,16 @@ import com.intellij.codeInsight.controlflow.ControlFlowUtil; import com.intellij.codeInsight.controlflow.Instruction; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.progress.ProgressManager; +import com.intellij.util.graph.DFSTBuilder; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Comparator; +import java.util.List; public class DFAEngine { private static final Logger LOG = Logger.getInstance(DFAEngine.class.getName()); - private static final double TIME_LIMIT = 10e9; // In nanoseconds, 10e9 = 1 sec + private static final long TIME_LIMIT = 1_000_000_000L; // In nanoseconds, 1_000_000_000 = 1 sec private final Instruction[] myFlow; @@ -38,99 +42,101 @@ public class DFAEngine { mySemilattice = semilattice; } - public List performDFA() throws DFALimitExceededException { final ArrayList info = new ArrayList<>(myFlow.length); - return performDFA(info); - } - - public List performDFA(final List info) throws DFALimitExceededException { if (LOG.isDebugEnabled()) { LOG.debug("Performing DFA\n" + "Instance: " + myDfa + " Semilattice: " + mySemilattice + "\nCon"); } // initializing dfa final E initial = myDfa.initial(); - for (int i = 0; i < myFlow.length; i++) { + final int length = myFlow.length; + for (int i = 0; i < length; i++) { info.add(i, initial); } - final boolean[] visited = new boolean[myFlow.length]; - - final int[] order = ControlFlowUtil.postOrder(myFlow); - -// Count limit for number of iterations per worklist +// Count limit for loops final int limit = getIterationLimit(); - int dfaCount = 0; final long startTime = System.nanoTime(); + DFSTBuilder dfsTBuilder = new DFSTBuilder<>(ControlFlowUtil.createGraph(myFlow)); - for (int i = 0; i < myFlow.length; i++) { - // Check if canceled - ProgressManager.checkCanceled(); + int[] instructionNumToNNumber = new int[myFlow.length]; + for (int i = 0; i < myFlow.length; ++i) { + instructionNumToNNumber[dfsTBuilder.getNodeByNNumber(i).num()] = i; + } + final int[] lastUpdate = new int[length]; + int count = 0; - if (System.nanoTime() - startTime > TIME_LIMIT) { - if (LOG.isDebugEnabled()) { - LOG.debug("Time limit exceeded"); - } - throw new DFALimitExceededException("Time limit exceeded"); - } - - // Iteration count per one worklist - int count = 0; - final Instruction instruction = myFlow[order[i]]; - final int number = instruction.num(); - - if (!visited[number]) { - final Queue worklist = new LinkedList<>(); - worklist.add(instruction); - visited[number] = true; - - while (true) { - // Check if canceled - ProgressManager.checkCanceled(); - - // It is essential to apply this check!!! - // This gives us more chances that resulting info will be closer to expected result - // Also it is used as indicator that "equals" method is implemented correctly in E - count++; - if (count > limit) { - if (LOG.isDebugEnabled()) { - LOG.debug("Iteration count exceeded on worklist"); - } - throw new DFALimitExceededException("Iteration count exceeded on worklist"); - } - - final Instruction currentInstruction = worklist.poll(); - if (currentInstruction == null) { - break; - } - - final int currentNumber = currentInstruction.num(); - final E oldE = info.get(currentNumber); - final E joinedE = join(currentInstruction, info); - final E newE = myDfa.fun(joinedE, currentInstruction); - if (!mySemilattice.eq(newE, oldE)) { - if (LOG.isDebugEnabled()) { - LOG.debug("Number: " + currentNumber + " old: " + oldE.toString() + " new: " + newE.toString()); - } - info.set(currentNumber, newE); - for (Instruction next : getNext(currentInstruction)) { - worklist.add(next); - visited[next.num()] = true; - } - } + List instructionsWithBackEdges = new ArrayList<>(); + for (Collection component : dfsTBuilder.getComponents()) { + List sortedInstructions = new ArrayList<>(component); + // component returns its instructions using getNodeByTNumber + // unfortunately its ordering is not suitable for dataflow goals because + // it does not start order in a SCC from entry nodes + // so We should resort nodes in a SCC by NNumber + sortedInstructions.sort(Comparator.comparingInt(it -> instructionNumToNNumber[it.num()])); + instructionsWithBackEdges.clear(); + for (Instruction instruction : sortedInstructions) { + applyTransferFunction(info, instruction); + if (instruction.allPred().stream().anyMatch(predecessor -> + instructionNumToNNumber[predecessor.num()] > instructionNumToNNumber[instruction.num()])) { + instructionsWithBackEdges.add(instruction); } } - // Move to another worklist - dfaCount += count; + int iteration = 0; + while (true) { + ++iteration; + final int currentIteration = iteration; + boolean anyUpdates = false; + for (Instruction instruction : instructionsWithBackEdges) { + if (applyTransferFunction(info, instruction)) { + lastUpdate[instruction.num()] = currentIteration; + anyUpdates = true; + count++; + } + } + if (!anyUpdates) { + break; + } + for (Instruction instruction : sortedInstructions) { + if (instruction.allPred().stream().anyMatch(it -> lastUpdate[it.num()] == currentIteration) + && applyTransferFunction(info, instruction)) { + lastUpdate[instruction.num()] = currentIteration; + count++; + } + } + + if (count > limit || (System.nanoTime() - startTime) > TIME_LIMIT) { + if (LOG.isDebugEnabled()) { + LOG.debug("Iteration count exceeded on worklist"); + } + throw new DFALimitExceededException("Iteration count exceeded on worklist"); + } + } } if (LOG.isDebugEnabled()) { - LOG.debug("Done in: " + (System.nanoTime() - startTime) / 10e6 + "ms. Ratio: " + dfaCount / myFlow.length); + LOG.debug("Done in: " + (System.nanoTime() - startTime) / 10e6 + "ms. Ratio: " + count / length); } return info; } + private boolean applyTransferFunction(List info, Instruction currentInstruction) { + ProgressManager.checkCanceled(); + final int currentNumber = currentInstruction.num(); + final E oldE = info.get(currentNumber); + final E joinedE = join(currentInstruction, info); + final E newE = myDfa.fun(joinedE, currentInstruction); + if (!mySemilattice.eq(newE, oldE)) { + if (LOG.isDebugEnabled()) { + LOG.debug("Number: " + currentNumber + " old: " + oldE.toString() + " new: " + newE.toString()); + } + info.set(currentNumber, newE); + return true; + } + return false; + } + /** * Count limit for dfa number of iterations. @@ -146,15 +152,11 @@ public class DFAEngine { } private E join(final Instruction instruction, final List info) { - final Iterable prev = myDfa.isForward() ? instruction.allPred() : instruction.allSucc(); + final Iterable prev = instruction.allPred(); final ArrayList prevInfos = new ArrayList<>(); for (Instruction i : prev) { prevInfos.add(info.get(i.num())); } return mySemilattice.join(prevInfos); } - - private Collection getNext(final Instruction curr) { - return myDfa.isForward() ? curr.allSucc() : curr.allPred(); - } } \ No newline at end of file diff --git a/platform/lang-impl/src/com/intellij/codeInsight/dataflow/DfaInstance.java b/platform/lang-impl/src/com/intellij/codeInsight/dataflow/DfaInstance.java index eea25a3e34c8..67c5b5a1a21d 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/dataflow/DfaInstance.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/dataflow/DfaInstance.java @@ -25,9 +25,4 @@ public interface DfaInstance { @NotNull E initial(); - - /** - * @deprecated - */ - boolean isForward(); } diff --git a/python/src/com/jetbrains/python/codeInsight/dataflow/PyReachingDefsDfaInstance.java b/python/src/com/jetbrains/python/codeInsight/dataflow/PyReachingDefsDfaInstance.java index d649bd9565f3..33fb9e41d128 100644 --- a/python/src/com/jetbrains/python/codeInsight/dataflow/PyReachingDefsDfaInstance.java +++ b/python/src/com/jetbrains/python/codeInsight/dataflow/PyReachingDefsDfaInstance.java @@ -107,8 +107,4 @@ public class PyReachingDefsDfaInstance implements DfaMapInstance public DFAMap initial() { return INITIAL_MAP; } - - public boolean isForward() { - return true; - } } diff --git a/python/testData/inspections/PyUnboundLocalVariableInspection/TooLargeToAnalyze.py b/python/testData/inspections/PyUnboundLocalVariableInspection/TooLargeToAnalyze.py new file mode 100644 index 000000000000..ed324b3cc4c2 --- /dev/null +++ b/python/testData/inspections/PyUnboundLocalVariableInspection/TooLargeToAnalyze.py @@ -0,0 +1,297 @@ +f1 = 10 +print(f1) + +if f1: + a1 = 1 + a2 = 2 + a3 = 3 + a4 = 4 + a5 = 5 + a6 = 6 +elif f1: + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + a1 = 1 + a2 = 2 + a3 = 3 + a4 = 4 + a5 = 5 +elif f1: + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + a1 = 1 + a2 = 2 + a3 = 3 + a4 = 4 +elif f1: + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + a1 = 1 + a2 = 2 + a3 = 3 +elif f1: + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + a1 = 1 + a2 = 2 +elif f1: + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + a1 = 1 +else: + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + print(f1) + +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) +print(f1) \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/inspections/PyUnboundLocalVariableInspectionTest.java b/python/testSrc/com/jetbrains/python/inspections/PyUnboundLocalVariableInspectionTest.java index f81cae1b6204..98bcbdc8c041 100644 --- a/python/testSrc/com/jetbrains/python/inspections/PyUnboundLocalVariableInspectionTest.java +++ b/python/testSrc/com/jetbrains/python/inspections/PyUnboundLocalVariableInspectionTest.java @@ -197,6 +197,10 @@ public class PyUnboundLocalVariableInspectionTest extends PyInspectionTestCase { doTest(); } + public void testTooLargeToAnalyze() { + doTest(); + } + @NotNull @Override protected Class getInspectionClass() {