ReviewOS

zig-utils/zig-js

Errors: class early errors and private-name diagnostics diverge from JavaScriptCore

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

Parent: #459 Related: #899

Problem

A 66-case differential over class, private-name, super, TDZ, Promise, async and iterator errors matches 34/66. Two clusters account for most of it.

Class early errors have no prose at all (8 cases)

Duplicate private name, undeclared private name, a private name outside a class, delete this.#x, await in a static block, a field named constructor, #constructor, and super outside a method all collapse to the SAME string: eval: UnexpectedToken at <line>:<col>.

Two structural problems: there is no per-rule message, so eight distinct spec early errors are indistinguishable; and the string carries an eval: prefix and a line:col that JavaScriptCore never puts in .message. This is the class-specific face of #899 — fixing #899's reason-discriminant mechanism should cover these, and they are a good first set of rules to carry through it since the parser already detects each one correctly at the right offset.

Private-name runtime messages need a split AND a merge (6 cases)

JavaScriptCore splits by what the private name IS, where zig-js uses one string:

brand missing onJavaScriptCore
a fieldCannot access invalid private field
a method or accessorCannot access private method or acessor

Note JSC's own typo — one c in "acessor". Exact parity requires reproducing it.

In the other direction zig-js is more specific than JSC, and matching means merging. For a present brand with a missing accessor half, JSC uses two fixed strings — Trying to access an undefined private getter / ... private setter — and uses the setter string even for writing a private method. zig-js emits three more descriptive strings (Cannot write private method, Cannot write private member: accessor has no setter, Cannot read private member: accessor has no getter).

That merge loses information, so it is a deliberate call rather than an obvious fix: worth confirming that Home/Bun code actually matches on these before giving up the more useful wording.

Verification

scratchpad/sweep/probe.sh, against a real JavaScriptCore backend.

9 days ago

Landed the non-lossy runtime half in b6cff0b1 (fix(class): distinguish private brand failure kinds).

The runtime PrivateEnvironment now carries each private binding's declaration kind together with its evaluation-unique storage key. Missing-brand reads/writes therefore preserve JSC's distinction:

  • field: Cannot access invalid private field
  • method/accessor: Cannot access private method or acessor

This is lexical metadata, so it remains correct through nested classes, closures, direct eval, generators, and forced VM execution; it does not guess from the unbranded receiver. Added eight focused tree-walker + VM cases and retained map collision/OOM coverage.

Verification:

  • focused diagnostic unit: 1 passed, 0 failed, 0 leaked
  • PrivateNameMap unit: 1 passed, 0 failed, 0 leaked
  • test/language/statements/class: 4,367 passed, 0 failed

The information-losing merge for present-brand missing getter/setter/method writes remains intentionally open, as do the parser early-error prose items shared with #899.

9 days ago

Landed f14bb72b (fix(class): retain private access diagnostic source).

Private brand errors now retain the owning member/assignment source through reads, writes, updates, compound assignments, calls, optional calls, tagged templates, nested classes, and direct eval. The bytecode and native property paths carry the same EvaluationSite and route private names through their brand/descriptor checks before ordinary inline caches.

Evidence:

  • Existing 66-case JSC diagnostic differential: 34/66 -> 37/66 exact matches.
  • test/language/statements/class: 4,367 -> 4,367 passed, 0 failures; flips 0 test262 cases.
  • Expanded focused regression: 14 cases in tree-walker mode and required bytecode with JIT disabled/enabled; 1 test passed, 0 failed, 0 leaked.
  • A separate GC-sensitive private-eval integration filter was terminated after the local non-collecting zig-gc stub emitted excessive leak diagnostics. It is not counted as passing evidence.

The parser-reason work in #899 and the remaining class diagnostics here stay open. Next slice: explicit class early-error reasons and class-heritage diagnostics, preserving error categories and evaluation order.

9 days ago

Landed b34796fe (fix(parser): preserve class diagnostic reasons and locations). Related: #899, #904.

The parser now records 17 explicit reasons for invalid class declarations. These distinguish field/method/accessor collisions, static versus instance accessor conflicts, forbidden constructor/prototype declarations, and duplicate constructors. Declaration offsets are temporary parser scratch, and the grammar's existing acceptance rules remain intact.

eval and Function render these reasons as JSC-compatible prose. Location stays in non-enumerable line, column, and byteOffset properties. Errors inside nested tagged/untagged template substitutions retain the reason and translate normalized CRLF offsets back into the original source. Shared class-heritage validation also uses JSC's two distinct messages without changing evaluation order.

Measured evidence:

  • New declaration/heritage differential: 79/79 exact JSC matches, including every pair of field/method/getter/setter declarations across static and instance placement.
  • Existing broad diagnostic differential: 37/66 -> 43/66 exact matches in this commit (34/66 -> 43/66 across the two latest commits).
  • Class test262 subtree: 4,367 -> 4,367 pass, zero failures; flips 0 tested cases.
  • 55/55 parser tests, plus the two focused diagnostic tests: zero failures and zero leaks. Focused checks cover eval/Function messages, declaration offsets, legal accessor pairs, and nested CRLF template provenance.

Both issues remain open. Undeclared private-name lists, private delete/static-await/super diagnostics, other parser/lexer reasons, and the deliberate present-brand accessor-message merge remain separate work. This is not a claim that all SyntaxErrors now match JSC.

9 days ago

Production-dependency verification update: an isolated clean checkout of bff189a, real zig-gc 0285ef8 and zig-regex c60344d now passes both GC-sensitive integration filters: private eval contexts survive function escape and suspension (24 scenarios across tree-walker/required VM), and private eval contexts retain suspended receivers across moving collection. Each filter reports 1 passed, 0 failed, 0 leaked. The original local GC shim was not modified. This replaces the prior unverified status for these two filters only, not the full suite. No timing/performance claim: another Home benchmark overlapped because the job used a different lock path; that scheduling mismatch has been identified and future jobs will use the canonical shared lock. Memory guard stayed enabled throughout (3500 MB peak conservative tree footprint, host low-water 33%).

9 days ago

Private-deletion slice landed as #919 in https://github.com/zig-utils/zig-js/commit/6256244884be309974ad72df75e7fabe5846fb08. It fixes eight optional-private early-error violations and the specific direct-private diagnostic without rejecting valid outer public properties/call results. Focused units and six affected corpus subtrees passed; full verification scope is recorded in the commit and #919. This parent stays open for the remaining parser diagnostics and previously discussed accessor-message decisions.

9 days ago

Landed the semantic early-error slices #920 and #921 in https://github.com/zig-utils/zig-js/commit/41104a44ee2a7bfd7939a617bfd45fcc0a369d3f. The shared scanner now visits expressions inside destructuring patterns and logical assignments/dynamic imports, and rejects await iteration/asynchronous disposal in static blocks while preserving valid function boundaries. Combined evidence: 25 distinct early-error gains in both execution modes; 72 focused units passed without failures or leaks; 7,462 tested corpus cases retained passes with zero before/after failures. Exact filters and scope limits are in the commit. This parent remains open for remaining diagnostic wording/location work and the previously deferred accessor-message decisions.

9 days ago

Follow-up status: #922 is fixed and pushed in 1adf483 (74 focused unit tests; 9368 targeted test262 passes before/after, zero failures; seven focused early-error gains in both modes). The next independently reproduced omission is #923: class heritage and computed names bypass outer lexical/suspension queries. Its compile-only differential is 6/16 for both zig-js modes versus 16/16 for Node/V8, with ten invalid programs accepted. A separate root-cause patch and class-body boundary controls are now under bounded verification. This parent and #899 remain open; these semantic fixes do not claim complete diagnostic-message parity.

9 days ago

#923 is now fixed and pushed in 877f25b: class heritage/computed-name queries reject ten previously accepted invalid programs before execution in both tree and required-VM modes. Verification: 76 focused Debug units, 10,224 test262 cases before/after with zero failures/regressions; complete scope and provenance in #923. Broader diagnostic prose work in #899/#904 remains open; no full-corpus or performance claim.

9 days ago

#924 landed in ed325e1: seven invalid acceptances hidden in labels/logical assignments/with now reject before execution. Both execution-mode probes improve 5/12 to 12/12; 78 focused units and 9,972 targeted test262 cases pass, with scope/provenance in #924. This closes another structural early-error defect; broader #899/#904 diagnostic work remains open.

9 days ago

#925 landed in 3b3d85c: ModuleBody's Contains-super restriction is now enforced inside parseModule itself, and the early-error visitor descends through exported declarations and default expressions instead of skipping them. Six previously accepted invalid modules now reject before entry or dependency execution, in both tree and required-VM modes, with four legal controls unchanged. 80 focused units clean; seven pinned test262 scopes hold at 10,844 pass / 0 fail before and after; scope and provenance in #925.

That completes the structural arc this parent spawned — #922 (arrow lexical queries), #923 (class heritage and computed names), #924 (labels, logical assignments, with) and now #925 (module bodies and exports) are all closed. Each was the same shape of defect: an early-error query that terminated before a syntactic position the spec's Contains operation reaches, so an invalid program was accepted and ran.

What remains open here is a different kind of work. #904 was filed for diagnostic wording, and that half is untouched:

  • the class early errors still collapse to one positional template rather than per-rule prose — that is the class-specific face of #899, and it needs #899's reason discriminant before the wording can differ per rule;
  • the private-name runtime messages still need a split in one direction (JavaScriptCore distinguishes a missing field brand from a missing method/accessor brand) and a merge in the other (JSC uses two fixed accessor strings where zig-js emits three more descriptive ones).

The merge direction loses information, so it is a judgement call rather than an obvious fix, and I have deliberately left it rather than quietly trading accuracy for parity. It would be worth confirming Home/Bun actually match on those strings before giving up the more useful wording.

No full-corpus, JIT or performance claim attaches to any of the above.

9 days ago

Landed the undeclared-private-name diagnostic slice in 393f84d0 (fix(parser): retain undeclared private diagnostics).

The parser now keeps private-in identifiers distinct from ordinary IdentifierReferences and retains the source offset of private member names. eval and Function therefore match JavaScriptCore's Cannot reference undeclared private names: "#name" wording for member access, private in, outside-class access, and escaped private names while reporting the exact offending token. This uses AST metadata rather than rescanning source or allocating a side index; Node and ClassMember remain 80 and 56 bytes.

Verification:

  • focused parser diagnostic: 1 passed
  • eval/Function diagnostic integration: 1 passed, 0 failed/leaked (four new cases through both boundaries)
  • private-brand runtime filters: 4 passed, 0 failed/leaked
  • exact-parent test262: expressions/in 36 → 36; expression class elements 1,428 → 1,428; statement class elements 1,534 → 1,534; zero failures, flips 0

#904 remains open for the independent static-block await and invalid-super prose/location slices, plus the previously deferred information-losing accessor-message decision.

9 days ago

Landed the invalid-super diagnostic slice in 794dfecc (fix(parser): preserve invalid super diagnostics).

super() and SuperProperty nodes now retain their exact source offsets through the existing context-sensitive Contains scans. Global/function/method/static-block violations report super is not valid in this context., while a field initializer reports JSC's distinct Unexpected token '('. super call is not valid in class field initializer context. at the call token. Direct-eval scan failures now preserve structured parser reasons and locations instead of replacing them with one legacy fallback string; unclassified rules retain the fallback. The AST Node remains 80 bytes.

Verification:

  • eval/Function diagnostic integration: 1 passed, 0 failed/leaked; four new cases through both boundaries
  • all focused super filters: 18 passed, 0 failed/leaked
  • exact-parent test262: expressions/super 94 → 94; statements/class/super 8 → 8; expression class-element early errors 222 → 222; statement class-element early errors 222 → 222; zero failures, flips 0

The remaining lossless parser item in #904 is static-block await wording/location. The information-losing present-brand accessor-message merge remains the separate product decision already noted here.

9 days ago

Landed the static-block await diagnostic slice in 5eb83f14 (fix(parser): preserve static block await diagnostics).

Await expressions, asynchronous disposal declarations, and for-await heads now retain their exact keyword offset through the class static-block ContainsAwait query. The scanner distinguishes JSC's three position-dependent messages:

  • direct StatementList await / await using: Unexpected identifier 'await'. Cannot use 'await' within static block.
  • nested/parenthesized IdentifierReference position (including for (await using …)): The 'await' keyword is disallowed in the IdentifierReference position within static block.
  • for await: for-await-of can only be used in an async function or async generator.

Nested async functions/arrows remain their own valid await boundary. The metadata fits in the existing AST layout; Node remains 80 bytes.

Verification:

  • focused eight-position parser differential: 1 passed
  • eval/Function diagnostic integration: 1 passed, 0 failed/leaked; three new cases through both boundaries
  • all focused static-block filters: 5 passed, 0 failed/leaked
  • exact-parent test262: expression classes 4,059 → 4,059; statement classes 4,367 → 4,367; zero failures, flips 0

This completes the lossless parser-diagnostic items tracked here (class declaration rules, private deletion/undeclared names, static-block await, and invalid super). The only remaining #904 item is the previously deferred information-losing present-brand accessor-message merge, which needs the product decision already called out in this thread.

9 days ago

The final present-brand diagnostic slice landed on main in e8bb5a6.\n\nA fresh direct JavaScriptCore C-API probe against macOS JavaScriptCore 625.1.22 confirmed the current oracle strings without passing through Home’s known private-method/accessor transpiler blind spot:\n- getter missing: Trying to access an undefined private getter\n- setter missing: Trying to access an undefined private setter\n- private-method write: Trying to access an undefined private setter\n\nThe implementation now matches those strings. A tracked-source audit found no exact-string dependency in Home; exact GitHub code search found no Bun matches for the three prior zig-js strings before the API search limit was reached.\n\nValidation:\n- new present-brand diagnostic filter: tree walker plus required VM with JIT disabled/enabled, 2/2 linked tests passed\n- existing missing-brand field/method/accessor filter: 2/2 linked tests passed\n- direct zig-js runtime probe: 3/3 exact messages\n- exact-parent test262 private methods/accessors: 50/50 before -> 50/50 after, zero failures and zero flips\n\nThis completes the remaining item identified in the issue thread; GitHub closed #904 from the commit.

Sign in to comment on this issue.