ReviewOS

zig-utils/zig-js

Parser: preserve reason and source position for invalid new.target

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

Parent: #899. Related: #912, #496.

Verified remaining gap

The saved parent/candidate gate for #913 retained 12/14 exact diagnostic matches. The two remaining cases are global template substitutions containing new.target:

  • `x${new.target}`
  • String.raw`x${new.target}`

System JavaScriptCore reports SyntaxError: new.target is only valid inside functions or static blocks.. zig-js reports the internal eval: UnexpectedToken at ... form. Both reject the invalid programs; this is a diagnostic/provenance gap, not evidence of an acceptance failure.

Root cause

Parser.parseNew already records new_start_token, validates the contextual target identifier, and checks new_target_depth. The depth-zero branch still returns bare ParseError.UnexpectedToken, without a DiagnosticReason or offending offset. The existing failWithReasonAt path can retain the reason and original new position, including through the template-subparser offset translation. Direct eval already seeds new_target_depth from its actual enclosing context, and #912 preserves that context inside substitutions.

Required fix and gates

  • Record a dedicated diagnostic reason only at the existing depth-zero rejection; preserve the ParseError category and all syntactic-context/admission rules.
  • Keep invalid/escaped new.<identifier> diagnostics separate; do not infer the reason by scanning source text at the JS boundary.
  • Verify direct and indirect eval, top-level arrows, valid enclosing functions/methods/static blocks, and nested tagged/untagged CRLF substitutions. Cover exact message, original byte/line/column metadata, and prevention of preceding side effects for early errors.
  • Exercise both tree-walker and required-bytecode callers and relevant exact-parent test262 groups. Do not claim behavioral flips or full diagnostic completeness without measuring them.

This is a next frontend batch, not a claim that the implementation or validation has already landed. #914's provenance tooling and #913's performance publication remain in progress.

9 days ago

Prepared a scoped implementation in an isolated checkout based on 650efda, leaving the active #914 validation checkout unchanged. The existing depth-zero NewTarget rejection now records its dedicated reason at the original new token; target spelling checks and scope admission remain unchanged. Added parser fixtures for script/module rejection positions, nested tagged/untagged CRLF substitutions, malformed/escaped contextual identifiers, and valid function/method/static-block controls. Added tree-walker/required-bytecode integration fixtures for direct/indirect/nested eval, early-error side-effect prevention, exact wording and non-enumerable source-location metadata. Zig formatting and diff checks pass. No runtime pass or conformance flip is claimed yet: builds/tests will run serially after the current full benchmark-validator inventory releases the shared memory guard.

9 days ago

Verification of the initial diagnostic patch: production frontend smoke 26/26; exact-parent test262 comparison 5155/5155 passing in both builds across new.target, template-literal, tagged-template, arrow-function, eval-code, and statements/class. No failures or denominator changes; peak guarded footprint 2059 MB. Combined inline-unit compilation exceeded the unchanged 3072 MB cap at 3099 MB and remains incomplete. The independent 22-case actual-JSC oracle then correctly failed: global arrow bodies require a distinct message (now patched, not yet rebuilt), and direct eval in a static block exposes an existing runtime new.target-context defect. A JSC static-block tagged-template case itself throws PrivateSymbol.newTargetLocal; specification semantics must take precedence over copying that oracle behavior. Logs: /private/tmp/zig-js-new-target-light.THuNjF and /private/tmp/zig-js-new-target-oracle.ZOJlB1. No landing or full diagnostic-parity claim yet.

9 days ago

Split the independently reproduced runtime defect into #916. The isolated batch now contains the global-arrow diagnostic correction plus a GC-rooted static-block new.target binding save/restore and direct-eval permission fix. The VM delegates class execution to this same evaluator. A fresh guarded serial verification is running; earlier 5155/0 corpus evidence applies only to the initial patch. The oracle explicitly reports the JSC static-block internal-symbol discrepancy and requires the specification-correct result, rather than silently accepting the reference-engine bug.

9 days ago

The current #915/#916 batch now also passes the actual production inline tests selected with native Zig compile-time filters: 71 passed, 0 skipped, 0 failed, 0 leaked (includes all parser tests and all five required context/parser regression names). Peak guarded footprint 2334 MB. The temporary two-line filter was removed afterward and build.zig is identical to HEAD; no filtering change will land. The alternate-build-file attempt had produced the default library and was rejected as test evidence; only /private/tmp/zig-js-new-target-existing-target-units.log is the accepted focused-unit run. Additional direct tree/VM probes passed 24 early-error/source-position assertions per mode. This supplements the latest 5155/0 before-and-after corpus and 13 JSC diagnostic gains; it is NOT a full combined unit-suite pass. Changes remain isolated and uncommitted pending the requested authorship-history scope clarification.

9 days ago

Landed directly on main as 92043c5 under glennmichael123, following #916 (06652de). Exact source matches the tested isolated batch; temporary compile-time test filtering is absent from the landed tree. September 14 recheck: 24 early-error/source-position assertions per tier, 7 static-block assertions per tier, and the 22-case oracle retained all 13 batch gains (18 exact matches, with the remaining differences explicit). Earlier exact-parent corpus and 71-test focused-unit evidence remain valid for these unchanged source bytes. No full combined-suite or universal JSC-message-parity claim.

Sign in to comment on this issue.