Skip to content

Commit

Permalink
RemoveUnusedCode: include externs in scope
Browse files Browse the repository at this point in the history 
Doing this simplifies the code by allowing us to handle extern vars
by just assigning a cannonical unremovable VarInfo to them as we do
for other unremovable vars.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=179433633
  • Loading branch information
@brad4d
brad4d authored and Tyler Breisacher committed Dec 18, 2017
1 parent 00ca044 commit 75bb8b3
Show file tree
Hide file tree
Showing 5 changed files with 671 additions and 547 deletions.
153 changes: 76 additions & 77 deletions src/com/google/javascript/jscomp/RemoveUnusedCode.java
View file
Expand Up @@ -222,9 +222,15 @@ public void process(Node externs, Node root) {
* Traverses a node recursively. Call this once per pass. * Traverses a node recursively. Call this once per pass.
*/ */
private void traverseAndRemoveUnusedReferences(Node root) { private void traverseAndRemoveUnusedReferences(Node root) {
// TODO(bradfordcsmith): Include externs in the scope. // Create scope from parent of root node, which also has externs as a child, so we'll
// Since we don't do this now, scope.getVar(someExtern) returns null. // have extern definitions in scope.
Scope scope = scopeCreator.createScope(root, null); Scope scope = scopeCreator.createScope(root.getParent(), null);
if (!scope.isDeclared(NodeUtil.JSC_PROPERTY_NAME_FN, /* recurse */ true)) {
// TODO(b/70730762): Passes that add references to this should ensure it is declared.
// NOTE: null input makes this an extern var.
scope.declare(
NodeUtil.JSC_PROPERTY_NAME_FN, /* no declaration node */ null, /* no input */ null);
}
worklist.add(new Continuation(root, scope)); worklist.add(new Continuation(root, scope));
while (!worklist.isEmpty()) { while (!worklist.isEmpty()) {
Continuation continuation = worklist.remove(); Continuation continuation = worklist.remove();
Expand Down Expand Up @@ -258,7 +264,6 @@ private void removeUnreferencedProperties() {
private void traverseNode(Node n, Scope scope) { private void traverseNode(Node n, Scope scope) {
Node parent = n.getParent(); Node parent = n.getParent();
Token type = n.getToken(); Token type = n.getToken();
Var var = null;
switch (type) { switch (type) {
case CATCH: case CATCH:
traverseCatch(n, scope); traverseCatch(n, scope);
Expand All @@ -270,7 +275,7 @@ private void traverseNode(Node n, Scope scope) {
// If this function is a removable var, then create a continuation // If this function is a removable var, then create a continuation
// for it instead of traversing immediately. // for it instead of traversing immediately.
if (NodeUtil.isFunctionDeclaration(n)) { if (NodeUtil.isFunctionDeclaration(n)) {
varInfo = traverseVar(scope.getVar(n.getFirstChild().getString())); varInfo = traverseNameNode(n.getFirstChild(), scope);
FunctionDeclaration functionDeclaration = FunctionDeclaration functionDeclaration =
new RemovableBuilder() new RemovableBuilder()
.addContinuation(new Continuation(n, scope)) .addContinuation(new Continuation(n, scope))
Expand Down Expand Up @@ -364,11 +369,7 @@ private void traverseNode(Node n, Scope scope) {
// class name() {} // class name() {}
// handled at a higher level // handled at a higher level
checkState(!((parent.isFunction() || parent.isClass()) && parent.getFirstChild() == n)); checkState(!((parent.isFunction() || parent.isClass()) && parent.getFirstChild() == n));
var = scope.getVar(n.getString()); traverseNameNode(n, scope).markAsReferenced();
if (var != null) {
// All name references that aren't handled elsewhere are references to vars.
traverseVar(var).markAsReferenced();
}
} }
break; break;


Expand All @@ -386,6 +387,10 @@ private void traverseNode(Node n, Scope scope) {
} }
} }


private VarInfo traverseNameNode(Node n, Scope scope) {
return traverseVar(getVarForNameNode(n, scope));
}

private void traverseCall(Node callNode, Scope scope) { private void traverseCall(Node callNode, Scope scope) {
Node parent = callNode.getParent(); Node parent = callNode.getParent();
String classVarName = null; String classVarName = null;
Expand All @@ -409,20 +414,24 @@ private void traverseCall(Node callNode, Scope scope) {
} }
} }


Var classVar = (classVarName == null) ? null : scope.getVar(classVarName); Var classVar = null;
if (classVarName != null && NodeUtil.isValidSimpleName(classVarName)) {
classVar = checkNotNull(scope.getVar(classVarName), classVarName);
}


if (classVar == null || !classVar.isGlobal()) { if (classVar == null || !classVar.isGlobal()) {
// This isn't one of the special call types, or it isn't acting on a global class name. // The call we are traversing does not modify a class definition,
// It would be more correct to only not track when the class name does not // or the class is not specified with a simple variable name,
// reference a constructor, but checking that it is a global is easier and mostly the same. // or the variable name is not global.
// TODO(bradfordcsmith): It would be more correct to check whether the class name references
// a known constructor and expand to allow QNames.
traverseChildren(callNode, scope); traverseChildren(callNode, scope);
} else { } else {
VarInfo classVarInfo = traverseVar(classVar);
RemovableBuilder builder = new RemovableBuilder(); RemovableBuilder builder = new RemovableBuilder();
for (Node child = callNode.getFirstChild(); child != null; child = child.getNext()) { for (Node child = callNode.getFirstChild(); child != null; child = child.getNext()) {
builder.addContinuation(new Continuation(child, scope)); builder.addContinuation(new Continuation(child, scope));
} }
classVarInfo.addRemovable(builder.buildClassSetupCall(callNode)); traverseVar(classVar).addRemovable(builder.buildClassSetupCall(callNode));
} }
} }


Expand All @@ -431,17 +440,19 @@ private void traverseRest(Node restNode, Scope scope) {
if (!target.isName()) { if (!target.isName()) {
traverseNode(target, scope); traverseNode(target, scope);
} else { } else {
Var var = scope.getVar(target.getString()); Var var = getVarForNameNode(target, scope);
if (var != null) { VarInfo varInfo = traverseVar(var);
VarInfo varInfo = traverseVar(var); // NOTE: DestructuringAssign is currently used for both actual destructuring and
// NOTE: DestructuringAssign is currently used for both actual destructuring and // default or rest parameters.
// default or rest parameters. // TODO(bradfordcsmith): Maybe distinguish between these 2 cases.
// TODO(bradfordcsmith): Maybe distinguish between these 2 cases. varInfo.addRemovable(new RemovableBuilder().buildDestructuringAssign(restNode, target));
varInfo.addRemovable(new RemovableBuilder().buildDestructuringAssign(restNode, target));
}
} }
} }


private Var getVarForNameNode(Node nameNode, Scope scope) {
return checkNotNull(scope.getVar(nameNode.getString()), nameNode);
}

private void traverseObjectLiteral(Node objectLiteral, Scope scope) { private void traverseObjectLiteral(Node objectLiteral, Scope scope) {
checkArgument(objectLiteral.isObjectLit(), objectLiteral); checkArgument(objectLiteral.isObjectLit(), objectLiteral);
// Is this an object literal that is assigned directly to a 'prototype' property? // Is this an object literal that is assigned directly to a 'prototype' property?
Expand Down Expand Up @@ -495,8 +506,7 @@ private boolean isPrototypeGetProp(Node n) {
private void traverseCatch(Node catchNode, Scope scope) { private void traverseCatch(Node catchNode, Scope scope) {
Node exceptionNameNode = catchNode.getFirstChild(); Node exceptionNameNode = catchNode.getFirstChild();
Node block = exceptionNameNode.getNext(); Node block = exceptionNameNode.getNext();
VarInfo exceptionVarInfo = VarInfo exceptionVarInfo = traverseNameNode(exceptionNameNode, scope);
traverseVar(scope.getVar(exceptionNameNode.getString()));
exceptionVarInfo.setIsExplicitlyNotRemovable(); exceptionVarInfo.setIsExplicitlyNotRemovable();
traverseNode(block, scope); traverseNode(block, scope);
} }
Expand All @@ -510,12 +520,10 @@ private void traverseEnhancedFor(Node enhancedFor, Scope scope) {
if (iterationTarget.isName()) { if (iterationTarget.isName()) {
// using previously-declared loop variable. e.g. // using previously-declared loop variable. e.g.
// `for (varName of collection) {}` // `for (varName of collection) {}`
Var var = forScope.getVar(iterationTarget.getString()); Var var = getVarForNameNode(iterationTarget, forScope);
// NOTE: var will be null if it was declared in externs // NOTE: var will be null if it was declared in externs
if (var != null) { VarInfo varInfo = traverseVar(var);
VarInfo varInfo = traverseVar(var); varInfo.setIsExplicitlyNotRemovable();
varInfo.setIsExplicitlyNotRemovable();
}
} else if (NodeUtil.isNameDeclaration(iterationTarget)) { } else if (NodeUtil.isNameDeclaration(iterationTarget)) {
// loop has const/var/let declaration // loop has const/var/let declaration
Node declNode = iterationTarget.getOnlyChild(); Node declNode = iterationTarget.getOnlyChild();
Expand All @@ -536,7 +544,7 @@ private void traverseEnhancedFor(Node enhancedFor, Scope scope) {
checkState(!declNode.hasChildren()); checkState(!declNode.hasChildren());
// We can never remove the loop variable of a for-in or for-of loop, because it's // We can never remove the loop variable of a for-in or for-of loop, because it's
// essential to loop syntax. // essential to loop syntax.
VarInfo varInfo = traverseVar(forScope.getVar(declNode.getString())); VarInfo varInfo = traverseNameNode(declNode, forScope);
varInfo.setIsExplicitlyNotRemovable(); varInfo.setIsExplicitlyNotRemovable();
} }
} else { } else {
Expand Down Expand Up @@ -574,7 +582,7 @@ private void traverseVanillaForNameDeclarations(Node nameDeclaration, Scope scop
} else { } else {
Node nameNode = child; Node nameNode = child;
@Nullable Node valueNode = child.getFirstChild(); @Nullable Node valueNode = child.getFirstChild();
VarInfo varInfo = traverseVar(scope.getVar(nameNode.getString())); VarInfo varInfo = traverseNameNode(nameNode, scope);
if (valueNode == null) { if (valueNode == null) {
varInfo.addRemovable(new RemovableBuilder().buildVanillaForNameDeclaration(nameNode)); varInfo.addRemovable(new RemovableBuilder().buildVanillaForNameDeclaration(nameNode));
} else if (NodeUtil.mayHaveSideEffects(valueNode)) { } else if (NodeUtil.mayHaveSideEffects(valueNode)) {
Expand Down Expand Up @@ -602,8 +610,7 @@ private void traverseDeclarationStatement(Node declarationStatement, Scope scope
traverseNode(nameNode, scope); traverseNode(nameNode, scope);
} else { } else {
Node valueNode = nameNode.getFirstChild(); Node valueNode = nameNode.getFirstChild();
VarInfo varInfo = VarInfo varInfo = traverseNameNode(nameNode, scope);
traverseVar(checkNotNull(scope.getVar(nameNode.getString())));
RemovableBuilder builder = new RemovableBuilder(); RemovableBuilder builder = new RemovableBuilder();
if (valueNode == null) { if (valueNode == null) {
varInfo.addRemovable(builder.buildNameDeclarationStatement(declarationStatement)); varInfo.addRemovable(builder.buildNameDeclarationStatement(declarationStatement));
Expand Down Expand Up @@ -662,10 +669,12 @@ private void traverseAssign(Node assignNode, Scope scope) {


// If we successfully identified a name node & there is a corresponding Var, // If we successfully identified a name node & there is a corresponding Var,
// then we have a removable assignment. // then we have a removable assignment.
Var var = (nameNode == null) ? null : scope.getVar(nameNode.getString()); if (nameNode == null) {
if (var == null) { // Not an assignment we need to track
traverseChildren(assignNode, scope); traverseChildren(assignNode, scope);
} else { } else {
VarInfo varInfo = traverseNameNode(nameNode, scope);

Node valueNode = assignNode.getLastChild(); Node valueNode = assignNode.getLastChild();
RemovableBuilder builder = RemovableBuilder builder =
new RemovableBuilder() new RemovableBuilder()
Expand All @@ -677,7 +686,6 @@ private void traverseAssign(Node assignNode, Scope scope) {
builder.addContinuation(new Continuation(valueNode, scope)); builder.addContinuation(new Continuation(valueNode, scope));
} }


VarInfo varInfo = traverseVar(var);
if (isNamedPropertyAssign) { if (isNamedPropertyAssign) {
varInfo.addRemovable(builder.buildNamedPropertyAssign(assignNode, propertyNode)); varInfo.addRemovable(builder.buildNamedPropertyAssign(assignNode, propertyNode));
} else if (isVariableAssign) { } else if (isVariableAssign) {
Expand All @@ -703,23 +711,19 @@ private void traverseDefaultValue(Node defaultValueNode, Scope scope) {
traverseNode(target, scope); traverseNode(target, scope);
traverseNode(value, scope); traverseNode(value, scope);
} else { } else {
var = scope.getVar(target.getString()); var = getVarForNameNode(target, scope);
if (var == null) { VarInfo varInfo = traverseVar(var);
if (NodeUtil.mayHaveSideEffects(value)) {
// TODO(johnlenz): we don't really need to retain all uses of the variable, just
// enough to host the default value assignment.
varInfo.markAsReferenced();
traverseNode(value, scope); traverseNode(value, scope);
} else { } else {
VarInfo varInfo = traverseVar(var); DestructuringAssign assign =
if (NodeUtil.mayHaveSideEffects(value)) { new RemovableBuilder()
// TODO(johnlenz): we don't really need to retain all uses of the variable, just .addContinuation(new Continuation(value, scope))
// enough to host the default value assignment. .buildDestructuringAssign(defaultValueNode, target);
varInfo.markAsReferenced(); varInfo.addRemovable(assign);
traverseNode(value, scope);
} else {
DestructuringAssign assign =
new RemovableBuilder()
.addContinuation(new Continuation(value, scope))
.buildDestructuringAssign(defaultValueNode, target);
varInfo.addRemovable(assign);
}
} }
} }
} }
Expand All @@ -730,11 +734,8 @@ private void traverseArrayPattern(Node arrayPattern, Scope scope) {
// TODO(bradfordcsmith): Treat destructuring assignments to properties as removable writes. // TODO(bradfordcsmith): Treat destructuring assignments to properties as removable writes.
traverseNode(c, scope); traverseNode(c, scope);
} else { } else {
Var var = scope.getVar(c.getString()); VarInfo varInfo = traverseNameNode(c, scope);
if (var != null) { varInfo.addRemovable(new RemovableBuilder().buildDestructuringAssign(c, c));
VarInfo varInfo = traverseVar(var);
varInfo.addRemovable(new RemovableBuilder().buildDestructuringAssign(c, c));
}
} }
} }
} }
Expand Down Expand Up @@ -778,11 +779,12 @@ private void traverseObjectPatternElement(Node elm, Scope scope) {
} }


// TODO(bradfordcsmith): Handle property assignments also // TODO(bradfordcsmith): Handle property assignments also
Var var = target.isName() ? scope.getVar(target.getString()) : null; // e.g. `({a: foo.bar, b: foo.baz}) = {a: 1, b: 2}`
VarInfo varInfo = target.isName() ? traverseNameNode(target, scope) : null;


// TODO(bradfordcsmith): Arrange to safely remove side-effect cases. // TODO(bradfordcsmith): Arrange to safely remove side-effect cases.
boolean cannotRemove = boolean cannotRemove =
var == null varInfo == null
|| (propertyExpression != null && NodeUtil.mayHaveSideEffects(propertyExpression)) || (propertyExpression != null && NodeUtil.mayHaveSideEffects(propertyExpression))
|| (defaultValue != null && NodeUtil.mayHaveSideEffects(defaultValue)); || (defaultValue != null && NodeUtil.mayHaveSideEffects(defaultValue));


Expand All @@ -797,9 +799,8 @@ private void traverseObjectPatternElement(Node elm, Scope scope) {
if (defaultValue != null) { if (defaultValue != null) {
traverseNode(defaultValue, scope); traverseNode(defaultValue, scope);
} }
if (var != null) { if (varInfo != null) {
// Since we cannot remove it, we must now treat this usage as a reference. varInfo.markAsReferenced();
traverseVar(var).markAsReferenced();
} }
} else { } else {
RemovableBuilder builder = new RemovableBuilder(); RemovableBuilder builder = new RemovableBuilder();
Expand All @@ -813,8 +814,7 @@ private void traverseObjectPatternElement(Node elm, Scope scope) {
if (defaultValue != null) { if (defaultValue != null) {
builder.addContinuation(new Continuation(defaultValue, scope)); builder.addContinuation(new Continuation(defaultValue, scope));
} }
traverseVar(var) varInfo.addRemovable(builder.buildDestructuringAssign(elm, target));
.addRemovable(builder.buildDestructuringAssign(elm, target));
} }
} }


Expand Down Expand Up @@ -848,7 +848,7 @@ private void traverseClassDeclaration(Node classNode, Scope scope) {
Node classBodyNode = baseClassExpression.getNext(); Node classBodyNode = baseClassExpression.getNext();
Scope classScope = scopeCreator.createScope(classNode, scope); Scope classScope = scopeCreator.createScope(classNode, scope);


VarInfo varInfo = traverseVar(scope.getVar(classNameNode.getString())); VarInfo varInfo = traverseNameNode(classNameNode, scope);
if (classNode.getParent().isExport()) { if (classNode.getParent().isExport()) {
// Cannot remove an exported class. // Cannot remove an exported class.
varInfo.setIsExplicitlyNotRemovable(); varInfo.setIsExplicitlyNotRemovable();
Expand Down Expand Up @@ -879,7 +879,7 @@ private void traverseClassExpression(Node classNode, Scope scope) {


if (classNameNode.isName()) { if (classNameNode.isName()) {
// We may be able to remove the name node if nothing ends up referring to it. // We may be able to remove the name node if nothing ends up referring to it.
VarInfo varInfo = traverseVar(classScope.getVar(classNameNode.getString())); VarInfo varInfo = traverseNameNode(classNameNode, classScope);
varInfo.addRemovable(new RemovableBuilder().buildNamedClassExpression(classNode)); varInfo.addRemovable(new RemovableBuilder().buildNamedClassExpression(classNode));
} }
// If we're traversing the class expression, we've already decided we cannot remove it. // If we're traversing the class expression, we've already decided we cannot remove it.
Expand Down Expand Up @@ -933,10 +933,10 @@ private void traverseFunction(Node function, Scope parentScope) {
// Checking the function body // Checking the function body
Scope fbodyScope = scopeCreator.createScope(body, fparamScope); Scope fbodyScope = scopeCreator.createScope(body, fparamScope);


String name = function.getFirstChild().getString(); Node nameNode = function.getFirstChild();
if (!name.isEmpty()) { if (!nameNode.getString().isEmpty()) {
// var x = function funcName() {}; // var x = function funcName() {};
Var var = checkNotNull(fparamScope.getVar(name)); Var var = getVarForNameNode(nameNode, fparamScope);
// make sure funcName gets into the varInfoMap so it will be considered for removal. // make sure funcName gets into the varInfoMap so it will be considered for removal.
traverseVar(var); traverseVar(var);
} }
Expand Down Expand Up @@ -1042,8 +1042,7 @@ private void markUnusedParameters(Node paramList, Scope fparamScope) {
if (lValue.isDestructuringPattern()) { if (lValue.isDestructuringPattern()) {
continue; continue;
} }
Var var = fparamScope.getVar(lValue.getString()); VarInfo varInfo = traverseNameNode(lValue, fparamScope);
VarInfo varInfo = getVarInfo(var);
if (varInfo.isRemovable()) { if (varInfo.isRemovable()) {
param.setUnusedParameter(true); param.setUnusedParameter(true);
compiler.reportChangeToEnclosingScope(paramList); compiler.reportChangeToEnclosingScope(paramList);
Expand Down Expand Up @@ -1090,8 +1089,7 @@ private void maybeRemoveUnusedTrailingParameters(Node argList, Scope fparamScope
} }
} }


Var var = fparamScope.getVar(lValue.getString()); VarInfo varInfo = getVarInfo(getVarForNameNode(lValue, fparamScope));
VarInfo varInfo = getVarInfo(var);
if (varInfo.isRemovable()) { if (varInfo.isRemovable()) {
NodeUtil.deleteNode(lastArg, compiler); NodeUtil.deleteNode(lastArg, compiler);
} else { } else {
Expand Down Expand Up @@ -1125,8 +1123,7 @@ private VarInfo traverseVar(Var var) {
if (lValue.isDestructuringPattern()) { if (lValue.isDestructuringPattern()) {
continue; continue;
} }
Var paramVar = functionScope.getVar(lValue.getString()); getVarInfo(getVarForNameNode(lValue, functionScope)).markAsReferenced();
getVarInfo(paramVar).markAsReferenced();
} }
// `arguments` is never removable. // `arguments` is never removable.
return canonicalUnremovableVarInfo; return canonicalUnremovableVarInfo;
Expand All @@ -1145,7 +1142,9 @@ private VarInfo traverseVar(Var var) {
private VarInfo getVarInfo(Var var) { private VarInfo getVarInfo(Var var) {
checkNotNull(var); checkNotNull(var);
boolean isGlobal = var.isGlobal(); boolean isGlobal = var.isGlobal();
if (isGlobal && !removeGlobals) { if (var.isExtern()) {
return canonicalUnremovableVarInfo;
} else if (isGlobal && !removeGlobals) {
return canonicalUnremovableVarInfo; return canonicalUnremovableVarInfo;
} else if (!isGlobal && !removeLocalVars) { } else if (!isGlobal && !removeLocalVars) {
return canonicalUnremovableVarInfo; return canonicalUnremovableVarInfo;
Expand Down

0 comments on commit 75bb8b3

Please sign in to comment.