Some small clean ups to do with DefinitionUseSiteFinder

------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=128539074
google · Jul 27, 2016 · ec16980 · ec16980
1 parent 4487c17
commit ec16980
Show file tree

Hide file tree

Showing 7 changed files with 28 additions and 31 deletions.
diff --git 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
@@ -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
@@ -32,7 +32,6 @@
  */
 public class DefinitionUseSiteFinder extends NameBasedDefinitionProvider {
 
-
   private final Multimap<String, UseSite> 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
@@ -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<Definition> 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
@@ -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
@@ -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,28 +142,25 @@ 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
       // stops running it, so we come to RemoveUnusedVars and the defFinder is
       // 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
@@ -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() {