RFC: Fold typeof and Array.isArray() using colors

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

@@ -23,9 +23,13 @@
 import com.google.javascript.jscomp.base.Tri;
 import com.google.javascript.jscomp.parsing.parser.FeatureSet;
 import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature;
+import com.google.javascript.rhino.IR;
 import com.google.javascript.rhino.Node;
+import com.google.javascript.rhino.Token;
 import java.math.BigInteger;
+import java.util.ArrayDeque;
 import java.util.LinkedHashSet;
+import org.jspecify.annotations.Nullable;
 
 /**
  * An abstract class whose implementations run peephole optimizations:
@@ -213,6 +217,239 @@ protected boolean nodeTypeMayHaveSideEffects(Node n) {
     return astAnalyzer.nodeTypeMayHaveSideEffects(n);
   }
 
+  /**
+   * Replaces an {@code expression} with a provided {@code value}, which must have been obtained by
+   * evaluating the expression. The present function ensures that the side effects of the {@code
+   * expression} is preserved after the replacement.
+   *
+   * <p>This is achieved by substituting the original expression with the comma node (expr, value)
+   * and then simplify the first child using {@link trySimplifyUnusedResult()}.
+   *
+   * <p>If the provided expression is a call such as `fn(innerExpr)`, we require that fn has no side
+   * effects. This is because if fn had side effects, no simplifications can occur and one ends up
+   * with (fn(innerExpr), value), which is less optimized than the initial expression.
+   *
+   * @return The replacement expression, which can be of the form `(..., value)` or just `value`.
+   */
+  protected final Node replaceExpressionWithEvalResult(Node expr, Node value) {
+    checkState(expr.hasParent(), expr);
+    checkState(!value.hasParent(), value);
+    if (expr.isCall() || expr.isOptChainCall()) {
+      checkState(expr.isNoSideEffectsCall(), expr);
+    }
+    Node comma = new Node(Token.COMMA, value.srcref(expr));
+    expr.replaceWith(comma);
+    comma.addChildToFront(expr); // comma = (expr, value)
+    expr = trySimplifyUnusedResult(expr);
+    if (expr == null || !mayHaveSideEffects(expr)) {
+      value.detach();
+      comma.replaceWith(value);
+      if (expr != null) {
+        markFunctionsDeleted(expr);
+      }
+      expr = value; // expr = value
+    } else {
+      expr = comma; // expr = (..., value)
+    }
+    reportChangeToEnclosingScope(expr);
+    return expr;
+  }
+
+  /**
+   * Replaces {@code expression} with an expression that contains only side-effects of the original.
+   *
+   * <p>This replacement is made under the assumption that the result of {@code expression} is
+   * unused and therefore it is correct to eliminate non-side-effectful nodes.
+   *
+   * @return The replacement expression, or {@code null} if there were no side-effects to preserve.
+   */
+  protected final @Nullable Node trySimplifyUnusedResult(Node expression) {
+    ArrayDeque<Node> sideEffectRoots = new ArrayDeque<>();
+    boolean atFixedPoint = trySimplifyUnusedResultInternal(expression, sideEffectRoots);
+
+    if (atFixedPoint) {
+      // `expression` is in a form that cannot be further optimized.
+      return expression;
+    } else if (sideEffectRoots.isEmpty()) {
+      deleteNode(expression);
+      return null;
+    } else if (sideEffectRoots.peekFirst() == expression) {
+      // Expression was a conditional that was transformed. There can't be any other side-effects,
+      // but we also can't detach the transformed root.
+      checkState(sideEffectRoots.size() == 1, sideEffectRoots);
+      reportChangeToEnclosingScope(expression);
+      return expression;
+    } else {
+      Node sideEffects = asDetachedExpression(sideEffectRoots.pollFirst());
+
+      // Assemble a tree of comma expressions for all the side-effects. The tree must execute the
+      // side-effects in FIFO order with respect to the queue. It must also be left leaning to match
+      // the parser's preferred structure.
+      while (!sideEffectRoots.isEmpty()) {
+        Node next = asDetachedExpression(sideEffectRoots.pollFirst());
+        sideEffects = IR.comma(sideEffects, next).srcref(next);
+      }
+
+      sideEffects.insertBefore(expression);
+      deleteNode(expression);
+      return sideEffects;
+    }
+  }
+
+  /**
+   * Collects any potentially side-effectful subtrees within {@code tree} into {@code
+   * sideEffectRoots}.
+   *
+   * <p>When a node is determined to have side-effects its descendants are not explored. This method
+   * assumes the entire subtree of such a node must be preserved. As a corollary, the contents of
+   * {@code sideEffectRoots} are a forest.
+   *
+   * <p>This operation generally does not mutate {@code tree}; however, exceptions are made for
+   * expressions that alter control-flow. Such expression will be pruned of their side-effectless
+   * branches. Even in this case, {@code tree} is never detached.
+   *
+   * @param sideEffectRoots The roots of subtrees determined to have side-effects, in execution
+   *     order.
+   * @return {@code true} iff there is no code to be removed from within {@code tree}; it is already
+   *     at a fixed point for code removal.
+   */
+  private boolean trySimplifyUnusedResultInternal(Node tree, ArrayDeque<Node> sideEffectRoots) {
+    // Special cases for conditional expressions that may be using results.
+    switch (tree.getToken()) {
+      case HOOK -> {
+        // Try to remove one or more of the conditional children and transform the HOOK to an
+        // equivalent operation. Remember that if either value branch still exists, the result of
+        // the predicate expression is being used, and so cannot be removed.
+        //    x() ? foo() : 1 --> x() && foo()
+        //    x() ? 1 : foo() --> x() || foo()
+        //    x() ? 1 : 1 --> x()
+        //    x ? 1 : 1 --> null
+        Node trueNode = trySimplifyUnusedResult(tree.getSecondChild());
+        Node falseNode = trySimplifyUnusedResult(tree.getLastChild());
+        if (trueNode == null && falseNode != null) {
+          checkState(tree.hasTwoChildren(), tree);
+
+          tree.setToken(Token.OR);
+          sideEffectRoots.addLast(tree);
+          return false; // The node type was changed.
+        } else if (trueNode != null && falseNode == null) {
+          checkState(tree.hasTwoChildren(), tree);
+
+          tree.setToken(Token.AND);
+          sideEffectRoots.addLast(tree);
+          return false; // The node type was changed.
+        } else if (trueNode == null && falseNode == null) {
+          // Don't bother adding true and false branch children to make the AST valid; this HOOK is
+          // going to be deleted. We just need to collect any side-effects from the predicate
+          // expression.
+          trySimplifyUnusedResultInternal(tree.getOnlyChild(), sideEffectRoots);
+          return false; // This HOOK must be cleaned up.
+        } else {
+          sideEffectRoots.addLast(tree);
+          return hasFixedPointParent(tree);
+        }
+      }
+      case AND, OR, COALESCE -> {
+        // Try to remove the second operand from a AND, OR, and COALESCE operations. Remember that
+        // if the second
+        // child still exists, the result of the first expression is being used, and so cannot be
+        // removed.
+        //    x() ?? f --> x()
+        //    x() || f --> x()
+        //    x() && f --> x()
+        Node conditionalResultNode = trySimplifyUnusedResult(tree.getLastChild());
+        if (conditionalResultNode == null) {
+          // Don't bother adding a second child to make the AST valid; this op is going to be
+          // deleted. We just need to collect any side-effects from the predicate first child.
+          trySimplifyUnusedResultInternal(tree.getOnlyChild(), sideEffectRoots);
+          return false; // This op must be cleaned up.
+        } else {
+          sideEffectRoots.addLast(tree);
+          return hasFixedPointParent(tree);
+        }
+      }
+      case FUNCTION -> {
+        // Functions that aren't being invoked are dead. If they were invoked we'd see the CALL
+        // before arriving here. We don't want to look at any children since they'll never execute.
+        return false;
+      }
+      default -> {
+        // This is the meat of this function. It covers the general case of nodes which are unused
+        if (nodeTypeMayHaveSideEffects(tree)) {
+          sideEffectRoots.addLast(tree);
+          return hasFixedPointParent(tree);
+        } else if (!tree.hasChildren()) {
+          return false; // A node must have children or side-effects to be at fixed-point.
+        }
+
+        boolean atFixedPoint = hasFixedPointParent(tree);
+        for (Node child = tree.getFirstChild(); child != null; child = child.getNext()) {
+          atFixedPoint &= trySimplifyUnusedResultInternal(child, sideEffectRoots);
+        }
+        return atFixedPoint;
+      }
+    }
+  }
+
+  /**
+   * Returns an expression executing {@code expr} which is legal in any expression context.
+   *
+   * @param expr An attached expression
+   * @return A detached expression
+   */
+  protected static Node asDetachedExpression(Node expr) {
+    switch (expr.getToken()) {
+      case ITER_SPREAD, OBJECT_SPREAD -> {
+        switch (expr.getParent().getToken()) {
+          case ARRAYLIT:
+          case NEW:
+          case CALL: // `Math.sin(...c)`
+          case OPTCHAIN_CALL: // `Math?.sin(...c)`
+            expr = IR.arraylit(expr.detach()).srcref(expr);
+            break;
+          case OBJECTLIT:
+            expr = IR.objectlit(expr.detach()).srcref(expr);
+            break;
+          default:
+            throw new IllegalStateException(expr.toStringTree());
+        }
+      }
+      default -> {}
+    }
+
+    if (expr.hasParent()) {
+      expr.detach();
+    }
+
+    checkState(IR.mayBeExpression(expr), expr);
+    return expr;
+  }
+
+  /**
+   * Returns {@code true} iff {@code expr} is parented such that it is valid in a fixed-point
+   * representation of an unused expression tree.
+   *
+   * <p>A fixed-point representation is one in which no further nodes should be changed or removed
+   * when removing unused code. This method assumes that the expression tree in question is unused,
+   * so only side-effects are relevant.
+   */
+  private static boolean hasFixedPointParent(Node expr) {
+    // Most kinds of nodes shouldn't be branches in the fixed-point tree of an unused
+    // expression. Those listed below are the only valid kinds.
+    return switch (expr.getParent().getToken()) {
+      case AND, COMMA, HOOK, OR, COALESCE -> true;
+      case ARRAYLIT, OBJECTLIT ->
+          // Make a special allowance for SPREADs so they remain in a legal context. Parent types
+          // other than ARRAYLIT and OBJECTLIT are not fixed-point because they are the tersest
+          // legal
+          // parents and are known to be side-effect free.
+          expr.isSpread();
+      default ->
+          // Statments are always fixed-point parents. All other expressions are not.
+          NodeUtil.isStatement(expr.getParent());
+    };
+  }
+
   /**
    * Returns whether the output language is ECMAScript 5 or later. Workarounds for quirks in
    * browsers that do not support ES5 can be ignored when this is true.

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

@@ -28,6 +28,8 @@
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.google.javascript.jscomp.NodeUtil.ValueType;
 import com.google.javascript.jscomp.base.Tri;
+import com.google.javascript.jscomp.colors.Color;
+import com.google.javascript.jscomp.colors.ColorId;
 import com.google.javascript.jscomp.colors.StandardColors;
 import com.google.javascript.rhino.IR;
 import com.google.javascript.rhino.Node;
@@ -48,7 +50,6 @@ class PeepholeFoldConstants extends AbstractPeepholeOptimization {
       DiagnosticType.warning("JSC_FRACTIONAL_BITWISE_OPERAND", "Fractional bitwise operand: {0}");
 
   private static final double MAX_FOLD_NUMBER = Math.pow(2, 53);
-
   private final boolean late;
 
   private final boolean shouldUseTypes;
@@ -239,46 +240,77 @@ private void tryConvertToNumber(Node n) {
     reportChangeToEnclosingScope(replacement);
   }
 
-  /**
-   * Folds 'typeof(foo)' if foo is a literal, e.g. typeof("bar") --> "string" typeof(6) --> "number"
-   */
-  private Node tryFoldTypeof(Node originalTypeofNode) {
-    checkArgument(originalTypeofNode.isTypeOf());
-
-    Node argumentNode = originalTypeofNode.getFirstChild();
-    if (argumentNode == null || !NodeUtil.isLiteralValue(argumentNode, true)) {
-      return originalTypeofNode;
+  private @Nullable String tryEvalTypeof(Node typeofExpr) {
+    Node operand = typeofExpr.getOnlyChild();
+    switch (operand.getToken()) {
+      case FUNCTION, CLASS -> {
+        return "function";
+      }
+      default -> {}
     }
+    return switch (NodeUtil.getKnownValueType(operand)) {
+      case STRING -> "string";
+      case NUMBER -> "number";
+      case BIGINT -> "bigint";
+      case BOOLEAN -> "boolean";
+      case NULL, OBJECT -> "object";
+      case VOID -> "undefined";
+      default -> shouldUseTypes ? tryEvalTypeofFromColor(operand.getColor()) : null;
+    };
+  }
 
-    String typeNameString = null;
-
-    switch (argumentNode.getToken()) {
-      case FUNCTION -> typeNameString = "function";
-      case STRINGLIT -> typeNameString = "string";
-      case NUMBER -> typeNameString = "number";
-      case TRUE, FALSE -> typeNameString = "boolean";
-      case NULL, OBJECTLIT, ARRAYLIT -> typeNameString = "object";
-      case VOID -> typeNameString = "undefined";
-      case NAME -> {
-        // We assume here that programs don't change the value of the
-        // keyword undefined to something other than the value undefined.
-        if (argumentNode.getString().equals("undefined")) {
-          typeNameString = "undefined";
+  private static @Nullable String tryEvalTypeofFromColor(@Nullable Color color) {
+    if (color == null) {
+      return null;
+    }
+    if (color.isUnion()) {
+      String unionResult = null;
+      for (Color element : color.getUnionElements()) {
+        String elementResult = tryEvalTypeofFromColor(element);
+        if (elementResult == null) {
+          return null;
+        }
+        if (unionResult == null) {
+          unionResult = elementResult;
+        } else if (!unionResult.equals(elementResult)) {
+          return null;
         }
       }
-      default -> {}
+      return unionResult;
+    } else if (color.isConstructor()) {
+      return "function";
+    }
+    ColorId colorId = color.getId();
+    if (color.isPrimitive()) {
+      if (colorId.equals(StandardColors.STRING.getId())) {
+        return "string";
+      } else if (colorId.equals(StandardColors.NUMBER.getId())) {
+        return "number";
+      } else if (colorId.equals(StandardColors.BIGINT.getId())) {
+        return "bigint";
+      } else if (colorId.equals(StandardColors.BOOLEAN.getId())) {
+        return "boolean";
+      } else if (colorId.equals(StandardColors.SYMBOL.getId())) {
+        return "symbol";
+      } else {
+        return null;
+      }
     }
+    return (colorId.equals(StandardColors.TOP_OBJECT.getId())
+            || colorId.equals(StandardColors.UNKNOWN.getId()))
+        ? null
+        : "object";
+  }
 
-    if (typeNameString != null) {
-      Node newNode = IR.string(typeNameString);
-      reportChangeToEnclosingScope(originalTypeofNode);
-      originalTypeofNode.replaceWith(newNode);
-      markFunctionsDeleted(originalTypeofNode);
+  /** Folds 'typeof foo' when the runtime result of {@code foo} is known. */
+  private Node tryFoldTypeof(Node typeOfNode) {
+    checkArgument(typeOfNode.isTypeOf());
 
-      return newNode;
+    String typeName = tryEvalTypeof(typeOfNode);
+    if (typeName != null) {
+      typeOfNode = replaceExpressionWithEvalResult(typeOfNode, IR.string(typeName));
     }
-
-    return originalTypeofNode;
+    return typeOfNode;
   }
 
   private Node tryFoldUnaryOperator(Node n) {
@@ -1758,8 +1790,9 @@ private Node tryFoldSpread(Node spread) {
       parent.addChildrenAfter(child.removeChildren(), spread);
       spread.detach();
       reportChangeToEnclosingScope(parent);
+      return parent;
     }
-    return parent;
+    return spread;
   }
 
   /**