ReviewOS

zig-utils/zig-js

Parser: retain offending identifier in malformed new.target diagnostics

#918
Closed glennmichael123 opened this 9 days ago ·
9 days ago

Follow-up to #915; parent diagnostic tracking #899. #917 is independent and already landed.

The existing 22-case real-JSC differential retains three malformed-meta-property mismatches: new.other; new.\u0074arget; and the same escaped spelling inside a function. Both zig-js tiers throw the right SyntaxError class, but report eval: UnexpectedToken at the following token (1:10, 1:16, or 1:29 in the saved cases). JSC identifies the offending raw identifier and says that new. can only be followed with target.

Root cause: parseNew advances past the identifier before rejecting its spelling, returns the compact error without a DiagnosticReason, and loses the owned token location. The existing failWithTokenReason helper already preserves raw spelling, identifier/keyword classification, and source position; template diagnostic forwarding already preserves this metadata across scratch-token cleanup.

Required fix: record a dedicated malformed-meta-property reason at the offending identifier before advancing, reuse existing token formatting, and preserve the compact error category and all acceptance rules. Verify raw escapes and UTF-8 spelling, source positions through nested/CRLF templates, early-error non-execution, both script/module parse goals and tree-walker/VM JS boundaries. Quantify the existing three differential gains and focused test262 before/after. Do not imitate the unrelated JSC static-block PrivateSymbol.newTargetLocal bug or claim universal diagnostic parity.

9 days ago

Fixed on main by https://github.com/zig-utils/zig-js/commit/03b21c3f3850dadd462fea47854d3ddf71e12bf5.

The commit message records the focused verification scope, before/after diagnostic comparisons, and targeted test262 results. This does not claim a full-suite run or universal diagnostic parity.

Follow-up to #915; related #899 and #917.

Sign in to comment on this issue.