fix(repl-jit): fn redefinition rebinds instead of being silently ignored - #339
Merged
Conversation
run_decl's is_fn_decl path skipped ANY already-compiled bind_name — a guard meant only for :reset scroll-replay — so redefining a fn at the REPL kept the first body on both JIT backends (f(1) = 2 after redefining to x + 100), while interpreter mode correctly rebound (= 101). Fingerprint each fn declaration (Digest of the marshaled AST, recorded only after a successful compile): identical resends keep the replay fast path, a changed body recompiles under a session-unique symbol ($redef$<n>, via a capture-avoiding TIR rename so self-recursion follows) and rebinds the closure slot — which is how cross-fragment calls resolve, so later calls pick up the new body. Names compiled outside run_decl (stdlib prelude, :load) keep the historical skip: they are called as direct externs, not through a slot. The old binding's slot is filtered out of prev_slots so the redefinition fragment emits no colliding slot loader under ORC; this keeps the fix independent of (and composable with) the ORC loader-linkage work. Tests: in-process redefinition/replay/self-recursion/arity-change coverage in test_codegen.ml (red pre-fix: Received "2"), plus subprocess REPL session tests in test_jit.ml for clang, ORC (skips without libLLVM), and interpreter mode. New Repl_jit.fragment_count exposes the replay skip.
The dune action pins HOME to _build/jit_home, a directory nothing creates. The REPL fatals at startup when $HOME/.cache cannot be created, so under `dune runtest` all three session tests died before reaching an assertion — invisible locally, where running the exe directly bypasses dune's setenv and inherits the developer's real HOME. Create and reuse one per-process temp HOME instead. Also keeps the stdlib precompile and JIT .so caches out of ~/.cache. Verified non-vacuous: 3 failures with the override reverted, 0 with it.
Ch4s3
added a commit
that referenced
this pull request
Aug 25, 2026
…dupe The redefinition path's filtered prev_slots + emit-name renaming merged with this branch's session_wraps threading; redefinition emits carry both. Validated: repl_jit_cross_line 12/12, repl_jit_regression 15/15, test_jit 7/7 (incl. the new redefinition trio on both backends through the merged run_decl), parity 16/16.
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
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.
Problem
At the REPL, redefining a function was silently ignored on both JIT backends:
The second definition printed
val f = <fn>but had no effect. Interpreter mode(
MARCH_REPL_INTERP=1) already rebound correctly (= 101), so the JIT pathsdiverged from the language's Elixir-style rebinding semantics.
run_decl'sis_fn_declpath early-returned wheneverbind_namewas already incompiled_fns— a guard added for:resetscroll-replay, where the scroll systemresends prior cells verbatim and recompiling is pure waste. The guard could not
tell a verbatim resend from a genuine redefinition.
Approach
Redefinition rebinds, matching interpreter mode. The mechanism the JIT already uses
makes this cheap: cross-fragment calls to a REPL-defined
fnresolve through itspersistent closure slot (
march_repl_get+ indirect call atclosure+16—verified in the emitted fragment IR), not through the LLVM symbol, so rebinding the
slot redirects every later call.
Digestof the marshaled AST), recorded onlyafter the fragment compiles and loads — the same success-only discipline as
mark_compiled_fns, so a failed compile leaves state intact for a clean retry.Identical resend → replay fast path preserved; changed body → recompile and rebind.
f$redef$<n>), because the earlier fragmentalready defined
fand ORC's single JITDylib hard-errors on duplicates (clang+dlopenwould merely shadow ambiguously in the flat namespace). A capture-avoiding
rename_top_fn_refsover the post-defun TIR makes self-recursion and first-classself-references follow the rename; locally bound shadows are left alone.
prev_slotsso the fragment emits no colliding slotloader under ORC. This is what makes the fix independent of the unmerged ORC
loader-linkage work on
claude/jit-repl-interpreted-perf-68373c; the two compose.run_decl(stdlib prelude,:load) keep the historicalskip — they are called as direct externs, not through a slot, so a redefinition
could never take effect there anyway.
Tests
test/test_codegen.ml— in-process (clang): original body, redefinition(
101not2), identical replay compiles no new fragment and keeps the binding,self-recursive redefinition reaches the new body, arity-changing redefinition.
Verified red pre-fix (
Received: "2").test/test_jit.ml— newrepl_sessiongroup: subprocess end-to-end sessions of thereal binary for clang, ORC (
MARCH_JIT_BACKEND=orc, skips when libLLVM is absent),and interpreter mode. Subprocess because a backend SIGSEGV would otherwise take down
the runner, and because the backend env vars are read once at module init. Both JIT
sessions verified red pre-fix; interpreter green before and after.
Repl_jit.fragment_countexposes the replay-skip assertion to tests.Full suite green: compiler 932, eval 272, codegen 588, stdlib 868 (incl.
Slow),stdlib_march 61, jit 4, snapshots 33.
scripts/check-docs.shpasses.Pre-existing bugs found, not fixed here
Both reproduced on unmodified
HEAD(68dff6e) with a freshHOME, and filedseparately:
fncalling a previously REPL-definedfn(fn g(x) do f(x) end) fails on bothbackends — the slot loader
define @f()collides with thedeclare @fextern inthe same fragment. This is the "Follow-up noticed" item on the ORC-fix branch.
fnwhose body contains any lambda fails its helpers-fragment dlopen(
symbol not found in flat namespace '_f') and loses the binding.Because of (1), later REPL functions cannot reference
fat all today, so there areno stale direct-call sites for a redefinition to miss. If that limitation is lifted,
direct extern calls inside other functions would pin the version compiled at their
definition time and should be routed through the slot instead — noted in the spec.
Details in
specs/progress/2026-08-24-repl-jit-fn-redefinition-silently-ignored.md.