ReviewOS

zig-utils/zig-js

Errors: render binding and parameter conflict SyntaxErrors

#953
Closed chris opened this 9 days ago ·
9 days ago

Parent: #899

Problem

Nineteen parser early-error sites across checkParamBodyConflict, checkCatchClause, validateStrictParams, and parseFunctionDecl still return bare UnexpectedToken. They cover four related binding invariants but lose both the conflicting name and the grammar reason before the JS boundary renders the exception.

Executed on current main (8ba110f), a 20-case eval differential renders UnexpectedToken at ... for every zig-js rejection. Raw JavaScriptCore 625.1.22 distinguishes the cases:

source familyJavaScriptCore message
parameter a + body let aCannot declare a let variable twice: 'a'.
parameter a + body const aCannot declare a const variable twice: 'a'.
parameter a + body class aCannot declare a class twice: 'a'.
duplicate catch bindingUnexpected identifier 'e'. Cannot declare a lexical variable twice: 'e'.
catch binding + body let / const / classdeclaration-kind-specific duplicate prose
catch binding + body functionCannot declare a function that shadows a let/const/class/function variable 'e'.
strict eval/arguments or duplicate parametersInvalid parameters or function name in strict mode.
anonymous FunctionDeclarationFunction statements must have a name.
keyword FunctionDeclaration nameCannot use the keyword 'if' as a function name.
non-simple parameters + own strict directive'use strict' directive not allowed inside a function with a non-simple parameter list.
strict function name eval / argumentsname-specific strict-mode prose

The same run exposed one oracle difference that must not change acceptance: JavaScriptCore accepts function f(interface) { "use strict"; } and a labeled function shadowing a catch binding, while Node rejects both and test262 defines the strict-reserved-word/catch conflicts as early errors. JavaScriptCore remains the prose oracle only where it implements the required rejection.

Implementation

  • Add structured reasons for parameter/body conflicts, catch binding conflicts, strict parameter lists, and FunctionDeclaration name/strictness failures.
  • Carry the already-parsed conflicting name and a declaration-kind discriminator to the renderer; do not rescan source text.
  • Preserve ParseError.UnexpectedToken and existing acceptance semantics.
  • Keep name lookup and formatting on failure paths; do not widen hot runtime values or add per-source side maps solely for diagnostics.
  • Convert all 19 bare returns in the four named functions.

Verification

  • Pin reason, message, and a stable grammar-site location for each shape in parser tests.
  • Add a production frontend boundary case for eval-visible .message values.
  • Compare supported messages with raw JavaScriptCore and rejected grammar with Node/test262 where the oracles differ.
  • Compare affected test262 function/catch subtrees against an exact-parent binary and report denominators plus gains/regressions.
9 days ago

Implemented and pushed to main in c124cca.

Evidence:

  • parser module: 231/231 passed
  • focused production frontend boundary: 1/1 passed
  • raw JavaScriptCore 625.1.22 differential: 18/18 exact messages for supported rejection cases
  • Node grammar oracle retained both standards-required rejections that this JSC build accepts
  • eight exact-parent test262 function/async/arrow/object/catch subtree pairs were byte-identical: 2,655 passes / 1 known failure / 2,656 denominator; zero gains and zero regressions

All 19 targeted bare returns now carry structured reasons and constant-time retained-name offsets. Diagnostic formatting and statement-location fallback lookup remain failure-only.

Sign in to comment on this issue.