Skip to content

Commit

Permalink
Change ShadowVariables to use traverseEs6
Browse files Browse the repository at this point in the history 
To be safe, make sure we don't shadow function names or parameters from a function block scope, eg.:
function f(a) { ... var a; ...} might fail.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=131072968
  • Loading branch information
@Dominator008 @brad4d
Dominator008 authored and brad4d committed Aug 23, 2016
1 parent bf7cab9 commit 7b130eb
Show file tree
Hide file tree
Showing 2 changed files with 231 additions and 60 deletions.
99 changes: 68 additions & 31 deletions src/com/google/javascript/jscomp/ShadowVariables.java
View file
Expand Up @@ -22,7 +22,6 @@
import com.google.javascript.jscomp.NodeTraversal.ScopedCallback; import com.google.javascript.jscomp.NodeTraversal.ScopedCallback;
import com.google.javascript.jscomp.RenameVars.Assignment; import com.google.javascript.jscomp.RenameVars.Assignment;
import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Node;

import java.util.Collection; import java.util.Collection;
import java.util.LinkedHashMap; import java.util.LinkedHashMap;
import java.util.Map; import java.util.Map;
Expand Down Expand Up @@ -73,7 +72,7 @@ class ShadowVariables implements CompilerPass {
private final Multimap<Node, String> scopeUpRefMap = HashMultimap.create(); private final Multimap<Node, String> scopeUpRefMap = HashMultimap.create();


// Maps each local variable to all of its referencing NAME nodes in any scope. // Maps each local variable to all of its referencing NAME nodes in any scope.
private final Multimap<Var, Node> varToNameUsage = HashMultimap.create(); private final Multimap<Var, Reference> varToNameUsage = HashMultimap.create();


private final AbstractCompiler compiler; private final AbstractCompiler compiler;


Expand Down Expand Up @@ -113,8 +112,8 @@ public void process(Node externs, Node root) {
// variable usage frequency map. // variable usage frequency map.
// //
// 3. Updates the pseudo naming map if needed. // 3. Updates the pseudo naming map if needed.
NodeTraversal.traverse(compiler, root, new GatherReferenceInfo()); NodeTraversal.traverseEs6(compiler, root, new GatherReferenceInfo());
NodeTraversal.traverse(compiler, root, new DoShadowVariables()); NodeTraversal.traverseEs6(compiler, root, new DoShadowVariables());


if (oldPseudoNameMap != null) { if (oldPseudoNameMap != null) {
oldPseudoNameMap.putAll(deltaPseudoNameMap); oldPseudoNameMap.putAll(deltaPseudoNameMap);
Expand All @@ -137,7 +136,8 @@ public void visit(NodeTraversal t, Node n, Node parent) {
return; return;
} }


Var var = t.getScope().getVar(n.getString()); Scope scope = t.getScope();
Var var = scope.getVar(n.getString());
if (var == null) { if (var == null) {
// extern name or undefined name. // extern name or undefined name.
return; return;
Expand All @@ -149,19 +149,16 @@ public void visit(NodeTraversal t, Node n, Node parent) {
} }


// Using the definition of upward referencing, fill in the map. // Using the definition of upward referencing, fill in the map.
if (var.getScope() != t.getScope()) { if (var.getScope() != scope) {
for (Scope s = t.getScope(); for (Scope s = scope; s != var.getScope() && s.isLocal(); s = s.getParent()) {
s != var.getScope() && s.isLocal(); s = s.getParent()) {
scopeUpRefMap.put(s.getRootNode(), var.name); scopeUpRefMap.put(s.getRootNode(), var.name);
} }
} } else {

if (var.getScope() == t.getScope()) {
scopeUpRefMap.put(t.getScopeRoot(), var.name); scopeUpRefMap.put(t.getScopeRoot(), var.name);
} }


// Find in the usage map that tracks a var and all of its usage. // Find in the usage map that tracks a var and all of its usage.
varToNameUsage.put(var, n); varToNameUsage.put(var, new Reference(n, scope));
} }
} }


Expand All @@ -170,20 +167,34 @@ private class DoShadowVariables extends AbstractPostOrderCallback


@Override @Override
public void enterScope(NodeTraversal t) { public void enterScope(NodeTraversal t) {
Scope s = t.getScope(); if (t.inGlobalScope()) {
if (!s.isLocal()) {
return; return;
} }


// Since we don't shadow global, there is nothing to be done in the // Since we don't shadow global, there is nothing to be done in the
// first immediate local scope as well. // first immediate local scope as well.
if (s.getParent().isGlobal()) { if ((t.getScopeRoot().isFunction()
&& NodeUtil.getEnclosingFunction(t.getScopeRoot().getParent()) == null)
|| (NodeUtil.isFunctionBlock(t.getScopeRoot())
&& NodeUtil.getEnclosingFunction(t.getScopeRoot().getGrandparent()) == null)) {
return; return;
} }


// Make sure that we don't shadow function parameters or function names from a function block
// scope, eg.:
// function f(a) { ... var a; ... } // Unsafe
Scope s = t.getScope();
if (s.isFunctionBlockScope()) {
for (Var var : s.getParent().getVarIterable()) {
scopeUpRefMap.put(s.getRootNode(), var.name);
}
}

for (Var var : s.getVarIterable()) { for (Var var : s.getVarIterable()) {
// Don't shadow variables that is bleed-out to fix an IE bug. // When languageOut is ES3, don't shadow variables that is bleed-out functions or caught
if (var.isBleedingFunction()) { // exceptions to avoid IE8 bugs.
if (!compiler.getOptions().getLanguageOut().isEs5OrHigher()
&& (var.isBleedingFunction() || var.isCatch())) {
continue; continue;
} }


Expand All @@ -193,7 +204,7 @@ public void enterScope(NodeTraversal t) {
} }


// Try to look for the best shadow for the current candidate. // Try to look for the best shadow for the current candidate.
Assignment bestShadow = findBestShadow(s); Assignment bestShadow = findBestShadow(s, var);
if (bestShadow == null) { if (bestShadow == null) {
continue; continue;
} }
Expand All @@ -212,8 +223,8 @@ public void enterScope(NodeTraversal t) {
if (oldPseudoNameMap != null) { if (oldPseudoNameMap != null) {
String targetPseudoName = String targetPseudoName =
oldPseudoNameMap.get(s.getVar(bestShadow.oldName).nameNode); oldPseudoNameMap.get(s.getVar(bestShadow.oldName).nameNode);
for (Node use : varToNameUsage.get(var)) { for (Reference use : varToNameUsage.get(var)) {
deltaPseudoNameMap.put(use, targetPseudoName); deltaPseudoNameMap.put(use.nameNode, targetPseudoName);
} }
} }
} }
Expand All @@ -229,13 +240,18 @@ public void visit(NodeTraversal t, Node n, Node parent) {}
* @return An assignment that can be used as a shadow for a local variable * @return An assignment that can be used as a shadow for a local variable
* in the scope defined by curScopeRoot. * in the scope defined by curScopeRoot.
*/ */
private Assignment findBestShadow(Scope curScope) { private Assignment findBestShadow(Scope curScope, Var var) {
// Search for the candidate starting from the most used local. // Search for the candidate starting from the most used local.
for (Assignment assignment : varsByFrequency) { for (Assignment assignment : varsByFrequency) {
if (assignment.oldName.startsWith(RenameVars.LOCAL_VAR_PREFIX)) { if (assignment.oldName.startsWith(RenameVars.LOCAL_VAR_PREFIX)) {
if (!scopeUpRefMap.containsEntry(curScope.getRootNode(), assignment.oldName)) { if (!scopeUpRefMap.containsEntry(curScope.getRootNode(), assignment.oldName)) {
if (curScope.isDeclared(assignment.oldName, true)) { if (curScope.isDeclared(assignment.oldName, true)) {
return assignment; // Don't shadow if the scopes are the same eg.:
// function f() { var a = 1; { var a = 2; } } // Unsafe
Var toShadow = curScope.getVar(assignment.oldName);
if (var.getScope() != toShadow.getScope()) {
return assignment;
}
} }
} }
} }
Expand All @@ -247,7 +263,7 @@ private void doShadow(Assignment original, Assignment toShadow, Var var) {
Scope s = var.getScope(); Scope s = var.getScope();
// We are now shadowing 'bestShadow' with localAssignment. // We are now shadowing 'bestShadow' with localAssignment.
// All of the reference NAME node of this variable. // All of the reference NAME node of this variable.
Collection<Node> references = varToNameUsage.get(var); Collection<Reference> references = varToNameUsage.get(var);


// First remove both assignments from the sorted list since they need // First remove both assignments from the sorted list since they need
// to be re-sorted. // to be re-sorted.
Expand All @@ -268,23 +284,44 @@ private void doShadow(Assignment original, Assignment toShadow, Var var) {
// declaring scope of the best shadow variable. // declaring scope of the best shadow variable.
Var shadowed = s.getVar(toShadow.oldName); Var shadowed = s.getVar(toShadow.oldName);
if (shadowed != null) { if (shadowed != null) {
for (Scope curScope = s; curScope != shadowed.scope; if (s.isFunctionScope() && s.getRootNode().getLastChild().isBlock()) {
curScope = curScope.getParent()) { scopeUpRefMap.put(s.getRootNode().getLastChild(), toShadow.oldName);
scopeUpRefMap.remove(s.getRootNode().getLastChild(), original.oldName);
}
for (Scope curScope = s; curScope != shadowed.scope; curScope = curScope.getParent()) {
scopeUpRefMap.put(curScope.getRootNode(), toShadow.oldName); scopeUpRefMap.put(curScope.getRootNode(), toShadow.oldName);
scopeUpRefMap.remove(curScope.getRootNode(), original.oldName);
} }
} }


// Mark all the references as shadowed. // Mark all the references as shadowed.
for (Node n : references) { for (Reference ref : references) {
Node n = ref.nameNode;
n.setString(toShadow.oldName); n.setString(toShadow.oldName);
Node cur = n; if (ref.scope.getRootNode() == s.getRootNode()) {
while (cur != s.getRootNode()) { if (var.getNameNode() != ref.nameNode) {
cur = cur.getParent(); scopeUpRefMap.put(s.getRootNode(), toShadow.oldName);
if (cur.isFunction()) { scopeUpRefMap.remove(s.getRootNode(), original.oldName);
scopeUpRefMap.put(cur, toShadow.oldName); }
} else {
for (Scope curScope = ref.scope;
curScope.getRootNode() != s.getRootNode();
curScope = curScope.getParent()) {
scopeUpRefMap.put(curScope.getRootNode(), toShadow.oldName);
scopeUpRefMap.remove(curScope.getRootNode(), original.oldName);
} }
} }
} }
} }
} }

private static final class Reference {
private final Node nameNode;
private final Scope scope;

private Reference(Node nameNode, Scope scope) {
this.nameNode = nameNode;
this.scope = scope;
}
}
} }

0 comments on commit 7b130eb

Please sign in to comment.