fix(ast/estree): set value for BigIntLiterals and RegExpLiterals on JS side#9044
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
4174797 to
f2914a5
Compare
|
@Boshen Can I just check that you don't see any downside to this? I guess we also need to do the same for WASM, though I'd like to leave that until later - not sure if it's going to be tricky or not. |
Before this PRAverages: After this PRAverages: |
165d78b to
ef553b9
Compare
f2914a5 to
9cc1080
Compare
9cc1080 to
7a0d537
Compare
Merge activity
|
…s on JS side (#9044) Add a "reviver" function to `JSON.parse` in NAPI parser module, to correctly set `value` field of `Literal`s for `RegExp`s and `BigInt`s. Surprisingly, judging by a local run of NAPI parser benchmarks (added in #9045), this does not seem to hurt performance. In fact the benchmarks were showing a ~2% speed-up. That's clearly nonsense - just noise - but it does suggest at least that this isn't hurting performance significantly.
7a0d537 to
41dba62
Compare

Add a "reviver" function to
JSON.parsein NAPI parser module, to correctly setvaluefield ofLiterals forRegExps andBigInts.Surprisingly, judging by a local run of NAPI parser benchmarks (added in #9045), this does not seem to hurt performance. In fact the benchmarks were showing a ~2% speed-up. That's clearly nonsense - just noise - but it does suggest at least that this isn't hurting performance significantly.