From a9f0ce063627e38755567400acb0a95bacf2eefa Mon Sep 17 00:00:00 2001 From: stalcup Date: Fri, 16 Jun 2017 13:36:51 -0700 Subject: [PATCH] Improves j2clOptBundlePass perf by running on just changed scopes. In [] j2clOptBundlePass accounts for 7.15% of execution time. 4.0% of that time is the PureFunctionIdentifier and the other 3.15% is J2CL pass visitors. Though this CL does not migrate *all* J2CL pass visitors to the new scope limited traversal, it does migrate most (and documents what is necessary to migrate the rest) and reduces that 3.15% cost to 1.5%. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=159268542 --- .../javascript/jscomp/DefaultPassConfig.java | 9 ++-- .../jscomp/FunctionToBlockMutator.java | 15 +++---- .../javascript/jscomp/InlineFunctions.java | 1 + .../jscomp/J2clClinitPrunerPass.java | 42 ++++++++++++++---- .../jscomp/J2clConstantHoisterPass.java | 29 +++++++----- .../jscomp/J2clEqualitySameRewriterPass.java | 7 ++- .../jscomp/MakeDeclaredNamesUnique.java | 44 +++++++++++++------ .../javascript/jscomp/NodeTraversal.java | 25 ++++++++++- .../google/javascript/jscomp/NodeUtil.java | 40 +++++++++++++++++ .../google/javascript/jscomp/Normalize.java | 3 +- .../jscomp/PeepholeFoldConstants.java | 4 +- .../jscomp/PeepholeRemoveDeadCode.java | 5 +-- .../javascript/jscomp/RenameLabels.java | 27 ++++++++---- .../jscomp/J2clClinitPrunerPassTest.java | 3 +- .../J2clEqualitySameRewriterPassTest.java | 3 +- .../javascript/jscomp/NodeTraversalTest.java | 39 ++++++++++++++++ 16 files changed, 230 insertions(+), 66 deletions(-) diff --git a/src/com/google/javascript/jscomp/DefaultPassConfig.java b/src/com/google/javascript/jscomp/DefaultPassConfig.java index 9076f47151e..308a7d1484d 100644 --- a/src/com/google/javascript/jscomp/DefaultPassConfig.java +++ b/src/com/google/javascript/jscomp/DefaultPassConfig.java @@ -3111,11 +3111,14 @@ protected FeatureSet featureSet() { new PassFactory("j2clOptBundlePass", false) { @Override protected CompilerPass create(AbstractCompiler compiler) { - final J2clClinitPrunerPass j2clClinitPrunerPass = new J2clClinitPrunerPass(compiler); + List changedScopeNodes = compiler.getChangedScopeNodesForPass(getName()); + + final J2clClinitPrunerPass j2clClinitPrunerPass = + new J2clClinitPrunerPass(compiler, changedScopeNodes); final J2clConstantHoisterPass j2clConstantHoisterPass = - (new J2clConstantHoisterPass(compiler)); + new J2clConstantHoisterPass(compiler); final J2clEqualitySameRewriterPass j2clEqualitySameRewriterPass = - (new J2clEqualitySameRewriterPass(compiler)); + new J2clEqualitySameRewriterPass(compiler, changedScopeNodes); return new CompilerPass() { @Override diff --git a/src/com/google/javascript/jscomp/FunctionToBlockMutator.java b/src/com/google/javascript/jscomp/FunctionToBlockMutator.java index 5285a734be3..7a1b0885c17 100644 --- a/src/com/google/javascript/jscomp/FunctionToBlockMutator.java +++ b/src/com/google/javascript/jscomp/FunctionToBlockMutator.java @@ -216,7 +216,6 @@ private static void fixUninitializedVarDeclarations(Node n, Node containingBlock } } - /** * Fix-up all local names to be unique for this subtree. * @param fnNode A mutable instance of the function to be inlined. @@ -225,16 +224,14 @@ private void makeLocalNamesUnique(Node fnNode, boolean isCallInLoop) { Supplier idSupplier = compiler.getUniqueNameIdSupplier(); // Make variable names unique to this instance. NodeTraversal.traverseEs6( - compiler, fnNode, new MakeDeclaredNamesUnique( + compiler, + fnNode, + new MakeDeclaredNamesUnique( new InlineRenamer( - compiler.getCodingConvention(), - idSupplier, - "inline_", - isCallInLoop, - true, - null))); + compiler.getCodingConvention(), idSupplier, "inline_", isCallInLoop, true, null), + false)); // Make label names unique to this instance. - new RenameLabels(compiler, new LabelNameSupplier(idSupplier), false) + new RenameLabels(compiler, new LabelNameSupplier(idSupplier), false, false) .process(null, fnNode); } diff --git a/src/com/google/javascript/jscomp/InlineFunctions.java b/src/com/google/javascript/jscomp/InlineFunctions.java index a84666083a2..1dd5f316fcb 100644 --- a/src/com/google/javascript/jscomp/InlineFunctions.java +++ b/src/com/google/javascript/jscomp/InlineFunctions.java @@ -776,6 +776,7 @@ void removeInlinedFunctions() { Preconditions.checkState(fn != null); verifyAllReferencesInlined(functionState); fn.remove(); + NodeUtil.markFunctionsDeleted(fn.getFunctionNode(), compiler); } } } diff --git a/src/com/google/javascript/jscomp/J2clClinitPrunerPass.java b/src/com/google/javascript/jscomp/J2clClinitPrunerPass.java index 41ac9c76a9e..0b6cf96830e 100644 --- a/src/com/google/javascript/jscomp/J2clClinitPrunerPass.java +++ b/src/com/google/javascript/jscomp/J2clClinitPrunerPass.java @@ -19,10 +19,12 @@ import com.google.common.base.Strings; import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback; import com.google.javascript.jscomp.NodeTraversal.Callback; +import com.google.javascript.jscomp.NodeTraversal.FunctionCallback; import com.google.javascript.rhino.Node; import java.util.ArrayDeque; import java.util.Deque; import java.util.HashSet; +import java.util.List; import java.util.Set; import javax.annotation.Nullable; @@ -33,9 +35,11 @@ public class J2clClinitPrunerPass implements CompilerPass { private final AbstractCompiler compiler; private boolean madeChange = false; + private final List changedScopeNodes; - J2clClinitPrunerPass(AbstractCompiler compiler) { + J2clClinitPrunerPass(AbstractCompiler compiler, List changedScopeNodes) { this.compiler = compiler; + this.changedScopeNodes = changedScopeNodes; } @Override @@ -44,19 +48,41 @@ public void process(Node externs, Node root) { return; } - NodeTraversal.traverseEs6(compiler, root, new RedundantClinitPruner()); - NodeTraversal.traverseEs6(compiler, root, new LookAheadRedundantClinitPruner()); - NodeTraversal.traverseEs6(compiler, root, new EmptyClinitPruner()); + RedundantClinitPruner redundantClinitPruner = new RedundantClinitPruner(); + NodeTraversal.traverseEs6ScopeRoots( + compiler, + root, + getNonNestedParentScopeNodes(), + redundantClinitPruner, + redundantClinitPruner, // FunctionCallback + true); + NodeTraversal.traverseEs6ScopeRoots( + compiler, root, changedScopeNodes, new LookAheadRedundantClinitPruner(), false); + NodeTraversal.traverseEs6ScopeRoots( + compiler, root, changedScopeNodes, new EmptyClinitPruner(), false); if (madeChange) { + // This invocation is ~70% of ALL the cost of j2clOptBundlePass :( new PureFunctionIdentifier.Driver(compiler, null).process(externs, root); } } - /** - * Removes redundant clinit calls inside method body if it is guaranteed to be called earlier. - */ - private final class RedundantClinitPruner implements Callback { + private List getNonNestedParentScopeNodes() { + return changedScopeNodes == null + ? null + : NodeUtil.removeNestedChangeScopeNodes( + NodeUtil.getParentChangeScopeNodes(changedScopeNodes)); + } + + /** Removes redundant clinit calls inside method body if it is guaranteed to be called earlier. */ + private final class RedundantClinitPruner implements Callback, FunctionCallback { + + @Override + public void enterFunction(AbstractCompiler compiler, Node fnRoot) { + // Reset the clinit call tracking when starting over on a new scope. + clinitsCalledAtBranch = new HierarchicalSet<>(null); + stateStack.clear(); + } private final Deque> stateStack = new ArrayDeque<>(); private HierarchicalSet clinitsCalledAtBranch = new HierarchicalSet<>(null); diff --git a/src/com/google/javascript/jscomp/J2clConstantHoisterPass.java b/src/com/google/javascript/jscomp/J2clConstantHoisterPass.java index 6b54175ea32..eb22044e4d2 100644 --- a/src/com/google/javascript/jscomp/J2clConstantHoisterPass.java +++ b/src/com/google/javascript/jscomp/J2clConstantHoisterPass.java @@ -46,17 +46,24 @@ public void process(Node externs, Node root) { final Multimap fieldAssignments = ArrayListMultimap.create(); final Set hoistableFunctions = new HashSet<>(); - NodeTraversal.traverseEs6(compiler, root, new AbstractPostOrderCallback() { - @Override - public void visit(NodeTraversal t, Node node, Node parent) { - if (parent != null && NodeUtil.isLValue(node)) { - fieldAssignments.put(node.getQualifiedName(), parent); - } - if (isHoistableFunction(t, node)) { - hoistableFunctions.add(node); - } - } - }); + NodeTraversal.traverseEs6( + compiler, + root, + new AbstractPostOrderCallback() { + @Override + public void visit(NodeTraversal t, Node node, Node parent) { + // TODO(stalcup): don't gather assignments ourselves, switch to a persistent + // DefinitionUseSiteFinder instead. + if (parent != null && NodeUtil.isLValue(node)) { + fieldAssignments.put(node.getQualifiedName(), parent); + } + + // TODO(stalcup): convert to a persistent index of hoistable functions. + if (isHoistableFunction(t, node)) { + hoistableFunctions.add(node); + } + } + }); for (Collection assignments : fieldAssignments.asMap().values()) { maybeHoistClassField(assignments, hoistableFunctions); diff --git a/src/com/google/javascript/jscomp/J2clEqualitySameRewriterPass.java b/src/com/google/javascript/jscomp/J2clEqualitySameRewriterPass.java index 374c712687e..c37f5163096 100644 --- a/src/com/google/javascript/jscomp/J2clEqualitySameRewriterPass.java +++ b/src/com/google/javascript/jscomp/J2clEqualitySameRewriterPass.java @@ -18,6 +18,7 @@ import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback; import com.google.javascript.rhino.IR; import com.google.javascript.rhino.Node; +import java.util.List; /** An optimization pass to re-write J2CL Equality.$same. */ public class J2clEqualitySameRewriterPass extends AbstractPostOrderCallback @@ -30,9 +31,11 @@ private static enum Eq { } private final AbstractCompiler compiler; + private final List changedScopeNodes; - J2clEqualitySameRewriterPass(AbstractCompiler compiler) { + J2clEqualitySameRewriterPass(AbstractCompiler compiler, List changedScopeNodes) { this.compiler = compiler; + this.changedScopeNodes = changedScopeNodes; } @Override @@ -41,7 +44,7 @@ public void process(Node externs, Node root) { return; } - NodeTraversal.traverseEs6(compiler, root, this); + NodeTraversal.traverseEs6ScopeRoots(compiler, root, changedScopeNodes, this, false); } @Override diff --git a/src/com/google/javascript/jscomp/MakeDeclaredNamesUnique.java b/src/com/google/javascript/jscomp/MakeDeclaredNamesUnique.java index 24babd2fee4..b1de55a22e7 100644 --- a/src/com/google/javascript/jscomp/MakeDeclaredNamesUnique.java +++ b/src/com/google/javascript/jscomp/MakeDeclaredNamesUnique.java @@ -25,6 +25,7 @@ import com.google.common.collect.ListMultimap; import com.google.common.collect.MultimapBuilder; import com.google.common.collect.Multiset; +import com.google.javascript.jscomp.MakeDeclaredNamesUnique.ContextualRenameInverter; import com.google.javascript.jscomp.NodeTraversal.ScopedCallback; import com.google.javascript.rhino.IR; import com.google.javascript.rhino.Node; @@ -61,17 +62,23 @@ class MakeDeclaredNamesUnique implements NodeTraversal.ScopedCallback { // In addition, ES6 introduced block scopes, which we also need to handle. private final Deque nameStack = new ArrayDeque<>(); private final Renamer rootRenamer; + private final boolean markChanges; MakeDeclaredNamesUnique() { - this(new ContextualRenamer()); + this(new ContextualRenamer(), true); } MakeDeclaredNamesUnique(Renamer renamer) { + this(renamer, true); + } + + MakeDeclaredNamesUnique(Renamer renamer, boolean markChanges) { this.rootRenamer = renamer; + this.markChanges = markChanges; } static CompilerPass getContextualRenameInverter(AbstractCompiler compiler) { - return new ContextualRenameInverter(compiler); + return new ContextualRenameInverter(compiler, true); } @Override @@ -186,6 +193,7 @@ public void visit(NodeTraversal t, Node n, Node parent) { break; } } + private void visitName(NodeTraversal t, Node n, Node parent) { String newName = getReplacementName(n.getString()); if (newName != null) { @@ -195,11 +203,13 @@ private void visitName(NodeTraversal t, Node n, Node parent) { n.removeProp(Node.IS_CONSTANT_NAME); } n.setString(newName); - t.reportCodeChange(); - // If we are renaming a function declaration, make sure the containing scope - // has the opporunity to act on the change. - if (parent.isFunction() && NodeUtil.isFunctionDeclaration(parent)) { - t.getCompiler().reportChangeToEnclosingScope(parent); + if (markChanges) { + t.reportCodeChange(); + // If we are renaming a function declaration, make sure the containing scope + // has the opporunity to act on the change. + if (parent.isFunction() && NodeUtil.isFunctionDeclaration(parent)) { + t.getCompiler().reportChangeToEnclosingScope(parent); + } } } } @@ -301,8 +311,12 @@ static class ContextualRenameInverter private final ListMultimap nameMap = MultimapBuilder.hashKeys().arrayListValues().build(); - private ContextualRenameInverter(AbstractCompiler compiler) { + // Whether to report changes to the compiler. + private final boolean markChanges; + + private ContextualRenameInverter(AbstractCompiler compiler, boolean markChanges) { this.compiler = compiler; + this.markChanges = markChanges; } @Override @@ -379,12 +393,14 @@ void handleScopeVar(Var v) { for (Node n : references) { checkState(n.isName(), n); n.setString(newName); - compiler.reportChangeToEnclosingScope(n); - Node parent = n.getParent(); - // If we are renaming a function declaration, make sure the containing scope - // has the opportunity to act on the change. - if (parent.isFunction() && NodeUtil.isFunctionDeclaration(parent)) { - compiler.reportChangeToEnclosingScope(parent); + if (markChanges) { + compiler.reportChangeToEnclosingScope(n); + Node parent = n.getParent(); + // If we are renaming a function declaration, make sure the containing scope + // has the opportunity to act on the change. + if (parent.isFunction() && NodeUtil.isFunctionDeclaration(parent)) { + compiler.reportChangeToEnclosingScope(parent); + } } } nameMap.removeAll(name); diff --git a/src/com/google/javascript/jscomp/NodeTraversal.java b/src/com/google/javascript/jscomp/NodeTraversal.java index 90919ab3763..d3e7f6feab5 100644 --- a/src/com/google/javascript/jscomp/NodeTraversal.java +++ b/src/com/google/javascript/jscomp/NodeTraversal.java @@ -432,8 +432,9 @@ private void traverseScopeRoot(Node scopeRoot) { } /** - * Traverses *just* the contents of provided scope nodes (but not scopes nested within them) but - * will fall back on traversing the entire AST from root if a null scope nodes list is provided. + * Traverses *just* the contents of provided scope nodes (and optionally scopes nested within + * them) but will fall back on traversing the entire AST from root if a null scope nodes list is + * provided. */ public static void traverseEs6ScopeRoots( AbstractCompiler compiler, @@ -441,6 +442,22 @@ public static void traverseEs6ScopeRoots( List scopeNodes, final Callback cb, final boolean traverseNested) { + traverseEs6ScopeRoots(compiler, root, scopeNodes, cb, null, traverseNested); + } + + /** + * Traverses *just* the contents of provided scope nodes (and optionally scopes nested within + * them) but will fall back on traversing the entire AST from root if a null scope nodes list is + * provided. Also allows for a callback to notify when starting on one of the provided scope + * nodes. + */ + public static void traverseEs6ScopeRoots( + AbstractCompiler compiler, + Node root, + List scopeNodes, + final Callback cb, + final FunctionCallback fcb, + final boolean traverseNested) { if (scopeNodes == null) { NodeTraversal.traverseEs6(compiler, root, cb); } else { @@ -448,6 +465,10 @@ public static void traverseEs6ScopeRoots( new MemoizedScopeCreator(new Es6SyntacticScopeCreator(compiler)); for (final Node scopeNode : scopeNodes) { + if (fcb != null) { + fcb.enterFunction(compiler, scopeNode); + } + Callback scb = new Callback() { @Override diff --git a/src/com/google/javascript/jscomp/NodeUtil.java b/src/com/google/javascript/jscomp/NodeUtil.java index d98ee2f574e..3e36e6be411 100644 --- a/src/com/google/javascript/jscomp/NodeUtil.java +++ b/src/com/google/javascript/jscomp/NodeUtil.java @@ -43,6 +43,7 @@ import java.util.Collections; import java.util.EnumSet; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -2647,6 +2648,17 @@ static boolean isInSyntheticScript(Node n) { return n.getSourceFileName() != null && n.getSourceFileName().startsWith(" [synthetic:"); } + /** + * Removes all children from this node, isolates the children from each other and reports any + * nested functions as deleted. + */ + public static void detachChildren(Node n, AbstractCompiler compiler) { + for (Node child = n.getFirstChild(); child != null; child = child.getNext()) { + NodeUtil.markFunctionsDeleted(child, compiler); + } + n.detachChildren(); + } + /** * Safely remove children while maintaining a valid node structure. * In some cases, this is done by removing the parent from the AST as well. @@ -4931,4 +4943,32 @@ static void markFunctionsDeleted(Node node, AbstractCompiler compiler) { markFunctionsDeleted(child, compiler); } } + + /** Returns the list of scope nodes which are parents of the provided list of scope nodes. */ + public static List getParentChangeScopeNodes(List scopeNodes) { + Set parentScopeNodes = new LinkedHashSet<>(scopeNodes); + for (Node scopeNode : scopeNodes) { + parentScopeNodes.add(getEnclosingChangeScopeRoot(scopeNode)); + } + return new ArrayList<>(parentScopeNodes); + } + + /** + * Removes any scope nodes from the provided list that are nested within some other scope node + * also in the list. Returns the modified list. + */ + public static List removeNestedChangeScopeNodes(List scopeNodes) { + Set uniqueScopeNodes = new LinkedHashSet<>(scopeNodes); + for (Node scopeNode : scopeNodes) { + for (Node ancestor = scopeNode.getParent(); + ancestor != null; + ancestor = ancestor.getParent()) { + if (isChangeScopeRoot(ancestor) && uniqueScopeNodes.contains(ancestor)) { + uniqueScopeNodes.remove(scopeNode); + break; + } + } + } + return new ArrayList<>(uniqueScopeNodes); + } } diff --git a/src/com/google/javascript/jscomp/Normalize.java b/src/com/google/javascript/jscomp/Normalize.java index 52d82048853..b20de1ffafb 100644 --- a/src/com/google/javascript/jscomp/Normalize.java +++ b/src/com/google/javascript/jscomp/Normalize.java @@ -75,7 +75,8 @@ static void normalizeSyntheticCode( NodeTraversal.traverseEs6(compiler, js, new Normalize.NormalizeStatements(compiler, false)); NodeTraversal.traverseEs6( - compiler, js, + compiler, + js, new MakeDeclaredNamesUnique( new BoilerplateRenamer( compiler.getCodingConvention(), diff --git a/src/com/google/javascript/jscomp/PeepholeFoldConstants.java b/src/com/google/javascript/jscomp/PeepholeFoldConstants.java index 09af61deb43..bf70521f37e 100644 --- a/src/com/google/javascript/jscomp/PeepholeFoldConstants.java +++ b/src/com/google/javascript/jscomp/PeepholeFoldConstants.java @@ -595,7 +595,7 @@ private Node tryFoldAndOr(Node n, Node left, Node right) { // or: false_with_sideeffects && foo() => false_with_sideeffects, foo() // This, combined with PeepholeRemoveDeadCode, helps reduce expressions // like "x() || false || z()". - n.detachChildren(); + NodeUtil.detachChildren(n, compiler); result = IR.comma(left, right); } } @@ -605,7 +605,7 @@ private Node tryFoldAndOr(Node n, Node left, Node right) { if (result != null) { // Fold it! - n.detachChildren(); + NodeUtil.detachChildren(n, compiler); parent.replaceChild(n, result); reportCodeChange(); diff --git a/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java b/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java index fd12fecb522..156c0465dca 100644 --- a/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java +++ b/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java @@ -865,10 +865,7 @@ private Node tryFoldHook(Node n) { boolean condHasSideEffects = mayHaveSideEffects(cond); // Must detach after checking for side effects, to ensure that the parents // of nodes are set correctly. - for (Node child = n.getFirstChild(); child != null; child = child.getNext()) { - NodeUtil.markFunctionsDeleted(child, compiler); - } - n.detachChildren(); + NodeUtil.detachChildren(n, compiler); if (condHasSideEffects) { replacement = IR.comma(cond, branchToKeep).srcref(n); diff --git a/src/com/google/javascript/jscomp/RenameLabels.java b/src/com/google/javascript/jscomp/RenameLabels.java index 6f2cfaa5822..93cebf48b6f 100644 --- a/src/com/google/javascript/jscomp/RenameLabels.java +++ b/src/com/google/javascript/jscomp/RenameLabels.java @@ -69,18 +69,21 @@ final class RenameLabels implements CompilerPass { private final AbstractCompiler compiler; private final Supplier nameSupplier; private final boolean removeUnused; + private final boolean markChanges; RenameLabels(final AbstractCompiler compiler) { - this(compiler, new DefaultNameSupplier(), true); + this(compiler, new DefaultNameSupplier(), true, true); } RenameLabels( AbstractCompiler compiler, Supplier supplier, - boolean removeUnused) { + boolean removeUnused, + boolean markChanges) { this.compiler = compiler; this.nameSupplier = supplier; this.removeUnused = removeUnused; + this.markChanges = markChanges; } static class DefaultNameSupplier implements Supplier { @@ -107,7 +110,10 @@ public String get() { */ class ProcessLabels implements ScopedCallback { - ProcessLabels() { + private final boolean markChanges; + + ProcessLabels(boolean markChanges) { + this.markChanges = markChanges; // Create a entry for global scope. namespaceStack.push(new LabelNamespace()); } @@ -208,7 +214,9 @@ private void visitBreakOrContinue(NodeTraversal t, Node node) { if (!name.equals(newName)) { // Give it the short name. nameNode.setString(newName); - t.reportCodeChange(); + if (markChanges) { + t.reportCodeChange(); + } } } } @@ -230,7 +238,9 @@ private void visitLabel(NodeTraversal t, Node node, Node parent) { if (!name.equals(newName)) { // ... and it is used, give it the short name. nameNode.setString(newName); - t.reportCodeChange(); + if (markChanges) { + t.reportCodeChange(); + } } } else { // ... and it is not referenced, just remove it. @@ -240,7 +250,9 @@ private void visitLabel(NodeTraversal t, Node node, Node parent) { if (newChild.isNormalBlock()) { NodeUtil.tryMergeBlock(newChild); } - t.reportCodeChange(); + if (markChanges) { + t.reportCodeChange(); + } } // Remove the label from the current stack of labels. @@ -268,10 +280,9 @@ LabelInfo getLabelInfo(String name) { @Override public void process(Node externs, Node root) { // Do variable reference counting. - NodeTraversal.traverseEs6(compiler, root, new ProcessLabels()); + NodeTraversal.traverseEs6(compiler, root, new ProcessLabels(markChanges)); } - private static class LabelInfo { boolean referenced = false; final int id; diff --git a/test/com/google/javascript/jscomp/J2clClinitPrunerPassTest.java b/test/com/google/javascript/jscomp/J2clClinitPrunerPassTest.java index b679ae65632..436411b4a6c 100644 --- a/test/com/google/javascript/jscomp/J2clClinitPrunerPassTest.java +++ b/test/com/google/javascript/jscomp/J2clClinitPrunerPassTest.java @@ -21,7 +21,8 @@ public class J2clClinitPrunerPassTest extends CompilerTestCase { @Override protected CompilerPass getProcessor(Compiler compiler) { - return new J2clClinitPrunerPass(compiler); + return new J2clClinitPrunerPass( + compiler, compiler.getChangedScopeNodesForPass("J2clClinitPrunerPass")); } @Override diff --git a/test/com/google/javascript/jscomp/J2clEqualitySameRewriterPassTest.java b/test/com/google/javascript/jscomp/J2clEqualitySameRewriterPassTest.java index d580940c2ab..67f3b715192 100644 --- a/test/com/google/javascript/jscomp/J2clEqualitySameRewriterPassTest.java +++ b/test/com/google/javascript/jscomp/J2clEqualitySameRewriterPassTest.java @@ -30,7 +30,8 @@ protected void setUp() throws Exception { @Override protected CompilerPass getProcessor(Compiler compiler) { - return new J2clEqualitySameRewriterPass(compiler); + return new J2clEqualitySameRewriterPass( + compiler, compiler.getChangedScopeNodesForPass("J2clEqualitySameRewriterPass")); } @Override diff --git a/test/com/google/javascript/jscomp/NodeTraversalTest.java b/test/com/google/javascript/jscomp/NodeTraversalTest.java index ebb94579a2d..e677680abcf 100644 --- a/test/com/google/javascript/jscomp/NodeTraversalTest.java +++ b/test/com/google/javascript/jscomp/NodeTraversalTest.java @@ -25,6 +25,7 @@ import com.google.javascript.jscomp.CompilerOptions.LanguageMode; import com.google.javascript.jscomp.NodeTraversal.AbstractNodeTypePruningCallback; import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback; +import com.google.javascript.jscomp.NodeTraversal.FunctionCallback; import com.google.javascript.rhino.IR; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Token; @@ -479,6 +480,44 @@ public void testTraverseEs6ScopeRoots_parentScopesWork() { "varDefinedInScript", "foo", "bar", "varDefinedInFoo", "baz", "varDefinedInBaz"); } + public void testTraverseEs6ScopeRoots_callsEnterFunction() { + Compiler compiler = new Compiler(); + EnterFunctionAccumulator callback = new EnterFunctionAccumulator(); + + String code = LINE_JOINER.join( + "function foo() {}", + "function bar() {}", + "function baz() {}"); + + Node tree = parse(compiler, code); + Node fooFunction = tree.getFirstChild(); + Node barFunction = fooFunction.getNext(); + Node bazFunction = barFunction.getNext(); + + NodeTraversal.traverseEs6ScopeRoots( + compiler, + fooFunction, + Lists.newArrayList(fooFunction, barFunction, bazFunction), + callback, + callback, // FunctionCallback + false); + assertThat(callback.enteredFunctions).containsExactly(fooFunction, barFunction, bazFunction); + } + + private static final class EnterFunctionAccumulator extends AbstractPostOrderCallback + implements FunctionCallback { + + List enteredFunctions = new ArrayList<>(); + + @Override + public void visit(NodeTraversal t, Node n, Node parent) {} + + @Override + public void enterFunction(AbstractCompiler compiler, Node fnRoot) { + enteredFunctions.add(fnRoot); + } + } + private static final class LexicallyScopedVarsAccumulator extends AbstractPostOrderCallback { final Set varNames = new LinkedHashSet<>();