ReviewOS

zig-utils/zig-js

Parser: module early errors omit lexical super validation and exported expressions

#925
Closed glennmichael123 opened this 8 days ago ·
8 days ago

Parent #904; related #899, #923, #924.

Confirmed on ed325e1

Ten synthetic module fixtures run through the existing test262 runner: 6 invalid accepted / 4 valid passed. Independent Node/V8 SourceTextModule compilation rejects all six invalid modules and accepts all four controls. These are local regression fixtures, not six upstream test262 failures or flips.

The invalid Module bodies are:

super.x;
super();
export default super.x;
export const x = super.x;
export class C extends super.Object {}
const f = () => super.x;

Each line is a separate module. Controls cover a legal derived constructor, an object method's SuperProperty, an arrow returning a class with its own method, and ordinary-function new.target.

Root cause

Context.loadModule relies on Parser.parseModule to perform module early errors. checkModuleEarlyErrors checks declarations, scopes and private names but never applies the module's global lexical-super query. Script/eval Context entry points perform an additional scanEvalContext, but the module loader does not. Separately, that visitor lacks an export_decl arm, so simply invoking it would still skip exported initializers/default expressions/class heritage.

ECMA-262 ModuleBody early errors reject a ModuleItemList containing super. The Contains query must descend through exports while preserving function/method/class-body boundaries.

Fix and gate

  • Enforce the lexical-super restriction inside the module parser's own early-error phase, so every module consumer benefits.
  • Traverse export declarations/default expressions without crossing their nested function/method boundaries.
  • Add direct parser fixtures plus module entry/dependency rejection-before-side-effects checks in both execution modes.
  • Verify legal method/derived-constructor super remains valid; run the synthetic module fixtures and relevant pinned module/import/export/class corpus scopes serially with the 3072 MB guard.

Evidence: /private/tmp/zig-js-module-early-error-probe.8rugxT/ contains fixture sources, copied standard harness files, independent reference and run script/log. Runner SHA-256: efd808f2d2eebabc4f31b207114c73756bf05ff481268cc3c88a4338ab2342ee. No implementation or upstream conformance improvement claimed yet.

8 days ago

Landed in 3b3d85c, now on origin/main. The issue auto-closed from that commit's Resolve #925.

What shipped

Two additive production changes in src/parser.zig:

  • checkModuleEarlyErrors applies scanEvalContext(stmts, true, true), so ModuleBody's Contains-super restriction is enforced during parseModule itself rather than depending on the Script evaluation boundary's extra validation. Every module consumer benefits, including Context.loadModule.
  • scanSuperAndArgs gains an export_decl arm traversing the exported declaration and the default expression, so exported initializers, default expressions and class heritage are no longer skipped. Nested functions', methods' and constructors' own binding boundaries are preserved.

Rejection happens before execution, so no runtime guard or tier-specific patch is involved.

Verification

Figures below are read from the retained gate log, not restated from a summary. Its source copies of parser.zig and context.zig byte-match what landed, and the frozen before-runner still hashes to its recorded SHA-256.

gateresult
focused Debug units80 passed; 0 skipped; 0 failed; 0 leaked
Node/V8 module reference8 invalid + 6 valid parser fixtures agree
synthetic module fixtures6 fail, 4 pass → 0 fail, 10 pass

Seven pinned test262 scopes, identical before and after — 10,844 pass / 0 fail:

language/module-code                599
language/expressions/dynamic-import 941
language/expressions/import.meta     22
language/statements/class          4367
language/expressions/class         4059
language/eval-code                  347
built-ins/Function                  509

Corpus pin 4249661388e5d3f92a85186213da140a6481490f. Before runner efd808f2…2342ee, candidate aef6aabb…c62755. Real clean zig-gc/zig-regex checkouts, not the non-collecting stub. Serial 3072 MB bounded run; peak process-tree footprint 2764 MB, host memory low-water 28%.

Precision on the baseline: three scopes were measured against the before runner in this gate; the other four reused an exact frozen baseline from that same SHA-pinned binary, which the log records explicitly.

What this does NOT claim

The ten synthetic fixtures are local regression fixtures — not six upstream test262 flips. This is not a full-unit-suite, full-corpus, native-JIT or performance result, and it does not by itself establish Bun/Home compatibility.

Evidence retained at /private/tmp/zig-js-module-super-landed-evidence.PVRwsk/.

Sign in to comment on this issue.