Conversation
Replace the recursive tree-walking interpreter with a CEK/CESK abstract machine whose continuation is an explicit, first-class stack of typed frames stepped by a driver loop. The environment becomes a first-class chain of shared scopes (preserving the previous set!/closure semantics), and each continuation frame carries a set of continuation marks - the representation Racket uses, and the foundation for the delimited- continuation primitives (prompts, call/cc, composable continuations) tracked as follow-ups. This also adds the machine-side pieces continuation marks need: the WithContinuationMark expression node, the ContinuationMarkSet runtime value, an eq?-approximating valueEq for mark keys, and the with- continuation-mark / current-continuation-marks transitions. They are not yet reachable from source (no parser wiring) or observable (no query primitives); that follows in the next commit. Behavior is otherwise unchanged - all existing tests pass.
Wire (with-continuation-mark key val result) into the parser and register the continuation-mark query primitives (current-continuation-marks, continuation-mark-set-first, continuation-mark-set->list) so marks are now both writable and observable from linklet source. A with-continuation-mark installs its key/value on the current continuation frame while the result runs in tail position; marks set for the same key in one frame overwrite, while marks in separate frames (e.g. a caller and a callee) accumulate innermost-first. Adds integration tests for each of these behaviours plus a parser unit test.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #83 +/- ##
==========================================
- Coverage 76.47% 75.17% -1.30%
==========================================
Files 26 25 -1
Lines 2567 2908 +341
Branches 371 393 +22
==========================================
+ Hits 1963 2186 +223
- Misses 604 722 +118
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4da6f46607
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Code reviewFound one significant issue in the new CEK machine's environment handling — see inline comment.
|
Details on the environment-scoping issue
Lines 327 to 332 in 4da6f46 // Build the callee environment: the caller's environment, extended with the
// closure's captured bindings, extended with the argument bindings.
EnvPtr AfterClosure = envExtend(ApplyEnv, C->getEnvironment());
auto CalleeScope = std::make_shared<Scope>();
CalleeScope->Parent = AfterClosure;
Since Concrete repro: (linklet () ()
(define-values (g) (lambda (x) y)) ; y is free in g's body, unbound at definition
(define-values (h) (lambda (y) (g 0))) ; h has a local binding for y
(h 42))Expected (Racket semantics): an unbound-identifier error for The existing test suite doesn't catch this because every closure test captures its free variables at definition time (so the captured binding shadows the caller's environment before the chain ever reaches it). Suggested fix: |
applyProcedure based the callee environment on the caller's environment (ApplyEnv), so a free variable left uncaptured at a closure's definition site (because it was unbound there) could resolve to a same-named binding at the call site - dynamic scoping. Base the callee chain on the persistent top-level GlobalEnv instead, so top-level define-values stay visible but caller locals never leak in. Also make getResult() return null on interpretation failure instead of asserting/dereferencing a null Result, so an unbound identifier exits cleanly (main already reports it) rather than aborting. Adds a regression test. Reported in review of 4da6f46 (claude[bot]).
with-continuation-mark wrote its mark into the enclosing continuation frame and never removed it, so in non-tail position the mark outlived its dynamic extent: (begin (with-continuation-mark 'k 1 0) (continuation-mark-set-first (current-continuation-marks) 'k)) returned 1 instead of #f. Push a dedicated mark-bearing frame that is popped when the result expression produces a value, bounding the mark's extent to that expression while keeping it visible across calls made within it. Same-key marks in separate frames still accumulate and continuation-mark-set-first still returns the innermost value; nested tail same-key marks no longer collapse in continuation-mark-set->list, consistent with the PR's documented no-tail-call-space-safety limitation. Adds a non-tail regression test and refreshes now-stale comments. Reported in review thread #discussion_r3507975126 (Codex).
|
@claude Regarding your code review of Fixed in Changed: the callee environment is now based on the persistent top-level This finding was independently confirmed by two AI reviewers (Claude Opus 4.8 and Codex GPT-5.5) before the fix was applied. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4bc809e34f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // produces a value (see the WcmMark case), so the mark's dynamic extent is | ||
| // exactly the result expression - a with-continuation-mark in non-tail | ||
| // position no longer leaks its mark into later expressions. | ||
| Kont.emplace_back(Frame::WcmMark); |
There was a problem hiding this comment.
When the result expression is itself a with-continuation-mark in tail position, this fresh WcmMark frame makes the two marks look like separate continuation frames. For example, (with-continuation-mark 'k 1 (with-continuation-mark 'k 2 (continuation-mark-set->list (current-continuation-marks) 'k))) returns (2 1) with this machine, but the inner tail-position mark should replace the outer mark for the same key on the current frame, so continuation-mark-set->list exposes a stale outer mark rather than just the current value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Out of scope for this PR — you're right: (with-continuation-mark 'k 1 (with-continuation-mark 'k 2 (continuation-mark-set->list (current-continuation-marks) 'k))) returns (2 1) here where Racket gives (2).
This is a symptom of the machine's documented lack of tail-call space safety: each with-continuation-mark currently gets its own frame, so directly-nested same-key marks aren't collapsed. continuation-mark-set-first is already correct, and only continuation-mark-set->list on directly-nested same-key marks diverges. A narrow patch (reuse the top frame in the WcmVal step when it is already a WcmMark) would fix only this exact shape and would leave (wcm 'k 1 (begin x (wcm 'k 2 e))) and (wcm 'k 1 (f)) still returning (2 1) — i.e. same-key collapse would depend on an incidental implementation artifact rather than a rule. The correct fix collapses same-key marks on genuine tail position together with tail-call space safety, so I'd rather land it holistically than special-case the WcmMark-on-top corner now.
Triaged by two independent AI reviewers (Claude Opus 4.8 and Codex GPT-5.5); they split FIX vs DEFER on scope, so this is deferred to a tracked issue rather than partially patched here. If you think the narrow fix is worth landing now, reply and we'll re-evaluate.
Reconcile the CEK abstract machine (with-continuation-mark, #11) with the features main gained since this branch was opened: the DiagnosticEngine error system (#57/#84) and the case-lambda (#3), letrec-values (#8), and #%variable-reference (#13) / keyword forms. - Closures now capture the live lexical environment chain instead of a copy of their free variables. This gives correct lexical scoping and makes letrec-values forward/mutual references resolve, and it subsumes the earlier dynamic-scope fix. The interpreter tracks every scope it creates and clears their bindings at teardown to break the closure/scope shared_ptr cycles this introduces (verified clean under LeakSanitizer). - All interpreter error paths now report through the DiagnosticEngine at the offending node's source location, matching main's messages; main() decides the exit status from Diag.hadError(). - case-lambda / case-lambda closures, #%variable-reference, and keyword are evaluated by the machine (case-lambda selects the first clause whose formals accept the argument count); letrec-values fills a recursive scope in place. All presets green: ctest 21/21, nora-lit 88/88 under debug, asan, and ubsan; warning-free build; clang-format clean.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59ad6299de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (Args.size() == 2) { | ||
| const auto *CMS = llvm::dyn_cast<ast::ContinuationMarkSet>(Args[0]); |
There was a problem hiding this comment.
When the mark-set argument is #f, this path either falls through after the failed ContinuationMarkSet cast or, for the generated 4-argument form, skips the lookup entirely because Args.size() != 2. The checked-in expander already relies on (continuation-mark-set-first #f parameterization-key) in expander/expander.rktl:79 and 4-argument calls like expander/expander.rktl:5358, so parameterization/code-inspector lookups will always see #f even inside a with-continuation-mark; this needs access to the current continuation like current-continuation-marks instead of being a pure runtime-function lookup.
Useful? React with 👍 / 👎.
1 participant
Summary
Implements
with-continuation-mark(issue #11). Because the interpreter had nofirst-class continuation to hang marks on, this takes the CEK/CESK abstract-machine
route agreed for the issue: the recursive tree-walker is replaced by a machine whose
continuation is an explicit stack of typed frames, and continuation marks live on
those frames (as they do in Racket). This is also the foundation for the remaining
delimited-continuation primitives, tracked as follow-ups #79, #80, #81.
Delivered in two reviewable commits:
frames + a driver loop, a first-class environment chain (preserving the previous
set!/closure semantics), per-frame mark maps, and theWithContinuationMarknode /ContinuationMarkSetvalue /valueEqinfrastructure. Behavior-preserving: allpre-existing tests pass on this commit alone.
current-continuation-marks,continuation-mark-set-first, andcontinuation-mark-set->list, with tests.Semantics
(with-continuation-mark key val result)evaluateskeyandval, installs the markon the current continuation frame, and evaluates
resultin tail position. Marks forthe same key in one frame overwrite; marks in separate frames (e.g. a caller and a
callee) accumulate, innermost-first.
Scope / follow-ups
Per the plan for this issue, the heavier delimited-continuation primitives are separate
follow-ups on top of this machine:
call-with-continuation-prompt/abort-current-continuationcall/cccall-with-composable-continuationKnown limitation for a future refinement: the machine pushes an activation frame per
call (no tail-call space-safety yet), so a tail loop that repeatedly re-marks the same
key accumulates frames rather than collapsing them. Mark values are correct; only the
space behaviour differs.
Fixes #11
Test plan
ctest --preset debug— 14/14 passing (adds awith-continuation-markparser test)nora-lit test/integration— 54/54 passing (adds 6with-continuation-mark*.rkttests)clang-format(v22) clean on all edited files