Skip to content

Commit

Permalink
Cleanups in PureFunctionIdentifier.
Browse files Browse the repository at this point in the history
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=144390278
  • Loading branch information
tadeegan authored and blickly committed Jan 14, 2017
1 parent 4dea41c commit ccc264c
Showing 1 changed file with 16 additions and 22 deletions.
38 changes: 16 additions & 22 deletions src/com/google/javascript/jscomp/PureFunctionIdentifier.java
Expand Up @@ -43,7 +43,6 @@
import java.io.File; import java.io.File;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap; import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
Expand Down Expand Up @@ -437,11 +436,6 @@ private class FunctionAnalyzer implements ScopedCallback {
this.inExterns = inExterns; this.inExterns = inExterns;
} }


private void resetFunctionVars(Node function) {
blacklistedVarsByFunction.replaceValues(function, Collections.<Var>emptySet());
taintedVarsByFunction.replaceValues(function, Collections.<Var>emptySet());
}

@Override @Override
public boolean shouldTraverse(NodeTraversal traversal, Node node, Node parent) { public boolean shouldTraverse(NodeTraversal traversal, Node node, Node parent) {
// Functions need to be processed as part of pre-traversal so that an entry for the function // Functions need to be processed as part of pre-traversal so that an entry for the function
Expand All @@ -456,7 +450,6 @@ public boolean shouldTraverse(NodeTraversal traversal, Node node, Node parent) {
functionInfo.graphNode = sideEffectGraph.createNode(functionInfo); functionInfo.graphNode = sideEffectGraph.createNode(functionInfo);
} }
} }

return true; return true;
} }


Expand Down Expand Up @@ -573,21 +566,23 @@ public void exitScope(NodeTraversal t) {
} }
} }


// Clean up memory after exiting out of the function scope where we will no longer need these.
if (t.getScopeRoot().isFunction()) { if (t.getScopeRoot().isFunction()) {
resetFunctionVars(function); blacklistedVarsByFunction.removeAll(function);
taintedVarsByFunction.removeAll(function);
} }
} }


private boolean varDeclaredInDifferentFunction(Var v, Scope scope) { private boolean isVarDeclaredInScope(Var v, Scope scope) {
if (v == null) { if (v == null) {
return true;
} else if (v.scope != scope) {
Node declarationRoot = NodeUtil.getEnclosingFunction(v.scope.rootNode);
Node scopeRoot = NodeUtil.getEnclosingFunction(scope.rootNode);
return declarationRoot != scopeRoot;
} else {
return false; return false;
} }
if (v.scope == scope) {
return true;
}
Node declarationRoot = NodeUtil.getEnclosingFunction(v.scope.rootNode);
Node scopeRoot = NodeUtil.getEnclosingFunction(scope.rootNode);
return declarationRoot == scopeRoot;
} }


/** /**
Expand All @@ -603,9 +598,7 @@ private void visitAssignmentOrUnaryOperator(
Node lhs = op.getFirstChild(); Node lhs = op.getFirstChild();
if (lhs.isName()) { if (lhs.isName()) {
Var var = scope.getVar(lhs.getString()); Var var = scope.getVar(lhs.getString());
if (varDeclaredInDifferentFunction(var, scope)) { if (isVarDeclaredInScope(var, scope)) {
sideEffectInfo.setTaintsGlobalState();
} else {
// Assignment to local, if the value isn't a safe local value, // Assignment to local, if the value isn't a safe local value,
// a literal or new object creation, add it to the local blacklist. // a literal or new object creation, add it to the local blacklist.
// parameter values depend on the caller. // parameter values depend on the caller.
Expand All @@ -617,6 +610,8 @@ private void visitAssignmentOrUnaryOperator(
if (rhs != null && op.isAssign() && !NodeUtil.evaluatesToLocalValue(rhs)) { if (rhs != null && op.isAssign() && !NodeUtil.evaluatesToLocalValue(rhs)) {
blacklistedVarsByFunction.put(enclosingFunction, var); blacklistedVarsByFunction.put(enclosingFunction, var);
} }
} else {
sideEffectInfo.setTaintsGlobalState();
} }
} else if (NodeUtil.isGet(lhs)) { } else if (NodeUtil.isGet(lhs)) {
if (lhs.getFirstChild().isThis()) { if (lhs.getFirstChild().isThis()) {
Expand All @@ -627,13 +622,12 @@ private void visitAssignmentOrUnaryOperator(
if (objectNode.isName()) { if (objectNode.isName()) {
var = scope.getVar(objectNode.getString()); var = scope.getVar(objectNode.getString());
} }
if (varDeclaredInDifferentFunction(var, scope)) { if (isVarDeclaredInScope(var, scope)) {
sideEffectInfo.setTaintsGlobalState();
} else {
// Maybe a local object modification. We won't know for sure until // Maybe a local object modification. We won't know for sure until
// we exit the scope and can validate the value of the local. // we exit the scope and can validate the value of the local.
//
taintedVarsByFunction.put(enclosingFunction, var); taintedVarsByFunction.put(enclosingFunction, var);
} else {
sideEffectInfo.setTaintsGlobalState();
} }
} }
} else { } else {
Expand Down

0 comments on commit ccc264c

Please sign in to comment.