Add abstract serializer mode for test262 execution

[calebmer]

I extended the --serializer command line argument I added in #2290 to now support --serializer abstract-scalar. What this mode does is it converts all boolean, string, number, and symbol literals into abstract values. I did not choose to extend this logic to object and array literals just yet since scalars alone showed some interesting results.

What I really want here is a review of the results.

Full suite execution results are real bad. 18% pass rate. I dug a bit into why.

=== RESULTS ===
Passes: 3356 / 17780 (18%)
ES5 passes: 2276 / 12045 (18%)
ES6 passes: 1080 / 5735 (18%)
Skipped: 13375
Timeouts: 28

I was mostly interested in the runtime failures we see since that means Prepack is serializing invalid code. However, I found ~14k failures in the Prepack stage (more on this in a bit) and ~3k failures in the runtime stage. This means ~80% of tests fail to compile with this abstract transformation applied.

Why are these tests failing? I took the first 4 items of the stack traces from errors thrown in the Prepack stage, sorted, and ranked them. Here’s the result. The top 5 with thousands of hits are:

7538 of:
    at AbstractValue.throwIfNotConcrete (/Users/calebmer/prepack/src/values/AbstractValue.js:536:11)
    at ToImplementation.ToStringPartial (/Users/calebmer/prepack/src/methods/to.js:717:69)
    at NativeFunctionValue._index.NativeFunctionValue [as callback] (/Users/calebmer/prepack/src/intrinsics/ecma262/String.js:34:37)
    at NativeFunctionValue.callCallback (/Users/calebmer/prepack/src/values/NativeFunctionValue.js:121:12)


4595 of:
    at AbstractValue.throwIfNotConcrete (/Users/calebmer/prepack/src/values/AbstractValue.js:536:11)
    at NativeFunctionValue.func.defineNativeMethod [as callback] (/Users/calebmer/prepack/src/intrinsics/ecma262/Object.js:328:41)
    at NativeFunctionValue.callCallback (/Users/calebmer/prepack/src/values/NativeFunctionValue.js:121:12)
    at functionCall (/Users/calebmer/prepack/src/methods/call.js:308:26)


1454 of:
    at AbstractValue.throwIfNotConcrete (/Users/calebmer/prepack/src/values/AbstractValue.js:536:11)
    at NativeFunctionValue.func.defineNativeMethod [as callback] (/Users/calebmer/prepack/src/intrinsics/ecma262/Object.js:364:41)
    at NativeFunctionValue.callCallback (/Users/calebmer/prepack/src/values/NativeFunctionValue.js:121:12)
    at functionCall (/Users/calebmer/prepack/src/methods/call.js:308:26)


1351 of:
    at invariant (/Users/calebmer/prepack/src/invariant.js:18:15)
    at EvalPropertyNamePartial (/Users/calebmer/prepack/src/evaluators/ObjectExpression.js:59:7)
    at _default (/Users/calebmer/prepack/src/evaluators/ObjectExpression.js:80:21)
    at LexicalEnvironment.evaluateAbstract (/Users/calebmer/prepack/src/environment.js:1368:20)


1053 of:
    at AbstractValue.throwIfNotConcrete (/Users/calebmer/prepack/src/values/AbstractValue.js:536:11)
    at NativeFunctionValue.obj.defineNativeMethod [as callback] (/Users/calebmer/prepack/src/intrinsics/ecma262/ObjectPrototype.js:35:39)
    at NativeFunctionValue.callCallback (/Users/calebmer/prepack/src/values/NativeFunctionValue.js:121:12)
    at functionCall (/Users/calebmer/prepack/src/methods/call.js:308:26)

This means there may be some low hanging fruit.

Questions

Here are my questions for you.

• Did you expect results like this?
• What is our ideal test262 pass rate with this transformation applied?
• What happens to React Compiler or other projects when these errors are thrown? (As I understand it, we bail out and don’t optimize the code, but do optimize the code around it.)
• Do you think my methodology is flawed?

It’s also possible that something in my methodology is wrong, but I didn’t spend much time investigating these failures as I spent investigating the failures I found in #2290.

My goal with this test suite is to build an understanding of what “correctness” for the React Compiler against all JavaScript code looks like. (Not just the few bundles we’ve selected to look at.) I don’t think these results suggest that we only safely compile 18% of the language, but it’s a data point. I’ll be looking into fixing a selection of these issues to better understand their nature or if I need to change methodologies.

[trueadm]

I think this is to be somewhat expected, minus the invariant you're seeing. We need to fix that.

You're hitting to many AbstractValue.throwIfNotConcrete errors because this transform is making abstract values, but abstract values are unsafe for many of the operations so it ends up in FatalErrors. The reason we can compile so much as of late is because we apply a "pure scope" to optimized functions and React components. Doing so gets us around the majority of AbstractValue.throwIfNotConcrete errors from occurring. You can apply the pure scope manually by wrapping the tests in __evaluatePureFunction(funcContainingTestCode).

I'd be interested to also know what test failures from the 104 tests are because of bad output – which is the worst possible case IMO. Are those tests failing currently on master without the transform too? If they are not, we should focus our efforts on those first, as those will be blockers soon enough.

[sebmarkbage]

All built-in methods basically have to be made compatible, one-by-one since they're implemented in "native Prepack". All the ones (except the invariant) you posted are that.

The pure scope mode is good for any built-in method because it basically allow it to fallback to leaving the call in place (see CallExpression). You could enable only that part of the pure scope mechanism.

However, it would be interesting to see what errors you get if you enable only the call expression fallback for NativeFunctionValue. That will find gaps outside built-in methods.

[calebmer]

I played around a bit trying to fix some of the high firing Prepack errors to get better coverage. I got to a 31% pass rate just by fixing String(x) where x is an object value.

=== RESULTS ===
Passes: 5602 / 17780 (31%)
ES5 passes: 3241 / 12045 (26%)
ES6 passes: 2361 / 5735 (41%)
Skipped: 13375
Timeouts: 28

New output. Now with error messages!

Other things I tried:

• Not using an abstract string in Object.defineProperty since that’s the second error which fires a lot. However, the static Babel based approach I was using only worked in 1k of cases. I’d rather just add support for abstract properties in Object.defineProperty. (That’s non-trivial though whereas String(x) was.)
• Taking @sebmarkbage’s advice to leave CallExpressions but havoc their arguments. I did this, but it only moved the pass rate by 4%. We still got throwIfNotConcrete errors from evaluator code and we run into a bunch of errors since that particular feature was designed for pure mode, but we run outside of pure mode (my commit for the curious). Decided that it would be more valuable to fix high firing fatal errors.

I’d like to get this merged. It’s not perfect, but I plan to keep iterating. I’m going to move on from trying to understand where failings are in the suite and try to fix as many issues as possible with this rough prioritization scheme:

1. Prepack build passes, but runtime has an unexpected result. (React relevant stuff.)
2. Prepack invariants and other unexpected JS errors.
3. Fatal errors.

I’m worried that invariants and fatal errors are hiding errors in the 1 category, so I might bounce around between these categories to uncover more issues in 1.

[trueadm]

Please can you pull out your String(x) fix into a separate PR if possible? I'll take another review of this PR again on Monday morning :)

[NTillmann]

LGTM

[facebook-github-bot]

calebmer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.