ReviewOS

zig-utils/zig-js

Parser: after a for statement, a lexer error is dropped and the bad character skipped (regression in 69fde052)

#969
Closed glennmichael123 opened this 10 days ago ·
10 days ago

Regression in 69fde05 (#947, streamed lexical goals). I found it bisecting with a parser-only probe (git bisect run, 7 steps, f2bafeb good). Everything below was executed on Debug builds, with node v24.4.1 as the oracle.

After a for statement, a lexer error later in the source is dropped and lexing resumes past the offending character. Invalid programs parse and run:

programnodef2bafebmain (b5fb0ec)
var C = class { # x; };SyntaxErrorUnexpectedCharacterUnexpectedCharacter
for (;;) break; then var C = class { # x; };SyntaxErrorUnexpectedCharacterparses and runs
for (;;) break; then var q = 1; # x; qSyntaxErrorUnexpectedCharacterruns, then ReferenceError for x
for (var k in {}) ; then var q = 1; \u00; qSyntaxErrorUnexpectedCharacterOK 1
while (0); or an arrow, then # xSyntaxErrorUnexpectedCharacterUnexpectedCharacter (unaffected)

Parser.tokens for for (;;) break;\nvar C = class { # x; }; on main is for ( ; ; ) break ; var C = class { x ; } ; eof: the # is simply missing. That suggests the error is recorded, but a later re-lex resumes from the lexer's cursor, which already stepped past #. The for-only trigger points at classicForHeadAhead keeping its forked lexer on the success path. That is my reading, not verified.

test262: 64 negative tests that pass at f2bafeb fail on main, 32 each in test/language/expressions/class and test/language/statements/class. They are all under elements/syntax/early-errors: grammar-privatename-whitespace-error-*, private-*-cannot-escape-token, and grammar-field-identifier-invalid-zwnj-error. The trigger is the harness: assert.js defines compareArray with a for loop, so every test that follows it has a preceding for. The runner then sees $DONOTEVALUATE() run instead of a parse error.

Minimal pin (parser-only, so it needs no engine link):

try std.testing.expect(rejects("for (;;) break;\nvar C = class { # x; };"));
try std.testing.expect(rejects("for (var k in {}) ;\nvar q = 1; \\u00; q"));

@chrisbbreuer — leaving this one to you, since it sits inside the streaming work you are still landing.

10 days ago

One more instance, found while gating #970: test/language/literals/regexp/S7.8.5_A1.2_T1.js fails on main for the same reason. Its body is $DONOTEVALUATE(); followed by an unterminated /*/. With a for anywhere earlier, which the harness's compareArray supplies, the UnterminatedComment lexer error is dropped and the program runs:

programnodemain
function $DONOTEVALUATE(){ throw "ran"; } $DONOTEVALUATE(); /*/SyntaxErrorUnterminatedComment
the same, after function f(a){ for (var i = 0; i < a.length; i++) {} }SyntaxErrorruns (ERR ran)

So the drop covers comment errors too, not just invalid characters, and the class early-error files are 64 of at least 65 test262 failures it causes.

10 days ago

Completed in 4663ead1 (fix(parser): preserve lexer errors after for lookahead (#969)).

Parser.init can fill the unambiguous token run through a for head and stop on a later lexical error. classicForHeadAhead cleared that pending error while inspecting the cached head, then its fast success path retained the token frontier without restoring the error. It now restores the saved error and offset while keeping the forked lexer's equivalent post-error cursor and the one-pass cache.

Verification:

  • Minimal invalid-character and unterminated-comment probes now reject at the original lexer offsets.
  • Standalone parser module: 244/244 passed.
  • #939 nested classic-for adversarial runtime gate: passed.
  • Exact-parent test262:
    • test/language/expressions/class: 4,027 pass / 32 fail -> 4,059 / 0.
    • test/language/statements/class: 4,335 pass / 32 fail -> 4,367 / 0.
    • test/language/literals/regexp: 237 pass / 1 fail -> 238 / 0.
    • 65 gains, 0 regressions by failure-inventory diff.

Sign in to comment on this issue.