From ec16980dc61022ae9298af6f5bb045b933b27d5c Mon Sep 17 00:00:00 2001 From: tdeegan Date: Tue, 26 Jul 2016 18:15:45 -0700 Subject: [PATCH] Some small clean ups to do with DefinitionUseSiteFinder ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=128539074 --- .../javascript/jscomp/AbstractCompiler.java | 4 ++-- .../google/javascript/jscomp/Compiler.java | 4 ++-- .../jscomp/DefinitionUseSiteFinder.java | 4 ---- .../jscomp/NameBasedDefinitionProvider.java | 6 ++--- .../javascript/jscomp/OptimizeCalls.java | 2 +- .../javascript/jscomp/RemoveUnusedVars.java | 17 +++++--------- .../jscomp/RemoveUnusedVarsTest.java | 22 +++++++++++++------ 7 files changed, 28 insertions(+), 31 deletions(-) diff --git a/src/com/google/javascript/jscomp/AbstractCompiler.java b/src/com/google/javascript/jscomp/AbstractCompiler.java index e53980f0826..b372533017a 100644 --- a/src/com/google/javascript/jscomp/AbstractCompiler.java +++ b/src/com/google/javascript/jscomp/AbstractCompiler.java @@ -186,9 +186,9 @@ public abstract class AbstractCompiler implements SourceExcerptProvider { * optimize-parameters, remove-unused-variables), to avoid having them * recompute it independently. */ - abstract DefinitionUseSiteFinder getSimpleDefinitionFinder(); + abstract DefinitionUseSiteFinder getDefinitionFinder(); - abstract void setSimpleDefinitionFinder(DefinitionUseSiteFinder defFinder); + abstract void setDefinitionFinder(DefinitionUseSiteFinder defFinder); /** * Parses code for injecting. diff --git a/src/com/google/javascript/jscomp/Compiler.java b/src/com/google/javascript/jscomp/Compiler.java index 1dd8eed5ebd..d10c34b0edc 100644 --- a/src/com/google/javascript/jscomp/Compiler.java +++ b/src/com/google/javascript/jscomp/Compiler.java @@ -1346,12 +1346,12 @@ void setSymbolTable(CompilerPass symbolTable) { } @Override - DefinitionUseSiteFinder getSimpleDefinitionFinder() { + DefinitionUseSiteFinder getDefinitionFinder() { return this.defFinder; } @Override - void setSimpleDefinitionFinder(DefinitionUseSiteFinder defFinder) { + void setDefinitionFinder(DefinitionUseSiteFinder defFinder) { this.defFinder = defFinder; } diff --git a/src/com/google/javascript/jscomp/DefinitionUseSiteFinder.java b/src/com/google/javascript/jscomp/DefinitionUseSiteFinder.java index 3d96a16cfb2..85dcbfcb4c8 100644 --- a/src/com/google/javascript/jscomp/DefinitionUseSiteFinder.java +++ b/src/com/google/javascript/jscomp/DefinitionUseSiteFinder.java @@ -32,7 +32,6 @@ */ public class DefinitionUseSiteFinder extends NameBasedDefinitionProvider { - private final Multimap nameUseSiteMultimap; public DefinitionUseSiteFinder(AbstractCompiler compiler) { @@ -42,9 +41,6 @@ public DefinitionUseSiteFinder(AbstractCompiler compiler) { @Override public void process(Node externs, Node source) { - if (hasProcessBeenRun) { - return; - } super.process(externs, source); NodeTraversal.traverseEs6( compiler, source, new UseSiteGatheringCallback()); diff --git a/src/com/google/javascript/jscomp/NameBasedDefinitionProvider.java b/src/com/google/javascript/jscomp/NameBasedDefinitionProvider.java index 64eadf6f8a9..1cb7c919ec4 100644 --- a/src/com/google/javascript/jscomp/NameBasedDefinitionProvider.java +++ b/src/com/google/javascript/jscomp/NameBasedDefinitionProvider.java @@ -56,9 +56,7 @@ public NameBasedDefinitionProvider(AbstractCompiler compiler) { @Override public void process(Node externs, Node source) { - if (hasProcessBeenRun) { - return; - } + Preconditions.checkState(!hasProcessBeenRun, "The definition provider is already initialized."); this.hasProcessBeenRun = true; NodeTraversal.traverseEs6(compiler, externs, new DefinitionGatheringCallback(true)); @@ -67,11 +65,11 @@ public void process(Node externs, Node source) { @Override public Collection getDefinitionsReferencedAt(Node useSite) { + Preconditions.checkState(hasProcessBeenRun, "The process was not run"); if (definitionSiteMap.containsKey(useSite)) { return null; } - Preconditions.checkState(hasProcessBeenRun, "The process was not run"); if (useSite.isGetProp()) { String propName = useSite.getLastChild().getString(); if (propName.equals("apply") || propName.equals("call")) { diff --git a/src/com/google/javascript/jscomp/OptimizeCalls.java b/src/com/google/javascript/jscomp/OptimizeCalls.java index 4a218103879..c57dfff2fa1 100644 --- a/src/com/google/javascript/jscomp/OptimizeCalls.java +++ b/src/com/google/javascript/jscomp/OptimizeCalls.java @@ -52,8 +52,8 @@ interface CallGraphCompilerPass { public void process(Node externs, Node root) { if (!passes.isEmpty()) { DefinitionUseSiteFinder defFinder = new DefinitionUseSiteFinder(compiler); - compiler.setSimpleDefinitionFinder(defFinder); defFinder.process(externs, root); + compiler.setDefinitionFinder(defFinder); for (CallGraphCompilerPass pass : passes) { pass.process(externs, root, defFinder); } diff --git a/src/com/google/javascript/jscomp/RemoveUnusedVars.java b/src/com/google/javascript/jscomp/RemoveUnusedVars.java index 9affca697db..bc257485422 100644 --- a/src/com/google/javascript/jscomp/RemoveUnusedVars.java +++ b/src/com/google/javascript/jscomp/RemoveUnusedVars.java @@ -120,7 +120,6 @@ class RemoveUnusedVars ArrayListMultimap.create(); private boolean modifyCallSites; - private boolean mustResetModifyCallSites; private CallSiteOptimizer callSiteOptimizer; @@ -134,7 +133,6 @@ class RemoveUnusedVars this.removeGlobals = removeGlobals; this.preserveFunctionExpressionNames = preserveFunctionExpressionNames; this.modifyCallSites = modifyCallSites; - this.mustResetModifyCallSites = false; } /** @@ -144,7 +142,7 @@ class RemoveUnusedVars @Override public void process(Node externs, Node root) { Preconditions.checkState(compiler.getLifeCycleStage().isNormalized()); - DefinitionUseSiteFinder defFinder = compiler.getSimpleDefinitionFinder(); + boolean shouldResetModifyCallSites = false; if (this.modifyCallSites) { // When RemoveUnusedVars is run after OptimizeCalls, this.modifyCallSites // is true. But if OptimizeCalls stops making changes, PhaseOptimizer @@ -152,20 +150,17 @@ public void process(Node externs, Node root) { // null. In this case, we temporarily set this.modifyCallSites to false // for this run, and then reset it back to true at the end, for // subsequent runs. - if (defFinder == null) { + if (compiler.getDefinitionFinder() == null) { this.modifyCallSites = false; - this.mustResetModifyCallSites = true; - } else { - defFinder.process(externs, root); + shouldResetModifyCallSites = true; } } - process(externs, root, defFinder); + process(externs, root, compiler.getDefinitionFinder()); // When doing OptimizeCalls, RemoveUnusedVars is the last pass in the // sequence, so the def finder must not be used by any subsequent passes. - compiler.setSimpleDefinitionFinder(null); - if (this.mustResetModifyCallSites) { + compiler.setDefinitionFinder(null); + if (shouldResetModifyCallSites) { this.modifyCallSites = true; - this.mustResetModifyCallSites = false; } } diff --git a/test/com/google/javascript/jscomp/RemoveUnusedVarsTest.java b/test/com/google/javascript/jscomp/RemoveUnusedVarsTest.java index 6e30c6f5f7f..b419fb8fdb4 100644 --- a/test/com/google/javascript/jscomp/RemoveUnusedVarsTest.java +++ b/test/com/google/javascript/jscomp/RemoveUnusedVarsTest.java @@ -17,6 +17,8 @@ package com.google.javascript.jscomp; +import com.google.javascript.rhino.Node; + public final class RemoveUnusedVarsTest extends CompilerTestCase { private boolean removeGlobal; @@ -38,13 +40,19 @@ public void setUp() { @Override protected CompilerPass getProcessor(final Compiler compiler) { - if (this.modifyCallSites) { - DefinitionUseSiteFinder defFinder = new DefinitionUseSiteFinder(compiler); - compiler.setSimpleDefinitionFinder(defFinder); - } - return new RemoveUnusedVars( - compiler, removeGlobal, preserveFunctionExpressionNames, - modifyCallSites); + return new CompilerPass() { + @Override + public void process(Node externs, Node root) { + if (modifyCallSites) { + DefinitionUseSiteFinder defFinder = new DefinitionUseSiteFinder(compiler); + defFinder.process(externs, root); + compiler.setDefinitionFinder(defFinder); + } + new RemoveUnusedVars( + compiler, removeGlobal, preserveFunctionExpressionNames, + modifyCallSites).process(externs, root); + } + }; } public void testRemoveUnusedVars() {