Skip to content

Commit

Permalink
RemoveUnusedVars: simplify tracking of removable properties
Browse files Browse the repository at this point in the history 
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=177359860
  • Loading branch information
@brad4d
brad4d committed Nov 30, 2017
1 parent d1eee18 commit 21d4814
Showing 1 changed file with 25 additions and 107 deletions.
132 changes: 25 additions & 107 deletions src/com/google/javascript/jscomp/RemoveUnusedVars.java
View file
Expand Up @@ -21,7 +21,6 @@
import static com.google.common.base.Preconditions.checkState;

import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Multimap;
import com.google.javascript.jscomp.CodingConvention.SubclassRelationship;
Expand Down Expand Up @@ -115,12 +114,8 @@ class RemoveUnusedVars implements CompilerPass {

private final Set<String> referencedPropertyNames = new HashSet<>(IMPLICITLY_USED_PROPERTIES);

/**
* Map from property name to variables on which the property is defined.
*
* TODO(bradfordcsmith): Rework to avoid the need for this map.
*/
private final Multimap<String, VarInfo> varInfoForPropertyNameMap = HashMultimap.create();
/** Stores Removable objects for each property name that is currently considered removable. */
private final Multimap<String, Removable> removablesForPropertyNames = HashMultimap.create();

/** Single value to use for all vars for which we cannot remove anything at all. */
private final VarInfo canonicalTotallyUnremovableVarInfo;
Expand Down Expand Up @@ -148,7 +143,7 @@ class RemoveUnusedVars implements CompilerPass {

// All Vars that are completely unremovable will share this VarInfo instance.
canonicalTotallyUnremovableVarInfo = new VarInfo();
canonicalTotallyUnremovableVarInfo.setCannotRemoveAnything();
canonicalTotallyUnremovableVarInfo.setIsExplicitlyNotRemovable();
}

/**
Expand Down Expand Up @@ -187,8 +182,8 @@ private void traverseAndRemoveUnusedReferences(Node root) {
}

private void removeUnreferencedProperties() {
for (VarInfo varInfo : varInfoForPropertyNameMap.values()) {
varInfo.removeUnreferencedProperties();
for (Removable removable : removablesForPropertyNames.values()) {
removable.remove(compiler);
}
}

Expand Down Expand Up @@ -405,7 +400,7 @@ private void traverseCatch(Node catchNode, Scope scope) {
Node block = exceptionNameNode.getNext();
VarInfo exceptionVarInfo =
traverseVar(scope.getVar(exceptionNameNode.getString()));
exceptionVarInfo.setCannotRemoveAnything();
exceptionVarInfo.setIsExplicitlyNotRemovable();
traverseNode(block, scope);
}

Expand All @@ -422,7 +417,7 @@ private void traverseEnhancedFor(Node enhancedFor, Scope scope) {
// NOTE: var will be null if it was declared in externs
if (var != null) {
VarInfo varInfo = traverseVar(var);
varInfo.setCannotRemoveAnything();
varInfo.setIsExplicitlyNotRemovable();
}
} else if (NodeUtil.isNameDeclaration(iterationTarget)) {
// loop has const/var/let declaration
Expand All @@ -445,7 +440,7 @@ private void traverseEnhancedFor(Node enhancedFor, Scope scope) {
// We can never remove the loop variable of a for-in or for-of loop, because it's
// essential to loop syntax.
VarInfo varInfo = traverseVar(forScope.getVar(declNode.getString()));
varInfo.setCannotRemoveAnything();
varInfo.setIsExplicitlyNotRemovable();
}
} else {
// using some general LHS value e.g.
Expand Down Expand Up @@ -840,9 +835,10 @@ private void removeUnreferencedFunctionArgs(Scope fparamScope) {

private void markPropertyNameReferenced(String propertyName) {
if (referencedPropertyNames.add(propertyName)) {
// on first reference, find all vars having that property and tell them it is now referenced.
for (VarInfo varInfo : varInfoForPropertyNameMap.get(propertyName)) {
varInfo.markPropertyNameReferenced(propertyName);
// Continue traversal of all of the property name's values and no longer consider them for
// removal.
for (Removable removable : removablesForPropertyNames.removeAll(propertyName)) {
removable.applyContinuations();
}
}
}
Expand Down Expand Up @@ -982,7 +978,6 @@ private VarInfo getVarInfo(Var var) {
varInfo.setIsExplicitlyNotRemovable();
} else if (var.getParentNode().isParamList()) {
varInfo.propertyAssignmentsWillPreventRemoval = true;
varInfo.unreferencedPropertiesMayBeRemoved = false;
}
varInfoMap.put(var, varInfo);
}
Expand Down Expand Up @@ -1529,27 +1524,8 @@ private class VarInfo {
*/
final List<Removable> removables = new ArrayList<>();

/**
* Objects that represent assignments to named properties on the variable or on
* `varName.prototype`. These can be considered for removal even if the variable itself
* cannot.
*
* NOTE: Once we realize that we cannot remove a property, the removables for that property
* will be dropped and no more will be added.
*/
Multimap<String, Removable> namedPropertyRemovables = null;

boolean isEntirelyRemovable = true;

/**
* Is it OK to remove properties that appear unused when we are leaving the variable itself in
* place?
*
* <p>Defaults to true if we allow this behavior at all.
* Once set to false, it will not be changed back to true.
*/
boolean unreferencedPropertiesMayBeRemoved = removeUnusedProperties;

/**
* Used along with hasPropertyAssignments to handle cases where property assignment may have
* an unknown side-effect, and so make it unsafe to remove the variable.
Expand Down Expand Up @@ -1599,20 +1575,12 @@ void addRemovable(Removable removable) {
}

// immediately apply continuations, or save the removable for possible removal
if (removable.isNamedPropertyAssignment()) {
String propertyName = removable.getPropertyName();

if (isPropertyRemovable(propertyName)) {
if (namedPropertyRemovables == null) {
namedPropertyRemovables = HashMultimap.create();
}
namedPropertyRemovables.put(propertyName, removable);
varInfoForPropertyNameMap.put(propertyName, this);
} else {
removable.applyContinuations();
}
} else if (isEntirelyRemovable) {
if (isEntirelyRemovable) {
removables.add(removable);
} else if (removeUnusedProperties
&& removable.isNamedPropertyAssignment()
&& !referencedPropertyNames.contains(removable.getPropertyName())) {
removablesForPropertyNames.put(removable.getPropertyName(), removable);
} else {
removable.applyContinuations();
}
Expand All @@ -1628,52 +1596,24 @@ boolean markAsReferenced() {
return setIsExplicitlyNotRemovable();
}

void markPropertyNameReferenced(String propertyName) {
// Only apply continuations and drop the removals for the name if we've decided we cannot
// remove this variable entirely.
if (!isEntirelyRemovable && namedPropertyRemovables != null) {
for (Removable r : namedPropertyRemovables.removeAll(propertyName)) {
r.applyContinuations();
}
}
}

boolean isRemovable() {
return isEntirelyRemovable;
}

/**
* Do not remove this variable or any of its property assignments.
*/
void setCannotRemoveAnything() {
unreferencedPropertiesMayBeRemoved = false;
setIsExplicitlyNotRemovable();
}

boolean isPropertyRemovable(String propertyName) {
return isEntirelyRemovable
|| (unreferencedPropertiesMayBeRemoved
&& !referencedPropertyNames.contains(propertyName));
}

boolean setIsExplicitlyNotRemovable() {
if (isEntirelyRemovable) {
isEntirelyRemovable = false;
for (Removable r : removables) {
r.applyContinuations();
}
removables.clear();
if (namedPropertyRemovables != null) {
// iterate over a copy to avoid ConcurrentModificationException when we remove keys
// within the loop
for (String propertyName : ImmutableList.copyOf(namedPropertyRemovables.keySet())) {
if (!isPropertyRemovable(propertyName)) {
for (Removable r : namedPropertyRemovables.removeAll(propertyName)) {
r.applyContinuations();
}
}
if (removeUnusedProperties
&& r.isNamedPropertyAssignment()
&& !referencedPropertyNames.contains(r.getPropertyName())) {
// we may yet remove individual properties
removablesForPropertyNames.put(r.getPropertyName(), r);
} else {
r.applyContinuations();
}
}
removables.clear();
return true;
} else {
return false;
Expand All @@ -1686,30 +1626,8 @@ void removeAllRemovables() {
removable.remove(compiler);
}
removables.clear();
if (namedPropertyRemovables != null) {
for (Removable removable : namedPropertyRemovables.values()) {
removable.remove(compiler);
}
namedPropertyRemovables.clear();
namedPropertyRemovables = null;
}
}

void removeUnreferencedProperties() {
if (!isEntirelyRemovable && namedPropertyRemovables != null) {
checkState(unreferencedPropertiesMayBeRemoved);
// iterate over a copy to avoid ConcurrentModificationException when we remove keys
// within the loop
for (String propertyName : ImmutableList.copyOf(namedPropertyRemovables.keySet())) {
// There shouldn't be any entries in namedPropertyRemovables for properties we know are
// referenced.
checkState(!referencedPropertyNames.contains(propertyName));
for (Removable r : namedPropertyRemovables.removeAll(propertyName)) {
r.remove(compiler);
}
}
}
}
}

/**
Expand Down

0 comments on commit 21d4814

Please sign in to comment.