Remove two boolean arguments to the InlineFunctions pass constructor.

Replace the inlineGlobalFunctions and inlineLocalFunctions booleans with a single Reach enum instead. Once remaining references to the public booleans in the CompilerOptions are removed, we can clean up the representation in CompilerOptions as well. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=179122024
google · Dec 15, 2017 · 3d9ecb4 · 3d9ecb4
1 parent 110b3b7
commit 3d9ecb4
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 56 deletions.
diff --git a/src/com/google/javascript/jscomp/CompilerOptions.java b/src/com/google/javascript/jscomp/CompilerOptions.java
@@ -72,7 +72,11 @@ public class CompilerOptions implements Serializable {
   public enum Reach {
     ALL,
     LOCAL_ONLY,
-    NONE
+    NONE;
+
+    boolean includesGlobals() {
+      return this == ALL;
+    }
   }
 
   public enum PropertyCollapseLevel {
@@ -428,19 +432,13 @@ public void setNumParallelThreads(int parallelism) {
   /** Inlines constants (symbols that are all CAPS) */
   public boolean inlineConstantVars;
 
-  /** Inlines global functions */
-  public boolean inlineFunctions;
-
   /**
    * For projects that want to avoid the creation of giant functions after
    * inlining.
    */
   int maxFunctionSizeAfterInlining;
   static final int UNLIMITED_FUN_SIZE_AFTER_INLINING = -1;
 
-  /** Inlines functions defined in local scopes */
-  public boolean inlineLocalFunctions;
-
   /** More aggressive function inlining */
   boolean assumeClosuresOnlyCaptureReferences;
 
@@ -1633,6 +1631,27 @@ public void setIdGeneratorsMap(String previousMappings) {
     this.idGeneratorsMapSerialized = previousMappings;
   }
 
+  /** Use {@link #getInlineFunctionsLevel()} instead */
+  @Deprecated
+  public boolean inlineFunctions;
+
+  /** Use {@link #getInlineFunctionsLevel()} instead */
+  @Deprecated
+  public boolean inlineLocalFunctions;
+
+  /** Use {@link #setInlineFunctions(Reach)} instead */
+  @Deprecated
+  public void setInlineFunctions(boolean inlineFunctions) {
+    this.inlineFunctions = inlineFunctions;
+  }
+
+  /** Use {@link #setInlineFunctions(Reach)} instead */
+  @Deprecated
+  public void setInlineLocalFunctions(boolean inlineLocalFunctions) {
+    this.inlineLocalFunctions = inlineLocalFunctions;
+  }
+
+
   /**
    * Set the function inlining policy for the compiler.
    */
@@ -1655,6 +1674,19 @@ public void setInlineFunctions(Reach reach) {
     }
   }
 
+  /**
+   * Get the function inlining policy for the compiler.
+   */
+  public Reach getInlineFunctionsLevel() {
+    if (this.inlineFunctions) {
+      return Reach.ALL;
+    }
+    if (this.inlineLocalFunctions) {
+      return Reach.LOCAL_ONLY;
+    }
+    return Reach.NONE;
+  }
+
   public void setMaxFunctionSizeAfterInlining(int funAstSize) {
     checkArgument(funAstSize > 0);
     this.maxFunctionSizeAfterInlining = funAstSize;
@@ -2226,14 +2258,6 @@ public void setInlineConstantVars(boolean inlineConstantVars) {
     this.inlineConstantVars = inlineConstantVars;
   }
 
-  public void setInlineFunctions(boolean inlineFunctions) {
-    this.inlineFunctions = inlineFunctions;
-  }
-
-  public void setInlineLocalFunctions(boolean inlineLocalFunctions) {
-    this.inlineLocalFunctions = inlineLocalFunctions;
-  }
-
   public void setCrossModuleCodeMotion(boolean crossModuleCodeMotion) {
     this.crossModuleCodeMotion = crossModuleCodeMotion;
   }

diff --git a/src/com/google/javascript/jscomp/CompilerOptionsPreprocessor.java b/src/com/google/javascript/jscomp/CompilerOptionsPreprocessor.java
@@ -48,12 +48,11 @@ static void preprocess(CompilerOptions options) {
           + "remove_unused_prototype_props to be turned on.");
     }
 
-    if (!options.inlineFunctions
+    if (options.getInlineFunctionsLevel() == CompilerOptions.Reach.NONE
         && options.maxFunctionSizeAfterInlining
-        != CompilerOptions.UNLIMITED_FUN_SIZE_AFTER_INLINING) {
+            != CompilerOptions.UNLIMITED_FUN_SIZE_AFTER_INLINING) {
       throw new InvalidOptionsException(
-          "max_function_size_after_inlining has no effect if inlining is"
+          "max_function_size_after_inlining has no effect if inlining is disabled.");
-          + " disabled.");
     }
 
     if (options.getNewTypeInference()) {

diff --git a/src/com/google/javascript/jscomp/DefaultPassConfig.java b/src/com/google/javascript/jscomp/DefaultPassConfig.java
@@ -34,6 +34,7 @@
 import com.google.javascript.jscomp.AbstractCompiler.LifeCycleStage;
 import com.google.javascript.jscomp.AbstractCompiler.MostRecentTypechecker;
 import com.google.javascript.jscomp.CompilerOptions.ExtractPrototypeMemberDeclarationsMode;
+import com.google.javascript.jscomp.CompilerOptions.Reach;
 import com.google.javascript.jscomp.CoverageInstrumentationPass.CoverageReach;
 import com.google.javascript.jscomp.CoverageInstrumentationPass.InstrumentOption;
 import com.google.javascript.jscomp.ExtractPrototypeMemberDeclarations.Pattern;
@@ -982,7 +983,7 @@ private List<PassFactory> getMainOptimizationLoop() {
 
     passes.addAll(getCodeRemovingPasses());
 
-    if (options.inlineFunctions || options.inlineLocalFunctions) {
+    if (options.getInlineFunctionsLevel() != Reach.NONE) {
       passes.add(inlineFunctions);
     }
 
@@ -2821,8 +2822,7 @@ protected CompilerPass create(AbstractCompiler compiler) {
           return new InlineFunctions(
               compiler,
               compiler.getUniqueNameIdSupplier(),
-              options.inlineFunctions,
+              options.getInlineFunctionsLevel(),
-              options.inlineLocalFunctions,
               options.assumeStrictThis() || options.expectStrictModeInput(),
               options.assumeClosuresOnlyCaptureReferences,
               options.maxFunctionSizeAfterInlining);

diff --git a/src/com/google/javascript/jscomp/InlineFunctions.java b/src/com/google/javascript/jscomp/InlineFunctions.java
@@ -24,10 +24,10 @@
 import com.google.common.base.Predicates;
 import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableSet;
+import com.google.javascript.jscomp.CompilerOptions.Reach;
 import com.google.javascript.jscomp.FunctionInjector.CanInlineResult;
 import com.google.javascript.jscomp.FunctionInjector.InliningMode;
 import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
-import com.google.javascript.jscomp.NodeTraversal.Callback;
 import com.google.javascript.rhino.Node;
 import java.util.Collection;
 import java.util.Collections;
@@ -67,8 +67,7 @@ class InlineFunctions implements CompilerPass {
 
   private final FunctionInjector injector;
 
-  private final boolean inlineGlobalFunctions;
+  private final Reach reach;
-  private final boolean inlineLocalFunctions;
   private final boolean assumeMinimumCapture;
 
   private final boolean enforceMaxSizeAfterInlining;
@@ -77,17 +76,17 @@ class InlineFunctions implements CompilerPass {
   InlineFunctions(
       AbstractCompiler compiler,
       Supplier<String> safeNameIdSupplier,
-      boolean inlineGlobalFunctions,
+      Reach reach,
-      boolean inlineLocalFunctions,
       boolean assumeStrictThis,
       boolean assumeMinimumCapture,
       int maxSizeAfterInlining) {
     checkArgument(compiler != null);
     checkArgument(safeNameIdSupplier != null);
+    checkArgument(reach != Reach.NONE);
+
     this.compiler = compiler;
 
-    this.inlineGlobalFunctions = inlineGlobalFunctions;
+    this.reach = reach;
-    this.inlineLocalFunctions = inlineLocalFunctions;
     this.assumeMinimumCapture = assumeMinimumCapture;
 
     this.maxSizeAfterInlining = maxSizeAfterInlining;
@@ -169,22 +168,13 @@ private boolean targetSizeAfterInlineExceedsLimit(NodeTraversal t, FunctionState
   }
 
   /** Find functions that might be inlined. */
-  private class FindCandidateFunctions implements Callback {
+  private class FindCandidateFunctions extends AbstractPostOrderCallback {
     private int callsSeen = 0;
 
-    @Override
-    public boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent) {
-      // Don't traverse into function bodies
-      // if we aren't inlining local functions.
-      return inlineLocalFunctions || nodeTraversal.inGlobalHoistScope();
-    }
-
     @Override
     public void visit(NodeTraversal t, Node n, Node parent) {
-      if ((t.inGlobalHoistScope() && inlineGlobalFunctions)
+      if (reach.includesGlobals() || !t.inGlobalHoistScope()) {
-          || (!t.inGlobalHoistScope() && inlineLocalFunctions)) {
         findNamedFunctions(t, n, parent);
-
         findFunctionExpressions(t, n);
       }
     }

diff --git a/src/com/google/javascript/jscomp/debugger/common/CompilationParam.java b/src/com/google/javascript/jscomp/debugger/common/CompilationParam.java
@@ -402,13 +402,12 @@ public String getJavaInfo() {
   INLINE_FUNCTIONS(ParamGroup.TYPE_CHECKING_OPTIMIZATION) {
     @Override
     public void apply(CompilerOptions options, boolean value) {
-      options.setInlineFunctions(value);
+      options.setInlineFunctions(value ? Reach.ALL : Reach.NONE);
-      options.setInlineLocalFunctions(value);
     }
 
     @Override
     public boolean isApplied(CompilerOptions options) {
-      return options.inlineLocalFunctions && options.inlineFunctions;
+      return options.getInlineFunctionsLevel() == Reach.ALL;
     }
 
     @Override

diff --git a/test/com/google/javascript/jscomp/InlineFunctionsTest.java b/test/com/google/javascript/jscomp/InlineFunctionsTest.java
@@ -16,17 +16,17 @@
 
 package com.google.javascript.jscomp;
 
+import com.google.javascript.jscomp.CompilerOptions.Reach;
 
 /**
  * Inline function tests.
  * @author johnlenz@google.com (john lenz)
  */
 
 public class InlineFunctionsTest extends CompilerTestCase {
-  boolean allowGlobalFunctionInlining;
+  Reach inliningReach;
   final boolean allowExpressionDecomposition = true;
   final boolean allowFunctionExpressionInlining = true;
-  final boolean allowLocalFunctionInlining = true;
   boolean assumeStrictThis;
   boolean assumeMinimumCapture;
   int maxSizeAfterInlining;
@@ -48,7 +48,7 @@ protected void setUp() throws Exception {
     maybeEnableInferConsts();
     enableNormalize();
     enableComputeSideEffects();
-    allowGlobalFunctionInlining = true;
+    inliningReach = Reach.ALL;
     assumeStrictThis = false;
     assumeMinimumCapture = false;
     maxSizeAfterInlining = CompilerOptions.UNLIMITED_FUN_SIZE_AFTER_INLINING;
@@ -61,8 +61,7 @@ protected CompilerPass getProcessor(Compiler compiler) {
     return new InlineFunctions(
         compiler,
         compiler.getUniqueNameIdSupplier(),
-        allowGlobalFunctionInlining,
+        inliningReach,
-        allowLocalFunctionInlining,
         assumeStrictThis,
         assumeMinimumCapture,
         maxSizeAfterInlining);
@@ -1946,30 +1945,30 @@ public void testLocalFunctionInlining6() {
   }
 
   public void testLocalFunctionInliningOnly1() {
-    this.allowGlobalFunctionInlining = true;
+    this.inliningReach = Reach.ALL;
     test("function f(){} f()", "void 0;");
-    this.allowGlobalFunctionInlining = false;
+    this.inliningReach = Reach.LOCAL_ONLY;
     testSame("function f(){} f()");
   }
 
   public void testLocalFunctionInliningOnly2() {
-    this.allowGlobalFunctionInlining = false;
+    this.inliningReach = Reach.LOCAL_ONLY;
     testSame("function f(){} f()");
 
     test("function f(){ function g() {return 1} return g() }; f();",
          "function f(){ return 1 }; f();");
   }
 
   public void testLocalFunctionInliningOnly3() {
-    this.allowGlobalFunctionInlining = false;
+    this.inliningReach = Reach.LOCAL_ONLY;
     testSame("function f(){} f()");
 
     test("(function(){ function g() {return 1} return g() })();",
          "(function(){ return 1 })();");
   }
 
   public void testLocalFunctionInliningOnly4() {
-    this.allowGlobalFunctionInlining = false;
+    this.inliningReach = Reach.LOCAL_ONLY;
     testSame("function f(){} f()");
 
     test("(function(){ return (function() {return 1})() })();",
@@ -2195,7 +2194,7 @@ public void testInlineObject() {
     disableCompareAsTree();
     enableMarkNoSideEffects();
 
-    allowGlobalFunctionInlining = false;
+    this.inliningReach = Reach.LOCAL_ONLY;
     assumeStrictThis = true;
     assumeMinimumCapture = true;
 

diff --git a/test/com/google/javascript/jscomp/MultiPassTest.java b/test/com/google/javascript/jscomp/MultiPassTest.java
@@ -353,8 +353,11 @@ private void addInlineFunctions() {
           @Override
           protected CompilerPass create(AbstractCompiler compiler) {
             return new InlineFunctions(
-                compiler, compiler.getUniqueNameIdSupplier(),
+                compiler,
-                true, true, true, true,
+                compiler.getUniqueNameIdSupplier(),
+                CompilerOptions.Reach.ALL,
+                true,
+                true,
                 CompilerOptions.UNLIMITED_FUN_SIZE_AFTER_INLINING);
           }