From b3d0fb82e6faa11b48816817f5be204266733a94 Mon Sep 17 00:00:00 2001 From: dimvar Date: Thu, 29 Jun 2017 09:34:22 -0700 Subject: [PATCH] [NTI] Use a static method to create the NTIWorkset. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=160535478 --- .../google/javascript/jscomp/NTIWorkset.java | 208 +++++++++--------- .../javascript/jscomp/NewTypeInference.java | 2 +- 2 files changed, 108 insertions(+), 102 deletions(-) diff --git a/src/com/google/javascript/jscomp/NTIWorkset.java b/src/com/google/javascript/jscomp/NTIWorkset.java index f5398d37d45..9cc76537838 100644 --- a/src/com/google/javascript/jscomp/NTIWorkset.java +++ b/src/com/google/javascript/jscomp/NTIWorkset.java @@ -23,6 +23,7 @@ import com.google.javascript.rhino.Node; import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.Collections; import java.util.Deque; import java.util.Iterator; import java.util.LinkedHashSet; @@ -31,33 +32,16 @@ /** * Represents the workset used by the flow-sensitive analysis in NTI. - * The workset is computed happens iteratively, otherwise large programs can cause stack overflow. + * We compute the workset iteratively, otherwise large programs can cause stack overflow. */ public class NTIWorkset { - private final ControlFlowGraph cfg; // What this class computes. Represents the workset used by the flow-sensitive analysis in NTI. - private final List> ntiWorkset; - // The algorithm that computes the NTI workset itself uses a workset. - private Deque> workset; - // If a node is in this set, don't revisit it. - private Set> seen; + private List> ntiWorkset; - NTIWorkset(ControlFlowGraph cfg) { - this.cfg = cfg; - this.ntiWorkset = new ArrayList<>(); - this.workset = new ArrayDeque<>(); - this.seen = new LinkedHashSet<>(); - buildWorkset(); - } - - private void buildWorkset() { - Preconditions.checkState(ntiWorkset.isEmpty()); - workset.push(cfg.getEntry()); - while (!workset.isEmpty()) { - processGraphNode(); - } - workset = null; - seen = null; + static NTIWorkset create(ControlFlowGraph cfg) { + NTIWorkset result = new NTIWorkset(); + result.ntiWorkset = Collections.unmodifiableList((new WorksetBuilder(cfg)).build()); + return result; } Iterable> forward() { @@ -65,9 +49,7 @@ Iterable> forward() { return ntiWorkset; } - /** - * The backwards analysis in NTI traverses the workset in the reverse direction. - */ + /** The backwards analysis in NTI traverses the workset in the reverse direction. */ private class BackwardIterator implements Iterator> { int i = ntiWorkset.size() - 1; @@ -97,94 +79,118 @@ public Iterator> iterator() { }; } - private void processGraphNode() { - DiGraphNode dn = workset.pop(); - if (seen.contains(dn) || dn == cfg.getImplicitReturn()) { - return; + private static class WorksetBuilder { + private final ControlFlowGraph cfg; + private List> ntiWorkset; + // The algorithm that computes the NTI workset itself uses a workset. + private Deque> workset; + // If a node is in this set, don't revisit it. + private Set> seen; + + WorksetBuilder(ControlFlowGraph cfg) { + this.cfg = cfg; } - switch (dn.getValue().getToken()) { - case DO: - case WHILE: - case FOR: - case FOR_IN: - case FOR_OF: { - List> outEdges = dn.getOutEdges(); - // The workset is a stack. If we want to analyze nodeA after nodeB, we need to push nodeA - // before nodeB. For this reason, we push the code after a loop before the loop body. - for (DiGraphEdge outEdge : outEdges) { - if (outEdge.getValue() == ControlFlowGraph.Branch.ON_FALSE) { - workset.push(outEdge.getDestination()); - } - } - for (DiGraphEdge outEdge : outEdges) { - if (outEdge.getValue() == ControlFlowGraph.Branch.ON_TRUE) { - workset.push(outEdge.getDestination()); - } - } - // The loop condition must be analyzed first, so it's pushed last. - seen.add(dn); - ntiWorkset.add(dn); + + List> build() { + ntiWorkset = new ArrayList<>(); + workset = new ArrayDeque<>(); + seen = new LinkedHashSet<>(); + workset.push(cfg.getEntry()); + while (!workset.isEmpty()) { + processGraphNode(); + } + return ntiWorkset; + } + + private void processGraphNode() { + DiGraphNode dn = workset.pop(); + if (seen.contains(dn) || dn == cfg.getImplicitReturn()) { return; } - default: { - for (DiGraphEdge inEdge : dn.getInEdges()) { - DiGraphNode source = inEdge.getSource(); - Node sourceNode = source.getValue(); - // Wait for all other incoming edges at join nodes. - if (!seen.contains(inEdge.getSource()) && !sourceNode.isDo()) { - return; + switch (dn.getValue().getToken()) { + case DO: + case WHILE: + case FOR: + case FOR_IN: + case FOR_OF: { + List> outEdges = dn.getOutEdges(); + // The workset is a stack. If we want to analyze nodeA after nodeB, we need to push nodeA + // before nodeB. For this reason, we push the code after a loop before the loop body. + for (DiGraphEdge outEdge : outEdges) { + if (outEdge.getValue() == ControlFlowGraph.Branch.ON_FALSE) { + workset.push(outEdge.getDestination()); + } } - // The loop header has already been added, and will be analyzed before the loop body. - // Here, we want to add it again, so that we analyze the header after the loop body, - // and before the code following the loop. - if (NodeUtil.isLoopStructure(sourceNode) && !sourceNode.isDo() - && inEdge.getValue() == ControlFlowGraph.Branch.ON_FALSE) { - ntiWorkset.add(source); + for (DiGraphEdge outEdge : outEdges) { + if (outEdge.getValue() == ControlFlowGraph.Branch.ON_TRUE) { + workset.push(outEdge.getDestination()); + } } - } - seen.add(dn); - if (cfg.getEntry() != dn) { + // The loop condition must be analyzed first, so it's pushed last. + seen.add(dn); ntiWorkset.add(dn); + return; } - Node n = dn.getValue(); - List> succs = cfg.getDirectedSuccNodes(dn); - // Currently, the ELSE branch of an IF is analyzed before the THEN branch. - // To do it the other way around, the ELSE branch has to be pushed to the workset - // *before* the THEN branch, so we need to reverse succs. But the order doesn't impact - // correctness, so we don't do the reversal. - for (DiGraphNode succ : succs) { - workset.push(succ); - if (succ == cfg.getImplicitReturn()) { - if (n.getNext() != null) { - processDeadNode(n.getNext()); + default: { + for (DiGraphEdge inEdge : dn.getInEdges()) { + DiGraphNode source = inEdge.getSource(); + Node sourceNode = source.getValue(); + // Wait for all other incoming edges at join nodes. + if (!seen.contains(inEdge.getSource()) && !sourceNode.isDo()) { + return; + } + // The loop header has already been added, and will be analyzed before the loop body. + // Here, we want to add it again, so that we analyze the header after the loop body, + // and before the code following the loop. + if (NodeUtil.isLoopStructure(sourceNode) && !sourceNode.isDo() + && inEdge.getValue() == ControlFlowGraph.Branch.ON_FALSE) { + ntiWorkset.add(source); } } - } - if (n.isTry()) { - processDeadNode(n.getSecondChild()); - } else if (n.isBreak() || n.isContinue() || n.isThrow()) { - processDeadNode(n.getNext()); + seen.add(dn); + if (cfg.getEntry() != dn) { + ntiWorkset.add(dn); + } + Node n = dn.getValue(); + List> succs = cfg.getDirectedSuccNodes(dn); + // Currently, the ELSE branch of an IF is analyzed before the THEN branch. + // To do it the other way around, the ELSE branch has to be pushed to the workset + // *before* the THEN branch, so we need to reverse succs. But the order doesn't impact + // correctness, so we don't do the reversal. + for (DiGraphNode succ : succs) { + workset.push(succ); + if (succ == cfg.getImplicitReturn()) { + if (n.getNext() != null) { + processDeadNode(n.getNext()); + } + } + } + if (n.isTry()) { + processDeadNode(n.getSecondChild()); + } else if (n.isBreak() || n.isContinue() || n.isThrow()) { + processDeadNode(n.getNext()); + } } } } - } - /** - * Analyze dead code, such as a catch that is never executed or a statement following a - * return/break/continue. This code can be a predecessor of live code in the cfg. We wait - * on incoming edges before adding nodes to the workset, and don't want dead code to block - * live code from being analyzed. - */ - private void processDeadNode(Node maybeDeadNode) { - if (maybeDeadNode == null) { - return; - } - DiGraphNode cfgNode = cfg.getDirectedGraphNode(maybeDeadNode); - if (cfgNode == null) { - return; - } - if (cfg.getDirectedPredNodes(cfgNode).isEmpty()) { - workset.push(cfgNode); + /** + * Analyze dead code, such as a catch that is never executed or a statement following a + * return/break/continue. This code can be a predecessor of live code in the cfg. We wait + * on incoming edges before adding nodes to the workset, and don't want dead code to block + * live code from being analyzed. + */ + private void processDeadNode(Node maybeDeadNode) { + if (maybeDeadNode == null) { + return; + } + DiGraphNode cfgNode = cfg.getDirectedGraphNode(maybeDeadNode); + if (cfgNode == null) { + return; + } + if (cfg.getDirectedPredNodes(cfgNode).isEmpty()) { + workset.push(cfgNode); + } } } } diff --git a/src/com/google/javascript/jscomp/NewTypeInference.java b/src/com/google/javascript/jscomp/NewTypeInference.java index c73165bb02d..0783f482272 100644 --- a/src/com/google/javascript/jscomp/NewTypeInference.java +++ b/src/com/google/javascript/jscomp/NewTypeInference.java @@ -755,7 +755,7 @@ private void analyzeFunction(NTIScope scope) { println(this.cfg); // The size is > 1 when multiple files are compiled // Preconditions.checkState(cfg.getEntry().getOutEdges().size() == 1); - NTIWorkset workset = new NTIWorkset(this.cfg); + NTIWorkset workset = NTIWorkset.create(this.cfg); /* println("Workset: ", workset); */ this.typeEnvFromDeclaredTypes = getTypeEnvFromDeclaredTypes(); if (scope.isFunction() && scope.hasUndeclaredFormalsOrOuters()) {