Rollback "Complete replacement of RUPP with RemoveUnusedCode."

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=178700650

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

@@ -2296,7 +2296,7 @@ public void setExtractPrototypeMemberDeclarations(ExtractPrototypeMemberDeclarat
   public void setRemoveUnusedPrototypeProperties(boolean enabled) {
     this.removeUnusedPrototypeProperties = enabled;
     // InlineSimpleMethods makes similar assumptions to
-    // RemoveUnusedCode, so they are enabled together.
+    // RemoveUnusedPrototypeProperties, so they are enabled together.
     this.inlineGetters = enabled;
   }
 

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

@@ -820,8 +820,8 @@ protected List<PassFactory> getOptimizations() {
 
       // After inlining some of the variable uses, some variables are unused.
       // Re-run remove unused vars to clean it up.
-      if (shouldRunRemoveUnusedCode()) {
-        passes.add(removeUnusedCodeOnce);
+      if (options.removeUnusedVars || options.removeUnusedLocalVars) {
+        passes.add(getRemoveUnusedCodeOnce());
       }
     }
 
@@ -1007,6 +1007,10 @@ private List<PassFactory> getMainOptimizationLoop() {
       passes.add(optimizeCalls);
     }
 
+    if (options.removeUnusedVars || options.removeUnusedLocalVars) {
+      passes.add(getRemoveUnusedCode());
+    }
+
     if (options.j2clPassMode.shouldAddJ2clPasses()) {
       passes.add(j2clConstantHoisterPass);
       passes.add(j2clClinitPass);
@@ -1037,8 +1041,8 @@ private List<PassFactory> getCodeRemovingPasses() {
       passes.add(removeUnreachableCode);
     }
 
-    if (shouldRunRemoveUnusedCode()) {
-      passes.add(removeUnusedCode);
+    if (options.removeUnusedPrototypeProperties) {
+      passes.add(removeUnusedPrototypeProperties);
     }
 
     if (options.removeUnusedClassProperties) {
@@ -1049,12 +1053,6 @@ private List<PassFactory> getCodeRemovingPasses() {
     return passes;
   }
 
-  private boolean shouldRunRemoveUnusedCode() {
-    return options.removeUnusedVars
-        || options.removeUnusedLocalVars
-        || options.removeUnusedPrototypeProperties;
-  }
-
   private final HotSwapPassFactory checkSideEffects =
       new HotSwapPassFactory("checkSideEffects") {
         @Override
@@ -2712,6 +2710,23 @@ protected FeatureSet featureSet() {
     }
   };
 
+  /** Remove prototype properties that do not appear to be used. */
+  private final PassFactory removeUnusedPrototypeProperties =
+      new PassFactory(PassNames.REMOVE_UNUSED_PROTOTYPE_PROPERTIES, false) {
+        @Override
+        protected CompilerPass create(AbstractCompiler compiler) {
+          return new RemoveUnusedPrototypeProperties(
+              compiler,
+              options.removeUnusedPrototypePropertiesInExterns,
+              !options.removeUnusedVars);
+        }
+
+        @Override
+        protected FeatureSet featureSet() {
+          return ES8_MODULES;
+        }
+      };
+
   /** Remove prototype properties that do not appear to be used. */
   private final PassFactory removeUnusedClassProperties =
       new PassFactory(PassNames.REMOVE_UNUSED_CLASS_PROPERTIES, false) {
@@ -2865,19 +2880,24 @@ protected FeatureSet featureSet() {
         }
       };
 
-  private PassFactory removeUnusedCodeOnce = getRemoveUnusedCode(true /* isOneTimePass */);
-  private PassFactory removeUnusedCode = getRemoveUnusedCode(false /* isOneTimePass */);
+  private PassFactory getRemoveUnusedCode() {
+    return getRemoveUnusedCode(false /* isOneTimePass */);
+  }
+
+  private PassFactory getRemoveUnusedCodeOnce() {
+    return getRemoveUnusedCode(true /* isOneTimePass */);
+  }
 
   private PassFactory getRemoveUnusedCode(boolean isOneTimePass) {
     /** Removes variables that are never used. */
     return new PassFactory(PassNames.REMOVE_UNUSED_CODE, isOneTimePass) {
       @Override
       protected CompilerPass create(AbstractCompiler compiler) {
+        boolean removeOnlyLocals = options.removeUnusedLocalVars && !options.removeUnusedVars;
         boolean preserveAnonymousFunctionNames =
             options.anonymousFunctionNaming != AnonymousFunctionNamingPolicy.OFF;
         return new RemoveUnusedCode.Builder(compiler)
-            .removeLocalVars(options.removeUnusedLocalVars)
-            .removeGlobals(options.removeUnusedVars)
+            .removeGlobals(!removeOnlyLocals)
             .preserveFunctionExpressionNames(preserveAnonymousFunctionNames)
             .removeUnusedPrototypeProperties(options.removeUnusedPrototypeProperties)
             .allowRemovalOfExternProperties(options.removeUnusedPrototypePropertiesInExterns)

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

@@ -32,7 +32,7 @@
  * constructors or interfaces. Explicitly ignored is the possibility that
  * these properties may be indirectly referenced using "for-in" or
  * "Object.keys".  This is the same assumption used with
- * RemoveUnusedCode but is slightly wider in scope.
+ * RemoveUnusedPrototypeProperties but is slightly wider in scope.
  *
  * TODO(tomnguyen) Handle destructuring of objects/classes as cases where the field is used.
  *

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

@@ -96,7 +96,6 @@ class RemoveUnusedCode implements CompilerPass {
 
   private final CodingConvention codingConvention;
 
-  private final boolean removeLocalVars;
   private final boolean removeGlobals;
 
   private final boolean preserveFunctionExpressionNames;
@@ -119,7 +118,7 @@ class RemoveUnusedCode implements CompilerPass {
   private final Multimap<String, Removable> removablesForPropertyNames = HashMultimap.create();
 
   /** Single value to use for all vars for which we cannot remove anything at all. */
-  private final VarInfo canonicalUnremovableVarInfo;
+  private final VarInfo canonicalTotallyUnremovableVarInfo;
 
   /**
    * Keep track of scopes that we've traversed.
@@ -134,22 +133,20 @@ class RemoveUnusedCode implements CompilerPass {
   RemoveUnusedCode(Builder builder) {
     this.compiler = builder.compiler;
     this.codingConvention = builder.compiler.getCodingConvention();
-    this.removeLocalVars = builder.removeLocalVars;
     this.removeGlobals = builder.removeGlobals;
     this.preserveFunctionExpressionNames = builder.preserveFunctionExpressionNames;
     this.removeUnusedPrototypeProperties = builder.removeUnusedPrototypeProperties;
     this.allowRemovalOfExternProperties = builder.allowRemovalOfExternProperties;
     this.scopeCreator = new Es6SyntacticScopeCreator(builder.compiler);
 
     // All Vars that are completely unremovable will share this VarInfo instance.
-    canonicalUnremovableVarInfo = new VarInfo();
-    canonicalUnremovableVarInfo.setIsExplicitlyNotRemovable();
+    canonicalTotallyUnremovableVarInfo = new VarInfo();
+    canonicalTotallyUnremovableVarInfo.setIsExplicitlyNotRemovable();
   }
 
   public static class Builder {
     private final AbstractCompiler compiler;
 
-    private boolean removeLocalVars = false;
     private boolean removeGlobals = false;
     private boolean preserveFunctionExpressionNames = false;
     private boolean removeUnusedPrototypeProperties = false;
@@ -159,11 +156,6 @@ public static class Builder {
       this.compiler = compiler;
     }
 
-    Builder removeLocalVars(boolean value) {
-      this.removeLocalVars = value;
-      return this;
-    }
-
     Builder removeGlobals(boolean value) {
       this.removeGlobals = value;
       return this;
@@ -1093,8 +1085,7 @@ private void maybeRemoveUnusedTrailingParameters(Node argList, Scope fparamScope
    */
   private VarInfo traverseVar(Var var) {
     checkNotNull(var);
-    if (removeLocalVars && var.isArguments()) {
-      // If we are considering removing local variables, that includes parameters.
+    if (var.isArguments()) {
       // If `arguments` is used in a function we must consider all parameters to be referenced.
       Scope functionScope = var.getScope().getClosestHoistScope();
       Node paramList = NodeUtil.getFunctionParameters(functionScope.getRootNode());
@@ -1113,7 +1104,7 @@ private VarInfo traverseVar(Var var) {
         getVarInfo(paramVar).markAsReferenced();
       }
       // `arguments` is never removable.
-      return canonicalUnremovableVarInfo;
+      return canonicalTotallyUnremovableVarInfo;
     } else {
       return getVarInfo(var);
     }
@@ -1128,28 +1119,26 @@ private VarInfo traverseVar(Var var) {
    */
   private VarInfo getVarInfo(Var var) {
     checkNotNull(var);
-    boolean isGlobal = var.isGlobal();
-    if (isGlobal && !removeGlobals) {
-      return canonicalUnremovableVarInfo;
-    } else if (!isGlobal && !removeLocalVars) {
-      return canonicalUnremovableVarInfo;
-    } else if (codingConvention.isExported(var.getName(), !isGlobal)) {
-      return canonicalUnremovableVarInfo;
-    } else if (var.isArguments()) {
-      return canonicalUnremovableVarInfo;
-    } else {
-      VarInfo varInfo = varInfoMap.get(var);
-      if (varInfo == null) {
+    VarInfo varInfo = varInfoMap.get(var);
+    if (varInfo == null) {
+      boolean isGlobal = var.isGlobal();
+      if (isGlobal && !removeGlobals && !removeUnusedPrototypeProperties) {
+        varInfo = canonicalTotallyUnremovableVarInfo;
+      } else if (codingConvention.isExported(var.getName(), !isGlobal)) {
+        varInfo = canonicalTotallyUnremovableVarInfo;
+      } else if (var.isArguments()) {
+        varInfo = canonicalTotallyUnremovableVarInfo;
+      } else {
         varInfo = new VarInfo();
-        if (var.getParentNode().isParamList()) {
-          // We don't know where a parameter value comes from, so setting a property on it
-          // has unknown side effects and makes it not removable.
+        if (isGlobal && !removeGlobals) {
+          varInfo.setIsExplicitlyNotRemovable();
+        } else if (var.getParentNode().isParamList()) {
           varInfo.propertyAssignmentsWillPreventRemoval = true;
         }
         varInfoMap.put(var, varInfo);
       }
-      return varInfo;
     }
+    return varInfo;
   }
 
   /**

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

@@ -0,0 +1,89 @@
+/*
+ * Copyright 2006 The Closure Compiler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.javascript.jscomp;
+
+import com.google.javascript.jscomp.AnalyzePrototypeProperties.NameInfo;
+import com.google.javascript.jscomp.AnalyzePrototypeProperties.Symbol;
+import com.google.javascript.rhino.Node;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+/**
+ * Removes unused properties from prototypes.
+ *
+ * NOTE: When canModifyExterns is true, this pass removes unused prototype properties
+ * in externs. This helped code size in the past, before type-based optimizations existed
+ * (and can help now for things that are not disambiguated), but is unsafe.
+ * For example, it can remove a polyfilled method that is not used in the source, but is
+ * used by the external code, so the call is not visible to the compiler.
+ * Therefore, the default for canModifyExterns is false, even though that increases code size
+ * for some projects.
+ *
+ * @author nicksantos@google.com (Nick Santos)
+ */
+class RemoveUnusedPrototypeProperties implements CompilerPass {
+
+  private static final Logger logger =
+    Logger.getLogger(RemoveUnusedPrototypeProperties.class.getName());
+
+  private final AbstractCompiler compiler;
+  private final boolean canModifyExterns;
+  private final boolean anchorUnusedVars;
+
+  /**
+   * Creates a new pass for removing unused prototype properties, based
+   * on the uniqueness of property names.
+   * @param compiler The compiler.
+   * @param canModifyExterns If true, then we can remove prototype
+   *     properties that are declared in the externs file.
+   * @param anchorUnusedVars If true, then we must keep unused variables
+   *     and the prototype properties they reference, even if they are
+   *     never used.
+   */
+  RemoveUnusedPrototypeProperties(AbstractCompiler compiler,
+      boolean canModifyExterns,
+      boolean anchorUnusedVars) {
+    this.compiler = compiler;
+    this.canModifyExterns = canModifyExterns;
+    this.anchorUnusedVars = anchorUnusedVars;
+  }
+
+  @Override
+  public void process(Node externRoot, Node root) {
+    AnalyzePrototypeProperties analyzer =
+        new AnalyzePrototypeProperties(
+            compiler,
+            null /* no module graph */,
+            canModifyExterns,
+            anchorUnusedVars,
+            false /* rootScopeUsesAreGlobal */);
+    analyzer.process(externRoot, root);
+    // Remove all properties under a given name if the property name is
+    // never referenced.
+    for (NameInfo nameInfo : analyzer.getAllNameInfo()) {
+      if (!nameInfo.isReferenced()) {
+        for (Symbol declaration : nameInfo.getDeclarations()) {
+          // Code-change reporting happens at the remove methods
+          declaration.remove(compiler);
+        }
+        if (logger.isLoggable(Level.FINE)) {
+          logger.fine("Removed unused prototype property: " + nameInfo.name);
+        }
+      }
+    }
+  }
+}

test/com/google/javascript/jscomp/MultiPassTest.java

@@ -425,7 +425,7 @@ private void addRemoveUnusedVars() {
         new PassFactory("removeUnusedVars", false) {
           @Override
           protected CompilerPass create(AbstractCompiler compiler) {
-            return new RemoveUnusedCode.Builder(compiler).removeLocalVars(true).build();
+            return new RemoveUnusedCode.Builder(compiler).build();
           }
 
           @Override

test/com/google/javascript/jscomp/OptimizeCallsTest.java

@@ -50,11 +50,7 @@ public void process(Node externs, Node root) {
         defFinder.process(externs, root);
 
         new PureFunctionIdentifier(compiler, defFinder).process(externs, root);
-        new RemoveUnusedCode.Builder(compiler)
-            .removeLocalVars(true)
-            .removeGlobals(true)
-            .build()
-            .process(externs, root);
+        new RemoveUnusedCode.Builder(compiler).removeGlobals(true).build().process(externs, root);
 
         final OptimizeCalls passes = new OptimizeCalls(compiler);
         passes.addPass(new OptimizeReturns(compiler));