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

[mnsaglam]

Summary

This change adds type aware folding of typeof and Array.isArray(), leading to important code size reductions in certain situations (explained below).

Currently typeof x is folded only for literals such as typeof 2, typeof "hi", typeof function() {} etc. and Array.isArray() is not handled specially. With this change, both typeof and Array.isArray() can be folded from the declared or inferred type of a variable or expression. Example:

/** @const {!Array<number>} */
const a = [0, 1, 2];
const b = a;
console.log(Array.isArray(b), typeof a[0], typeof (a[0] + b[0]) + b[1]);
// compiles to console.log(!0, "number", "number1");

Such a transforms, combined with function inline + argument inline, can have cascaded effects.

Details

These folds are handled in PeepholeFoldConstants and PeepholeReplaceKnownMethods which take place in the PeepholeOptimizationsPass. By this stage, all JSTypes are reduced to colors so we rely on colors only to evaluate these expressions. In particular

@Nullable String tryEvalTypeof(Node typeofExpr)
Tri tryEvalArrayIsArray(Node arrayaIsArrayExpr) 

return the evaluated result from the colors, or if the value is not determined given the colors, return respectively null or Tri.UNKNOWN.

Then the method we've introducedAbstractPeepholeOptimization.replaceExpressionWithEvalResult(), will replace the expression with the evaluation result value while preserving the side-effects of the expression.

This is handled by rewriting expr to the comma node (expr, value) and then invoking the already present trySimplifyUnusedResult() method on the first child, that is expr. For instance an expression like

const f = () => (console.log("f"), 1);
const g = () => (console.log("g"), 2);

Array.isArray([prompt("enter bool") ? f() : g()]);

will be re-written to

(Array.isArray([prompt("enter bool") ? f() : g()]), true);

first and then in the same pass and peephole trySimplifyUnusedResult() will be invoked, simplifying it to

prompt("enter bool") ? f() : g(), true;

Outlook

The same approach can be extended to instanceof, though with more effort. These type based folds currently have a modest but noticeable contribution. What will unlock outsized gains is a recolor pass which can narrow the colors of a function body after the body is inlined to a call site (InlineFunctions) or the arguments are inlined into the function body (OptimizeParameters).

In such cases inlining can lead to much narrower typing than originally authored and all type aware folds (including the ones in this PR) can lead to very significant reductions.

Commit description

• This change adds better folding for Array.isArray() and the typeof operator.
• Some peephole helpers are moved to AbstractPeepholeOptimization and a new method replaceExpressionWithEvalResult() is introduced.
• Using these, it is possible to replace some type predicates and functions with their evaluation result.
• Currently, Array.isArray() and typeof is implemented however it should be possible to extend this to instanceof.
• Tests added for both folds, with a new integration test combining PureFunctionIdentifier and PeepholeOptimizationsPass'es.

[mnsaglam]

Hi @lauraharker @concavelenz, would appreciate your comments!

[mnsaglam]

I actually found a miscompile:

/**
 * @template T
 * @param {T|{ a: number }} x
 * @return {boolean}
 */
const f = (x) => Array.isArray(x) || typeof x == "object"

globalThis["f"] = f;

// compiles to
globalThis.f=a=>Array.isArray(a)||!0;

But it looks like the issue was already present in ChainableReverseAbstractInterpreter with template types and this fold exposed it. Investigating.

[concavelenz]

We generally feel that type based optimizations are not safe, as it is not uncommon for code authors to accidentally or deliberately lie about a type. We know that code is often inaccurate with respect to null or undefined.

Assuming "typeof x" where x is typed as "number" does return "number" when it is actually null or undefined.

It is also problematic for "Number" or "String" to leak into methods typed as accepting "number" or "string" respectively.

"typeof" and "instanceof" are also often used at a library or application boundary to validate that the declared types are correct.

We do today have unsafe type optimizations with regard to property access, and a few other places, but we aren't looking to add more.

What is more interesting is "provably correct" optimizations.

In your example:

/** @const {!Array<number>} */
const a = [0, 1, 2];
const b = a;
console.log(Array.isArray(b), typeof a[0], typeof (a[0] + b[0]) + b[1]);

It is possible to "prove" the concrete real type of "a[0]" is always "number" and the concrete real type of "b" is a native Array but that isn't peephole optimization (at least not today).

If the optimization opportunity were meaningful, I can imagine investing in some kind of simple "concrete" type analysis.

[mnsaglam]

I see, thanks! Could --use_types_for_optimization be a level (say 0,1,2) instead of a boolean in the future maybe?

What is more interesting is "provably correct" optimizations.

Even with the useColors flag turned off, the change uses NodeUtil.getKnownValueType() (plus direct check for literals FUNCTIONS,CLASS, which aren't covered by ValueType), which I believe is weaker than the concrete type analysis you suggested, but more general than direct literal checks.

If the optimization opportunity were meaningful, I can imagine investing in some kind of simple "concrete" type analysis.

I think there are important natural patterns that get optimized this way. I have to tone down my original claim about recolor pass: Already today when InlineFunctions or OptimizeParameters happens the inserted NAME nodes inherit the color of the call site and quite a lot of cascaded folds happen. Here is an important mode:

const arr = (a) => Array.isArray(a) ? a : [a];
/** @param {!Array<number>} x */
const double = (x) => {
  const a = arr(x);
  for (let i of a)
    console.log(2 * i);
}
globalThis["double"] = double; // becomes globalThis["double"]=a=>{for(let b of a)console.log(2*b)};

Here, generic utility function got inlined, and optimized as if it was monomorphized, since the insert NAME x node has the call site color. Of course, in the path you suggested the optimization can happen when a concrete value is supplied; not from the declared type like here.

Also, I believe the ChainableReverseAbstractInterpreter has an unsound assumption that template type extends object:

    @Override
    public JSType caseTemplateType(TemplateType templateType) {
      return caseObjectType(templateType);
    }

I think this can be changed to

    @Override
    public JSType caseTemplateType(TemplateType templateType) {
      return this.visit(templateType.getBound());
    }

so that for T that can assume primitive types, it would be visited as caseTopType(getNativeType(CHECKED_UNKNOWN_TYPE));.

It is also problematic for "Number" or "String" to leak into methods typed as accepting "number" or "string" respectively.

Within the type system this doesn't appear to be a problem:

const /** @type {string} */ s = new String("s"); // JSC_TYPE_MISMATCH

but of course, there may be cast etc.