Skip to content

Commit

Permalink
Refactored the ScopeCreator within the compiler
Browse files Browse the repository at this point in the history
Methods that use it have an additional parameter - they either take in
the old scope creator or the new ES6 scope creator depending on which
one they use in their classes.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=158028965
  • Loading branch information
simran-arora authored and brad4d committed Jun 5, 2017
1 parent 2c34d22 commit 51bd73b
Show file tree
Hide file tree
Showing 12 changed files with 53 additions and 35 deletions.
Expand Up @@ -32,7 +32,6 @@
import com.google.javascript.rhino.jstype.JSType;
import com.google.javascript.rhino.jstype.ObjectType;
import com.google.javascript.rhino.jstype.UnionType;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -1249,7 +1248,7 @@ public void enterScope(NodeTraversal t) {
*/
ControlFlowGraph<Node> cfg = t.getControlFlowGraph();
LiveVariablesAnalysis liveness =
new LiveVariablesAnalysis(cfg, t.getTypedScope(), compiler);
new LiveVariablesAnalysis(cfg, t.getTypedScope(), compiler, t.getScopeCreator());
liveness.analyze();

for (TypedVar v : ((Set<TypedVar>) liveness.getEscapedLocals())) {
Expand Down
3 changes: 2 additions & 1 deletion src/com/google/javascript/jscomp/CoalesceVariableNames.java
Expand Up @@ -110,7 +110,8 @@ public void enterScope(NodeTraversal t) {
Preconditions.checkState(scope.isFunctionScope(), scope);

ControlFlowGraph<Node> cfg = t.getControlFlowGraph();
LiveVariablesAnalysis liveness = new LiveVariablesAnalysis(cfg, scope, compiler);
LiveVariablesAnalysis liveness =
new LiveVariablesAnalysis(cfg, scope, compiler, t.getScopeCreator());
if (compiler.getOptions().getLanguageOut() == CompilerOptions.LanguageMode.ECMASCRIPT3) {
// If the function has exactly 2 params, mark them as escaped. This is a work-around for a
// bug in IE 8 and below, where it throws an exception if you write to the parameters of the
Expand Down
31 changes: 17 additions & 14 deletions src/com/google/javascript/jscomp/DataFlowAnalysis.java
Expand Up @@ -540,20 +540,24 @@ public int hashCode() {
}

/**
* Compute set of escaped variables. When a variable is escaped in a
* dataflow analysis, it can be reference outside of the code that we are
* analyzing. A variable is escaped if any of the following is true:
* Compute set of escaped variables. When a variable is escaped in a dataflow analysis, it can be
* reference outside of the code that we are analyzing. A variable is escaped if any of the
* following is true:
*
* <p><ol>
* <li>It is defined as the exception name in CATCH clause so it became a
* variable local not to our definition of scope.</li>
* <li>Exported variables as they can be needed after the script terminates.
* </li>
* <li>Names of named functions because in JavaScript, <i>function foo(){}</i>
* does not kill <i>foo</i> in the dataflow.</li>
* <p>
*
* <ol>
* <li>It is defined as the exception name in CATCH clause so it became a variable local not to
* our definition of scope.
* <li>Exported variables as they can be needed after the script terminates.
* <li>Names of named functions because in JavaScript, <i>function foo(){}</i> does not kill
* <i>foo</i> in the dataflow.
*/
static void computeEscaped(final Scope jsScope, final Set<Var> escaped,
AbstractCompiler compiler) {
static void computeEscaped(
final Scope jsScope,
final Set<Var> escaped,
AbstractCompiler compiler,
ScopeCreator scopeCreator) {
// TODO(user): Very good place to store this information somewhere.
AbstractPostOrderCallback finder = new AbstractPostOrderCallback() {
@Override
Expand All @@ -570,8 +574,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
}
};

NodeTraversal t =
new NodeTraversal(compiler, finder, SyntacticScopeCreator.makeUntyped(compiler));
NodeTraversal t = new NodeTraversal(compiler, finder, scopeCreator);
t.traverseAtScope(jsScope);

// 1: Remove the exception name in CATCH which technically isn't local to
Expand Down
Expand Up @@ -123,7 +123,12 @@ private void eliminateDeadAssignments(NodeTraversal t) {

// Computes liveness information first.
ControlFlowGraph<Node> cfg = t.getControlFlowGraph();
liveness = new LiveVariablesAnalysis(cfg, functionScope, compiler);
/*TODO (simranarora) We are currently traversing in Es6 for this pass, but the conversion
*to an Es6 scope creator is breaking existing test cases
*/
liveness =
new LiveVariablesAnalysis(
cfg, functionScope, compiler, SyntacticScopeCreator.makeUntyped(compiler));
liveness.analyze();
tryRemoveDeadAssignments(t, cfg);
}
Expand Down
Expand Up @@ -140,7 +140,7 @@ public void enterScope(NodeTraversal t) {
// Process the body of the function.
cfa.process(null, t.getScopeRoot());
cfg = cfa.getCfg();
reachingDef = new MustBeReachingVariableDef(cfg, t.getScope(), compiler);
reachingDef = new MustBeReachingVariableDef(cfg, t.getScope(), compiler, t.getScopeCreator());
reachingDef.analyze();
candidates = new LinkedHashSet<>();

Expand All @@ -149,7 +149,7 @@ public void enterScope(NodeTraversal t) {
NodeTraversal.traverseEs6(compiler, t.getScopeRoot().getLastChild(), new GatherCandiates());

// Compute the backward reaching use. The CFG can be reused.
reachingUses = new MaybeReachingVariableUse(cfg, t.getScope(), compiler);
reachingUses = new MaybeReachingVariableUse(cfg, t.getScope(), compiler, t.getScopeCreator());
reachingUses.analyze();
while (!candidates.isEmpty()) {
Candidate c = candidates.iterator().next();
Expand Down
9 changes: 6 additions & 3 deletions src/com/google/javascript/jscomp/LiveVariablesAnalysis.java
Expand Up @@ -115,12 +115,15 @@ public int hashCode() {
private final Scope jsScope;
private final Set<Var> escaped;

LiveVariablesAnalysis(ControlFlowGraph<Node> cfg, Scope jsScope,
AbstractCompiler compiler) {
LiveVariablesAnalysis(
ControlFlowGraph<Node> cfg,
Scope jsScope,
AbstractCompiler compiler,
ScopeCreator scopeCreator) {
super(cfg, new LiveVariableJoinOp());
this.jsScope = jsScope;
this.escaped = new HashSet<>();
computeEscaped(jsScope, escaped, compiler);
computeEscaped(jsScope, escaped, compiler, scopeCreator);
}

public Set<? extends Var> getEscapedLocals() {
Expand Down
Expand Up @@ -47,14 +47,17 @@ class MaybeReachingVariableUse extends
private final Set<Var> escaped;

MaybeReachingVariableUse(
ControlFlowGraph<Node> cfg, Scope jsScope, AbstractCompiler compiler) {
ControlFlowGraph<Node> cfg,
Scope jsScope,
AbstractCompiler compiler,
ScopeCreator scopeCreator) {
super(cfg, new ReachingUsesJoinOp());
this.jsScope = jsScope;
this.escaped = new HashSet<>();

// TODO(user): Maybe compute it somewhere else and re-use the escape
// local set here.
computeEscaped(jsScope, escaped, compiler);
computeEscaped(jsScope, escaped, compiler, scopeCreator);
}

/**
Expand Down
Expand Up @@ -46,12 +46,15 @@ final class MustBeReachingVariableDef extends
private final Set<Var> escaped;

MustBeReachingVariableDef(
ControlFlowGraph<Node> cfg, Scope jsScope, AbstractCompiler compiler) {
ControlFlowGraph<Node> cfg,
Scope jsScope,
AbstractCompiler compiler,
ScopeCreator scopeCreator) {
super(cfg, new MustDefJoin());
this.jsScope = jsScope;
this.compiler = compiler;
this.escaped = new HashSet<>();
computeEscaped(jsScope, escaped, compiler);
computeEscaped(jsScope, escaped, compiler, scopeCreator);
}

/**
Expand Down
4 changes: 4 additions & 0 deletions src/com/google/javascript/jscomp/NodeTraversal.java
Expand Up @@ -849,6 +849,10 @@ private Node getCfgRoot() {
return result;
}

public ScopeCreator getScopeCreator() {
return scopeCreator;
}

/**
* Determines whether the traversal is currently in the global scope. Note that this returns false
* in a global block scope.
Expand Down
Expand Up @@ -20,7 +20,6 @@
import com.google.javascript.rhino.InputId;
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.Token;

import junit.framework.TestCase;

/**
Expand Down Expand Up @@ -363,13 +362,12 @@ private static LiveVariablesAnalysis computeLiveness(String src) {
Node script = new Node(Token.SCRIPT, n);
script.setInputId(new InputId("test"));
assertEquals(0, compiler.getErrorCount());
Scope scope = SyntacticScopeCreator.makeUntyped(compiler)
.createScope(n, Scope.createGlobalScope(script));
ScopeCreator scopeCreator = SyntacticScopeCreator.makeUntyped(compiler);
Scope scope = scopeCreator.createScope(n, Scope.createGlobalScope(script));
ControlFlowAnalysis cfa = new ControlFlowAnalysis(compiler, false, true);
cfa.process(null, n);
ControlFlowGraph<Node> cfg = cfa.getCfg();
LiveVariablesAnalysis analysis =
new LiveVariablesAnalysis(cfg, scope, compiler);
LiveVariablesAnalysis analysis = new LiveVariablesAnalysis(cfg, scope, compiler, scopeCreator);
analysis.analyze();
return analysis;
}
Expand Down
Expand Up @@ -151,7 +151,7 @@ private void computeUseDef(String src) {
ControlFlowAnalysis cfa = new ControlFlowAnalysis(compiler, false, true);
cfa.process(null, root);
ControlFlowGraph<Node> cfg = cfa.getCfg();
useDef = new MaybeReachingVariableUse(cfg, scope, compiler);
useDef = new MaybeReachingVariableUse(cfg, scope, compiler, scopeCreator);
useDef.analyze();
def = null;
uses = new ArrayList<>();
Expand Down
Expand Up @@ -18,7 +18,6 @@

import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
import com.google.javascript.rhino.Node;

import junit.framework.TestCase;

/**
Expand Down Expand Up @@ -179,7 +178,7 @@ private void computeDefUse(String src) {
ControlFlowAnalysis cfa = new ControlFlowAnalysis(compiler, false, true);
cfa.process(null, root);
ControlFlowGraph<Node> cfg = cfa.getCfg();
defUse = new MustBeReachingVariableDef(cfg, scope, compiler);
defUse = new MustBeReachingVariableDef(cfg, scope, compiler, scopeCreator);
defUse.analyze();
def = null;
use = null;
Expand Down

0 comments on commit 51bd73b

Please sign in to comment.