Skip to content

Commit ac8c8de

Browse files
brad4dcopybara-github
authored andcommitted
RemoveUnusedCode: log reasons for not removing variables.
Add a new log file that contains the reason why RemoveUnusedCode decided it could not remove a variable declaration. PiperOrigin-RevId: 552975927
1 parent eb4a3b7 commit ac8c8de

File tree

1 file changed

+36
-19
lines changed

1 file changed

+36
-19
lines changed

src/com/google/javascript/jscomp/RemoveUnusedCode.java

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import com.google.javascript.jscomp.CodingConvention.SubclassRelationship;
2929
import com.google.javascript.jscomp.PolyfillUsageFinder.PolyfillUsage;
3030
import com.google.javascript.jscomp.PolyfillUsageFinder.Polyfills;
31+
import com.google.javascript.jscomp.base.format.SimpleFormat;
3132
import com.google.javascript.jscomp.diagnostic.LogFile;
3233
import com.google.javascript.jscomp.resources.ResourceLoader;
3334
import com.google.javascript.rhino.IR;
@@ -142,6 +143,7 @@ class RemoveUnusedCode implements CompilerPass {
142143

143144
// Allocated & cleaned up by process()
144145
private @Nullable LogFile removalLog;
146+
private @Nullable LogFile unremovableLog;
145147

146148
RemoveUnusedCode(Builder builder) {
147149
this.compiler = builder.compiler;
@@ -342,11 +344,15 @@ public void process(Node externs, Node root) {
342344
pinnedPropertyNames.addAll(compiler.getExternProperties());
343345

344346
try (LogFile removalLogFile =
345-
compiler.createOrReopenIndexedLog(this.getClass(), "removals.log")) {
347+
compiler.createOrReopenIndexedLog(this.getClass(), "removals.log");
348+
LogFile keepLogFile =
349+
compiler.createOrReopenIndexedLog(this.getClass(), "unremovable.log")) {
346350
removalLog = removalLogFile; // avoid passing the log file through a bunch of methods
351+
unremovableLog = keepLogFile;
347352
traverseAndRemoveUnusedReferences(root);
348353
} finally {
349354
removalLog = null;
355+
unremovableLog = null;
350356
}
351357
}
352358

@@ -420,7 +426,7 @@ private void traverseNode(Node n, Scope scope) {
420426
.buildFunctionDeclaration(n);
421427
varInfo.addRemovable(functionDeclaration);
422428
if (parent.isExport()) {
423-
varInfo.setIsExplicitlyNotRemovable();
429+
varInfo.setIsExplicitlyNotRemovable(() -> "exported class");
424430
}
425431
} else {
426432
traverseFunction(n, scope);
@@ -526,7 +532,9 @@ private void traverseNode(Node n, Scope scope) {
526532
// class name() {}
527533
// handled at a higher level
528534
checkState(!((parent.isFunction() || parent.isClass()) && parent.getFirstChild() == n));
529-
traverseNameNode(n, scope).setIsExplicitlyNotRemovable();
535+
traverseNameNode(n, scope)
536+
.setIsExplicitlyNotRemovable(
537+
() -> SimpleFormat.format("reference found: %s", n.getLocation()));
530538
}
531539
break;
532540

@@ -925,7 +933,7 @@ private void traverseCatch(Node catchNode, Scope scope) {
925933
if (exceptionNameNode.isName()) {
926934
// exceptionNameNode can be an empty node if not using a binding in 2019.
927935
VarInfo exceptionVarInfo = traverseNameNode(exceptionNameNode, scope);
928-
exceptionVarInfo.setIsExplicitlyNotRemovable();
936+
exceptionVarInfo.setIsExplicitlyNotRemovable(() -> "catch variable");
929937
}
930938
traverseNode(block, scope);
931939
}
@@ -940,7 +948,7 @@ private void traverseEnhancedFor(Node enhancedFor, Scope scope) {
940948
// using previously-declared loop variable. e.g.
941949
// `for (varName of collection) {}`
942950
VarInfo varInfo = traverseNameNode(iterationTarget, forScope);
943-
varInfo.setIsExplicitlyNotRemovable();
951+
varInfo.setIsExplicitlyNotRemovable(() -> "for-of or for-in loop variable");
944952
} else if (NodeUtil.isNameDeclaration(iterationTarget)) {
945953
// loop has const/var/let declaration
946954
Node declNode = iterationTarget.getOnlyChild();
@@ -962,7 +970,7 @@ private void traverseEnhancedFor(Node enhancedFor, Scope scope) {
962970
// We can never remove the loop variable of a for-in or for-of loop, because it's
963971
// essential to loop syntax.
964972
VarInfo varInfo = traverseNameNode(declNode, forScope);
965-
varInfo.setIsExplicitlyNotRemovable();
973+
varInfo.setIsExplicitlyNotRemovable(() -> "for-of or for-in loop variable");
966974
}
967975
} else {
968976
// using some general LHS value e.g.
@@ -1005,7 +1013,8 @@ private void traverseVanillaForNameDeclarations(Node nameDeclaration, Scope scop
10051013
} else if (mayHaveSideEffects(valueNode)) {
10061014
// TODO(bradfordcsmith): Actually allow for removing the variable while keeping the
10071015
// valueNode for its side-effects.
1008-
varInfo.setIsExplicitlyNotRemovable();
1016+
varInfo.setIsExplicitlyNotRemovable(
1017+
() -> "for-loop variable initialization has side-effects");
10091018
traverseNode(valueNode, scope);
10101019
} else {
10111020
VanillaForNameDeclaration vanillaForNameDeclaration =
@@ -1277,18 +1286,18 @@ private void traverseClassDeclaration(Node classNode, Scope scope) {
12771286
VarInfo varInfo = traverseNameNode(classNameNode, scope);
12781287
if (classNode.getParent().isExport()) {
12791288
// Cannot remove an exported class.
1280-
varInfo.setIsExplicitlyNotRemovable();
1289+
varInfo.setIsExplicitlyNotRemovable(() -> "exported class");
12811290
traverseNode(baseClassExpression, scope);
12821291
// Use traverseChildren() here, because we should not consider any properties on the exported
12831292
// class to be removable.
12841293
traverseChildren(classBodyNode, classScope);
12851294
} else if (mayHaveSideEffects(baseClassExpression)) {
12861295
// TODO(bradfordcsmith): implement removal without losing side-effects for this case
1287-
varInfo.setIsExplicitlyNotRemovable();
1296+
varInfo.setIsExplicitlyNotRemovable(() -> "base class expression has side-effects");
12881297
traverseNode(baseClassExpression, scope);
12891298
traverseClassMembers(classBodyNode, classScope);
12901299
} else if (mayHaveSideEffects(classBodyNode)) {
1291-
varInfo.setIsExplicitlyNotRemovable();
1300+
varInfo.setIsExplicitlyNotRemovable(() -> "class body has side-effects");
12921301
traverseNode(baseClassExpression, scope);
12931302
traverseClassMembers(classBodyNode, classScope);
12941303
} else {
@@ -1620,7 +1629,8 @@ private VarInfo traverseVar(Var var) {
16201629
continue;
16211630
}
16221631

1623-
getVarInfo(getVarForNameNode(lValue, functionScope)).setIsExplicitlyNotRemovable();
1632+
getVarInfo(getVarForNameNode(lValue, functionScope))
1633+
.setIsExplicitlyNotRemovable(() -> "parameter in function using arguments");
16241634
}
16251635
// `arguments` is never removable.
16261636
return canonicalUnremovableVarInfo;
@@ -1661,10 +1671,13 @@ private VarInfo getVarInfo(Var var) {
16611671
checkNotNull(var);
16621672
boolean isGlobal = var.isGlobal();
16631673
if (var.isExtern()) {
1674+
unremovableLog.log(() -> SimpleFormat.format("%s: extern", var.getName()));
16641675
return canonicalUnremovableVarInfo;
16651676
} else if (codingConvention.isExported(var.getName(), /* local= */ !isGlobal)) {
1677+
unremovableLog.log(() -> SimpleFormat.format("%s: exported by convention", var.getName()));
16661678
return canonicalUnremovableVarInfo;
16671679
} else if (var.isArguments()) {
1680+
// No point in logging that we cannot remove "arguments"
16681681
return canonicalUnremovableVarInfo;
16691682
} else {
16701683
VarInfo varInfo = varInfoMap.get(var);
@@ -1677,9 +1690,9 @@ private VarInfo getVarInfo(Var var) {
16771690
// varInfo needs to track what value is assigned to it for the purpose of correctly allowing
16781691
// or preventing removal of properties set on it.
16791692
if (!removeGlobals && isGlobal) {
1680-
varInfo.setIsExplicitlyNotRemovable();
1693+
varInfo.setIsExplicitlyNotRemovable(() -> "not removing globals");
16811694
} else if (!removeLocalVars && !isGlobal) {
1682-
varInfo.setIsExplicitlyNotRemovable();
1695+
varInfo.setIsExplicitlyNotRemovable(() -> "not removing locals");
16831696
}
16841697
varInfoMap.put(var, varInfo);
16851698
}
@@ -1851,8 +1864,7 @@ boolean isAssignedValueLocal() {
18511864
* @return the Node representing the local value that is being assigned or `null` if the value
18521865
* is non-local or cannot be determined.
18531866
*/
1854-
@Nullable
1855-
Node getLocalAssignedValue() {
1867+
@Nullable Node getLocalAssignedValue() {
18561868
return null;
18571869
}
18581870

@@ -2852,7 +2864,7 @@ private interface VarInfo {
28522864
* all of the `Removable` objects associated with this variable and either apply their
28532865
* continuations or consider them for independent removal.
28542866
*/
2855-
void setIsExplicitlyNotRemovable();
2867+
void setIsExplicitlyNotRemovable(Supplier<String> reasonSupplier);
28562868

28572869
/**
28582870
* Record that at least one value assigned to the variable is non-local (comes from or escapes
@@ -2891,7 +2903,7 @@ public boolean isRemovable() {
28912903
}
28922904

28932905
@Override
2894-
public void setIsExplicitlyNotRemovable() {
2906+
public void setIsExplicitlyNotRemovable(Supplier<String> reasonSupplier) {
28952907
// nothing to do
28962908
}
28972909

@@ -2917,6 +2929,7 @@ public void removeAllRemovables() {
29172929
/** Tracks the removable code and other state related to variables we may be able to remove. */
29182930
private final class RealVarInfo implements VarInfo {
29192931
final String varName;
2932+
29202933
/**
29212934
* Objects that represent variable declarations, assignments, or class setup calls that can be
29222935
* removed.
@@ -2975,7 +2988,8 @@ public void addRemovable(Removable removable) {
29752988
requiresLocalLiteralValueForRemoval = true;
29762989
}
29772990
if (hasNonLocalOrNonLiteralValue && requiresLocalLiteralValueForRemoval) {
2978-
setIsExplicitlyNotRemovable();
2991+
setIsExplicitlyNotRemovable(
2992+
() -> "hasNonLocalOrNonLiteralValue && requiresLocalLiteralValueForRemoval");
29792993
}
29802994

29812995
if (isEntirelyRemovable) {
@@ -2992,9 +3006,10 @@ public boolean isRemovable() {
29923006
}
29933007

29943008
@Override
2995-
public void setIsExplicitlyNotRemovable() {
3009+
public void setIsExplicitlyNotRemovable(Supplier<String> reasonSupplier) {
29963010
if (isEntirelyRemovable) {
29973011
isEntirelyRemovable = false;
3012+
unremovableLog.log(SimpleFormat.format("%s: %s", varName, reasonSupplier.get()));
29983013
for (Removable r : removables) {
29993014
considerForIndependentRemoval(r);
30003015
}
@@ -3097,8 +3112,10 @@ private PolyfillInfo createPolyfillInfo(Node call, Scope scope, String name) {
30973112
private abstract class PolyfillInfo {
30983113
/** The {@link Polyfill} instance corresponding to the polyfill's definition. */
30993114
final Polyfill removable;
3115+
31003116
/** The rightmost component of the polyfill's qualified name (does not contain a dot). */
31013117
final String key;
3118+
31023119
/** Whether the polyfill is unreferenced and this can be removed safely. */
31033120
boolean isRemovable = true;
31043121

0 commit comments

Comments
 (0)