Skip to content

Commit

Permalink
Get rid of unnecessary code for collecting scopes in a function
Browse files Browse the repository at this point in the history 
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=162774114
  • Loading branch information
@simran-arora @brad4d
simran-arora authored and brad4d committed Jul 25, 2017
1 parent 14711cc commit af9d80e
Show file tree
Hide file tree
Showing 7 changed files with 136 additions and 216 deletions.
13 changes: 4 additions & 9 deletions src/com/google/javascript/jscomp/DataFlowAnalysis.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,7 @@ abstract class DataFlowAnalysis<N, L extends LatticeElement> {
DataFlowAnalysis(ControlFlowGraph<N> targetCfg, JoinOp<L> joinOp) {
this.cfg = targetCfg;
this.joinOp = joinOp;
Comparator<DiGraphNode<N, Branch>> nodeComparator =
cfg.getOptionalNodeComparator(isForward());
Comparator<DiGraphNode<N, Branch>> nodeComparator = cfg.getOptionalNodeComparator(isForward());
if (nodeComparator != null) {
this.orderedWorkSet = new TreeSet<>(nodeComparator);
} else {
Expand Down Expand Up @@ -209,7 +208,7 @@ final void analyze(int maxSteps) {
while (!orderedWorkSet.isEmpty()) {
if (step > maxSteps) {
throw new MaxIterationsExceededException(
"Analysis did not terminate after " + maxSteps + " iterations");
"Analysis did not terminate after " + maxSteps + " iterations");
}
DiGraphNode<N, Branch> curNode = orderedWorkSet.iterator().next();
orderedWorkSet.remove(curNode);
Expand Down Expand Up @@ -421,8 +420,7 @@ protected void initialize() {
}
}

BranchedForwardDataFlowAnalysis(ControlFlowGraph<N> targetCfg,
JoinOp<L> joinOp) {
BranchedForwardDataFlowAnalysis(ControlFlowGraph<N> targetCfg, JoinOp<L> joinOp) {
super(targetCfg, joinOp);
}

Expand Down Expand Up @@ -603,8 +601,6 @@ public void visit(NodeTraversal t, Node n, Node parent) {
* 1. Exported variables as they can be needed after the script terminates.
* 2. Names of named functions because in JavaScript, function foo(){} does not kill
* foo in the dataflow.
*
*
*/
static void computeEscapedEs6(
final Scope jsScope,
Expand Down Expand Up @@ -638,9 +634,8 @@ public void visit(NodeTraversal t, Node n, Node parent) {

Map<String, Var> allVarsInFn = new HashMap<>();
List<Var> orderedVars = new LinkedList<>();
List<Scope> scopesInFunction = new LinkedList<>();
NodeUtil.getAllVarsDeclaredInFunction(
allVarsInFn, orderedVars, scopesInFunction, compiler, scopeCreator, jsScope);
allVarsInFn, orderedVars, compiler, scopeCreator, jsScope);
NodeTraversal t = new NodeTraversal(compiler, finder, scopeCreator);
t.traverseAtScope(jsScope);

Expand Down
40 changes: 21 additions & 19 deletions src/com/google/javascript/jscomp/LiveVariablesAnalysisEs6.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import javax.annotation.Nullable;

/**
* Compute the "liveness" of all local variables. A variable is "live" at a point of a program if
Expand Down Expand Up @@ -121,42 +122,44 @@ public int hashCode() {
// obtain variables in the order in which they appear in the code
private final List<Var> orderedVars;

// The scopes within a given function
private final List<Scope> scopesInFunction;

private final Map<String, Var> allVarsInFn;

/**
* ******************************************************* Live Variables Analysis using the ES6
* scope creator. This analysis should only be done on a function where jsScope is the function
* scope and jsScopeChild should be the function body scope.
* Live Variables Analysis using the ES6 scope creator. This analysis should only be done on
* function where jsScope is the function scope. If we call LiveVariablesAnalysis from the
* function scope of our pass, we can pass a null value for the JsScopeChild, but if we call it
* from the function block scope, then JsScopeChild will be the function block scope.
*
* <p>We call from the function scope when the pass requires us to traverse nodes beginning at the
* function parameters, and it from the function block scope when we are ignoring function
* parameters.
*
* @param cfg
* @param jsScope
* @param jsScopeChild
* @param jsScope the function scope
* @param jsScopeChild null or function block scope
* @param compiler
* @param scopeCreator Es6 Scope creator
*/
LiveVariablesAnalysisEs6(
ControlFlowGraph<Node> cfg,
Scope jsScope,
Scope jsScopeChild,
@Nullable Scope jsScopeChild,
AbstractCompiler compiler,
Es6SyntacticScopeCreator scopeCreator) {
super(cfg, new LiveVariableJoinOp());
checkState(jsScope.isFunctionScope(), jsScope);
checkState(jsScopeChild.isFunctionBlockScope(), jsScopeChild);

this.jsScope = jsScope;
this.jsScopeChild = jsScopeChild;
this.escaped = new HashSet<>();
this.scopeVariables = new HashMap<>();
this.allVarsInFn = new HashMap<>();
this.orderedVars = new LinkedList<>();
this.scopesInFunction = new LinkedList<>();

computeEscapedEs6(jsScope, escaped, compiler, scopeCreator);

NodeUtil.getAllVarsDeclaredInFunction(
allVarsInFn, orderedVars, scopesInFunction, compiler, scopeCreator, jsScope);
allVarsInFn, orderedVars, compiler, scopeCreator, jsScope);
addScopeVariables();
}

Expand Down Expand Up @@ -185,10 +188,6 @@ public List<Var> getAllVariablesInOrder() {
return orderedVars;
}

public List<Scope> getAllScopesInFunction() {
return scopesInFunction;
}

public int getVarIndex(String var) {
return scopeVariables.get(var);
}
Expand Down Expand Up @@ -348,7 +347,7 @@ private void addToSetIfLocal(Node node, BitSet set) {
// to the function body.
if (localScope.isFunctionBlockScope()) {
local = localScope.isDeclaredInFunctionBlockOrParameter(name);
} else if (localScope == jsScope) {
} else if (localScope == jsScope && jsScopeChild != null) {
local = jsScopeChild.isDeclaredInFunctionBlockOrParameter(name);
} else {
local = localScope.isDeclared(name, false);
Expand All @@ -358,7 +357,6 @@ private void addToSetIfLocal(Node node, BitSet set) {
return;
}


if (!escaped.contains(var)) {
set.set(getVarIndex(var.getName()));
}
Expand All @@ -377,7 +375,11 @@ void markAllParametersEscaped() {

private boolean isArgumentsName(Node n) {
boolean childDeclared;
childDeclared = jsScopeChild.isDeclared(ARGUMENT_ARRAY_ALIAS, false);
if (jsScopeChild != null) {
childDeclared = jsScopeChild.isDeclared(ARGUMENT_ARRAY_ALIAS, false);
} else {
childDeclared = true;
}
return n.isName()
&& n.getString().equals(ARGUMENT_ARRAY_ALIAS)
&& (!jsScope.isDeclared(ARGUMENT_ARRAY_ALIAS, false) || !childDeclared);
Expand Down
4 changes: 1 addition & 3 deletions src/com/google/javascript/jscomp/MaybeReachingVariableUse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ class MaybeReachingVariableUse extends
private final Set<Var> escaped;
private final Map<String, Var> allVarsInFn;
private final List<Var> orderedVars;
private final List<Scope> scopesInFunction;

MaybeReachingVariableUse(
ControlFlowGraph<Node> cfg,
Expand All @@ -62,13 +61,12 @@ class MaybeReachingVariableUse extends
this.escaped = new HashSet<>();
this.allVarsInFn = new HashMap<>();
this.orderedVars = new LinkedList<>();
this.scopesInFunction = new LinkedList<>();

// TODO(user): Maybe compute it somewhere else and re-use the escape
// local set here.
computeEscapedEs6(jsScope.getParent(), escaped, compiler, scopeCreator);
NodeUtil.getAllVarsDeclaredInFunction(
allVarsInFn, orderedVars, scopesInFunction, compiler, scopeCreator, jsScope.getParent());
allVarsInFn, orderedVars, compiler, scopeCreator, jsScope.getParent());
}

/**
Expand Down
37 changes: 18 additions & 19 deletions src/com/google/javascript/jscomp/MustBeReachingVariableDef.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ final class MustBeReachingVariableDef extends
private final Set<Var> escaped;
private final Map<String, Var> allVarsInFn;
private final List<Var> orderedVars;
private final List<Scope> scopesInFunction;

MustBeReachingVariableDef(
ControlFlowGraph<Node> cfg,
Expand All @@ -61,10 +60,9 @@ final class MustBeReachingVariableDef extends
this.escaped = new HashSet<>();
this.allVarsInFn = new HashMap<>();
this.orderedVars = new LinkedList<>();
this.scopesInFunction = new LinkedList<>();
computeEscapedEs6(jsScope.getParent(), escaped, compiler, scopeCreator);
NodeUtil.getAllVarsDeclaredInFunction(
allVarsInFn, orderedVars, scopesInFunction, compiler, scopeCreator, jsScope.getParent());
allVarsInFn, orderedVars, compiler, scopeCreator, jsScope.getParent());
}

/**
Expand Down Expand Up @@ -300,8 +298,8 @@ private void computeMustDef(
if (n.getFirstChild().isName()) {
Node name = n.getFirstChild();
computeMustDef(name.getNext(), cfgNode, output, conditional);
addToDefIfLocal(name.getString(), conditional ? null : cfgNode,
n.getLastChild(), output);
addToDefIfLocal(
name.getString(), conditional ? null : cfgNode, n.getLastChild(), output);
return;
} else if (NodeUtil.isGet(n.getFirstChild())) {
// Treat all assignments to arguments as redefining the
Expand All @@ -324,8 +322,7 @@ private void computeMustDef(
if (n.isDec() || n.isInc()) {
Node target = n.getFirstChild();
if (target.isName()) {
addToDefIfLocal(target.getString(),
conditional ? null : cfgNode, null, output);
addToDefIfLocal(target.getString(), conditional ? null : cfgNode, null, output);
return;
}
}
Expand Down Expand Up @@ -408,20 +405,22 @@ private static boolean isParameter(Var v) {
* in the def's depends set.
*/
private void computeDependence(final Definition def, Node rValue) {
NodeTraversal.traverseEs6(compiler, rValue,
NodeTraversal.traverseEs6(
compiler,
rValue,
new AbstractCfgNodeTraversalCallback() {
@Override
public void visit(NodeTraversal t, Node n, Node parent) {
if (n.isName()) {
Var dep = allVarsInFn.get(n.getString());
if (dep == null) {
def.unknownDependencies = true;
} else {
def.depends.add(dep);
@Override
public void visit(NodeTraversal t, Node n, Node parent) {
if (n.isName()) {
Var dep = allVarsInFn.get(n.getString());
if (dep == null) {
def.unknownDependencies = true;
} else {
def.depends.add(dep);
}
}
}
}
}
});
});
}

/**
Expand Down

0 comments on commit af9d80e

Please sign in to comment.