Fix for-of continue raising ReferenceError instead of skipping iteration#425
Merged
Fix for-of continue raising ReferenceError instead of skipping iteration#425
continue raising ReferenceError instead of skipping iteration#425Conversation
…ation (#401) `continue` was completely unimplemented — the lexer treated it as an identifier, so `continue;` inside a for-of loop produced "ReferenceError: Undefined variable: continue". Add the full pipeline: keyword constant, token type, lexer registration, control-flow kind (cfkContinue, tag value 3 in the 2-bit scheme), AST node, parser rule, evaluator propagation (including through switch), bytecode compiler (GContinueJumps with pending-finally support for all three loop variants), and an illegal-continue guard in function bodies. Also register gttContinue in all five keyword-as-property-name lists so `{ continue: "v" }` keeps working. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 Walkthrough🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Contributor
Benchmark Results386 benchmarks Interpreted: 🟢 8 improved · 🔴 233 regressed · 145 unchanged · avg -3.6% arraybuffer.js — Interp: 🔴 5, 9 unch. · avg -3.3% · Bytecode: 🔴 12, 2 unch. · avg -10.8%
arrays.js — Interp: 🔴 8, 11 unch. · avg -2.2% · Bytecode: 🔴 19 · avg -11.0%
async-await.js — Interp: 🔴 5, 1 unch. · avg -8.2% · Bytecode: 🔴 5, 1 unch. · avg -12.7%
base64.js — Interp: 🔴 10 · avg -5.2% · Bytecode: 🔴 10 · avg -11.2%
classes.js — Interp: 🔴 18, 13 unch. · avg -2.8% · Bytecode: 🔴 30, 1 unch. · avg -19.6%
closures.js — Interp: 🔴 11 · avg -4.8% · Bytecode: 🔴 11 · avg -15.0%
collections.js — Interp: 🔴 3, 9 unch. · avg -1.3% · Bytecode: 🔴 10, 2 unch. · avg -6.6%
csv.js — Interp: 🔴 7, 6 unch. · avg -3.1% · Bytecode: 🔴 13 · avg -13.6%
destructuring.js — Interp: 🔴 11, 11 unch. · avg -2.9% · Bytecode: 🔴 22 · avg -15.4%
fibonacci.js — Interp: 🔴 5, 3 unch. · avg -3.2% · Bytecode: 🔴 8 · avg -11.7%
float16array.js — Interp: 🟢 1, 🔴 16, 15 unch. · avg -2.7% · Bytecode: 🔴 29, 3 unch. · avg -9.5%
for-of.js — Interp: 🔴 5, 2 unch. · avg -3.4% · Bytecode: 🔴 7 · avg -13.7%
helpers/bench-module.js — Interp: 0 · Bytecode: 0
iterators.js — Interp: 🟢 1, 🔴 31, 10 unch. · avg -4.3% · Bytecode: 🔴 42 · avg -15.0%
json.js — Interp: 🔴 10, 10 unch. · avg -3.1% · Bytecode: 🔴 19, 1 unch. · avg -10.9%
jsx.jsx — Interp: 🔴 19, 2 unch. · avg -4.7% · Bytecode: 🔴 21 · avg -17.9%
modules.js — Interp: 🔴 4, 5 unch. · avg -2.4% · Bytecode: 🔴 7, 2 unch. · avg -7.5%
numbers.js — Interp: 🔴 5, 6 unch. · avg -3.2% · Bytecode: 🔴 11 · avg -10.2%
objects.js — Interp: 🔴 2, 5 unch. · avg -2.7% · Bytecode: 🔴 7 · avg -12.7%
promises.js — Interp: 🔴 7, 5 unch. · avg -4.0% · Bytecode: 🔴 12 · avg -18.2%
regexp.js — Interp: 🔴 8, 3 unch. · avg -3.6% · Bytecode: 🔴 11 · avg -10.6%
strings.js — Interp: 🔴 13, 6 unch. · avg -20.1% · Bytecode: 🟢 3, 🔴 15, 1 unch. · avg -5.5%
tsv.js — Interp: 🔴 1, 8 unch. · avg -0.9% · Bytecode: 🔴 8, 1 unch. · avg -13.3%
typed-arrays.js — Interp: 🟢 1, 🔴 21 · avg -6.7% · Bytecode: 🔴 21, 1 unch. · avg -10.2%
uint8array-encoding.js — Interp: 🟢 5, 🔴 8, 5 unch. · avg +13.0% · Bytecode: 🔴 18 · avg -10.5%
Measured on ubuntu-latest x64. Benchmark ranges compare cached main-branch min/max ops/sec with the PR run; overlapping ranges are treated as unchanged noise. Percentage deltas are secondary context. |
Contributor
Suite Timing
Measured on ubuntu-latest x64. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@source/units/Goccia.Compiler.Statements.pas`:
- Around line 1619-1637: The loop that compiles pending-finally entries for
continue must pop the current Entry from GPendingFinally before compiling its
cleanup and then restore it afterward to avoid reusing the same entry during
nested abrupt completions; modify the block inside the for-loop (where Entry is
assigned and before calling CompileBlockStatement or
EmitDisposalSequence/EmitJumpInstruction/EmitInstruction) to snapshot/remove the
active entry (mirror the snapshot/pop/restore pattern used in
EmitPendingCleanup), call EmitInstruction(ACtx, EncodeABC(OP_POP_HANDLER,...))
and then compile the FinallyBlock or emit disposal sequence, and finally restore
the popped entry back onto GPendingFinally so the stack state matches the
original after the cleanup is emitted.
- Around line 1143-1145: Continue targets are currently patched after the
per-iteration OP_CLOSE_UPVALUE block which lets a continue skip the
upvalue-close; change the patch target so GContinueJumps (and any ContinueJumps
list handling) is set to a label placed immediately before the emitted
OP_CLOSE_UPVALUE sequence and then fall through into the increment/ITER_NEXT
back-edge, and update calls to PatchJumpTarget(ACtx, ContinueJumps[I]) to use
that label position; apply the same adjustment to the other loop-generation
sites (the other ContinueJumps patches referenced) so continues jump to just
before OP_CLOSE_UPVALUE rather than past it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fe58ebd2-8ce0-4835-afcf-9a42eaa394aa
📒 Files selected for processing (12)
source/units/Goccia.AST.Statements.passource/units/Goccia.Compiler.Statements.passource/units/Goccia.Compiler.passource/units/Goccia.ControlFlow.passource/units/Goccia.Error.Messages.passource/units/Goccia.Evaluator.passource/units/Goccia.Keywords.Reserved.passource/units/Goccia.Lexer.passource/units/Goccia.Parser.passource/units/Goccia.Token.passource/units/Goccia.Values.FunctionValue.pastests/language/for-of/for-of-continue.js
👮 Files not reviewed due to content moderation or server errors (11)
- source/units/Goccia.Token.pas
- source/units/Goccia.Error.Messages.pas
- source/units/Goccia.Lexer.pas
- source/units/Goccia.Compiler.pas
- source/units/Goccia.Keywords.Reserved.pas
- source/units/Goccia.Values.FunctionValue.pas
- source/units/Goccia.Evaluator.pas
- source/units/Goccia.AST.Statements.pas
- source/units/Goccia.Parser.pas
- source/units/Goccia.ControlFlow.pas
- tests/language/for-of/for-of-continue.js
…ally safety 1. Move continue-jump patch targets to before the OP_CLOSE_UPVALUE sequence in all three loop compilers (counted for-of, for-of, for-await-of) so that continue falls through the upvalue-closing instructions instead of skipping them — preserves correct closure semantics for captured loop bindings. 2. Apply the snapshot/pop/restore pattern from EmitPendingCleanup to CompileContinueStatement so that pending-finally entries are removed before compiling their cleanup blocks, preventing duplicate cleanup or infinite recursion when a finally block contains another abrupt completion. 3. Add test for closure capture correctness across continue. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #401.
continuewas completely unimplemented — the lexer treated it as an identifier, socontinue;inside a for-of loop producedReferenceError: Undefined variable: continuecfkContinue, tag value 3 in the 2-bit tagged-pointer scheme) → AST node → parser rule → evaluator propagation (including throughswitch) → bytecode compiler (GContinueJumpswith pending-finally support for all three loop variants: counted for-of, iterator for-of, for-await-of) → illegal-continue guard in function bodiesgttContinuein all five keyword-as-property-name lists so{ continue: "v" }keeps working