Skip to content

fix(repl-jit): fn redefinition rebinds instead of being silently ignored - #339

Merged
Ch4s3 merged 2 commits into
mainfrom
claude/cool-lumiere-2199e9
Aug 24, 2026
Merged

fix(repl-jit): fn redefinition rebinds instead of being silently ignored#339
Ch4s3 merged 2 commits into
mainfrom
claude/cool-lumiere-2199e9

Conversation

@Ch4s3

@Ch4s3 Ch4s3 commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Problem

At the REPL, redefining a function was silently ignored on both JIT backends:

march(1)> fn f(x) do x + 1 end
march(2)> fn f(x) do x + 100 end
march(3)> f(1)
= 2            <- should be 101

The second definition printed val f = <fn> but had no effect. Interpreter mode
(MARCH_REPL_INTERP=1) already rebound correctly (= 101), so the JIT paths
diverged from the language's Elixir-style rebinding semantics.

run_decl's is_fn_decl path early-returned whenever bind_name was already in
compiled_fns — a guard added for :reset scroll-replay, where the scroll system
resends 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 fn resolve through its
persistent closure slot (march_repl_get + indirect call at closure+16
verified in the emitted fragment IR), not through the LLVM symbol, so rebinding the
slot redirects every later call.

  • Fingerprint each fn declaration (Digest of the marshaled AST), recorded only
    after 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.
  • Unique symbol for a redefinition (f$redef$<n>), because the earlier fragment
    already defined f and ORC's single JITDylib hard-errors on duplicates (clang+dlopen
    would merely shadow ambiguously in the flat namespace). A capture-avoiding
    rename_top_fn_refs over the post-defun TIR makes self-recursion and first-class
    self-references follow the rename; locally bound shadows are left alone.
  • Filter the old slot out of prev_slots so the fragment emits no colliding slot
    loader 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.
  • Names compiled outside run_decl (stdlib prelude, :load) keep the historical
    skip — 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
    (101 not 2), 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 — new repl_session group: subprocess end-to-end sessions of the
    real 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.
  • New Repl_jit.fragment_count exposes 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.sh passes.

Pre-existing bugs found, not fixed here

Both reproduced on unmodified HEAD (68dff6e) with a fresh HOME, and filed
separately:

  1. A fn calling a previously REPL-defined fn (fn g(x) do f(x) end) fails on both
    backends — the slot loader define @f() collides with the declare @f extern in
    the same fragment. This is the "Follow-up noticed" item on the ORC-fix branch.
  2. A fn whose 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 f at all today, so there are
no 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.

Ch4s3 added 2 commits August 24, 2026 17:07
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
Ch4s3 merged commit b06b6ea into main Aug 24, 2026
25 checks passed
@Ch4s3
Ch4s3 deleted the claude/cool-lumiere-2199e9 branch August 24, 2026 22:03
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant