Skip to content

feat(borrow): flow-sensitive escape via outer = &y (CORE-01 pt3 Slice B, Refs #177)#351

Closed
hyperpolymath wants to merge 1 commit into
mainfrom
claude/bold-brahmagupta-0T7Bh
Closed

feat(borrow): flow-sensitive escape via outer = &y (CORE-01 pt3 Slice B, Refs #177)#351
hyperpolymath wants to merge 1 commit into
mainfrom
claude/bold-brahmagupta-0T7Bh

Conversation

@hyperpolymath
Copy link
Copy Markdown
Owner

Summary

CORE-01 Phase 3 Slice B — flow-sensitive escape via re-assignment. let mut r = &x; r = &y now correctly updates the borrow graph: the old held borrow on x is released and the (r → new_borrow) entry is re-bound to the freshly-created borrow on y. The NLL last-use machinery from Slice A (#335) now sees the current referent after re-assignment, not the stale original.

Before this PR (post-Slice-A behaviour):

let mut x = 1;
let mut y = 2;
let mut r = &x;
let z = *r;
r = &y;
x = 10;       // ← MoveWhileBorrowed: the &x held by r was never released
*r + x + z

After this PR: x = 10 is Ok, *r reads through the new borrow on y.

Mechanics in lib/borrow.ml StmtAssign

When LHS is a ref-binder symbol that already holds a borrow AND RHS is a direct &p/&mut p:

  1. Pre-release the old held borrow (via end_borrow) and remove the stale entry from ref_bindings before checking the RHS. The pre-release order matters for the same-target reborrow case (r = &mut x while r already holds &mut x): post-release ordering would trip ConflictingBorrow on the about-to-be-replaced exclusive borrow at record_borrow time, which is a sequential-modelling artefact, not a real conflict.
  2. Check the RHS — record_borrow adds the new borrow to state.borrows the usual way.
  3. After RHS-check, look up the freshly-created borrow on the new target place and bind it into ref_bindings as (binder_sym, new_borrow) — the symmetric assignment-side of record_ref_binding's let-graph contract.

Tests (E2E Borrow Graph, +3)

Fixture Direction Expected Distinguishes
slice_b_outer_assign_releases_old.affine positive Ok Without pre-release, x = 10 still fails MoveWhileBorrowed
slice_b_nll_expires_new.affine positive Ok Without re-bind, NLL expires the wrong borrow, y = 10 fails
slice_b_new_borrow_still_protects.affine anti-regression MoveWhileBorrowed Confirms new borrow is tracked, not silently dropped

Anti-regression sweep

All existing borrow / quantity / linear-arrow fixtures audited and remain green by construction: the new path only fires when LHS is a ref-binder AND RHS is a direct &p/&mut p. Every other StmtAssign shape (literal RHS, function-call RHS, non-ref-binder LHS, deref LHS) takes the pre-existing code path unchanged.

Scope of this PR

This completes the original "flow-sensitive escape" residual from CORE-01 pt3.

Deferred (Slices C–D residual):

  • Reborrow through indirection: r = some_other_ref_var (RHS not a direct &place) still leaves the ref-binding stale. Same limitation as record_ref_binding's let-graph path; would need symmetric ref-to-ref binding for both let and assign.
  • Slice C — origin/region variables (Polonius surface) + loan-live-at-point dataflow across CFG joins for ExprHandle/ExprTry/loops. Needs an ADR for the type-system shape before implementation.
  • Slice D — tighter quantity-checker integration for captured linears.

Docs

  • STATE.a2ml borrow-checkerphase-3-parts-1-3-Slices-A-and-B-landed
  • CAPABILITY-MATRIX.adoc borrow-checker row records Slice B
  • TECH-DEBT.adoc CORE-01 row records Slice B + narrows residual to Slices C–D

Test plan

  • CI build (opam exec -- dune build) green
  • CI lint + Run tests (opam exec -- dune runtest) — E2E Borrow Graph count +3 (the 3 new Slice B cases), all green
  • CI Run codegen WASM tests green — full stdlib AOT exercises the corpus
  • CI Check formatting green

Local-build caveat: container has no OCaml toolchain, so dune build / dune runtest were not run locally. CI is the source of truth.

Refs #177


Generated by Claude Code

…ce B, Refs #177)

Under purely lexical analysis (and even after Slice A's NLL last-use),
re-assigning a ref-binder — `let mut r = &x; r = &y` — left the
*old* held borrow on `x` in `state.borrows` and the *stale*
`(r -> old_borrow)` entry in `state.ref_bindings`. The new borrow
on `y` was added by `check_expr(rhs)` but never wired into the
ref-graph, so:

- `x = 10` after the reassignment was rejected as MoveWhileBorrowed
  even though `r` no longer pointed at `x` (the bug this PR fixes).
- NLL last-use on `r` expired the WRONG borrow (the old one on `x`),
  leaving the new borrow on `y` hanging — masking valid writes to
  `y`.
- `check_return_escape` looked up the stale `(r -> &x)` entry rather
  than the actual current referent `&y`.

This is the "flow-sensitive escape via assignment to an outer
mutable" residual called out in the Slice A docstring.

Implementation in `lib/borrow.ml` `StmtAssign`:

When LHS is a ref-binder symbol that already holds a borrow AND RHS
is a direct `&p`/`&mut p`, the code now:

1. *Pre*-releases the old held borrow (via `end_borrow`) and removes
   the stale entry from `ref_bindings` BEFORE checking the RHS. The
   pre-release order matters for the same-target reborrow case
   (`r = &mut x` while `r` already holds `&mut x`): post-release
   ordering would trip `ConflictingBorrow` on the about-to-be-
   replaced exclusive borrow at `record_borrow` time, which is
   user-confusing because the conflict is purely an artefact of
   sequential modelling. Pre-release dissolves the conflict.

2. Checks the RHS, which creates the new borrow on `state.borrows`
   the usual way (`record_borrow`).

3. After RHS-check, looks up the freshly-created borrow on the new
   target place and binds it into `ref_bindings` as
   `(binder_sym, new_borrow)` — the symmetric assignment-side of
   `record_ref_binding`'s let-graph contract.

Sound: NLL last-use, in-block `BorrowOutlivesOwner`, and
`check_return_escape` now consult the current referent. The new
borrow continues to live on `state.borrows` and continues to be
visible to `find_aliasing_exclusive` / "active borrow of LHS"
detection — see the anti-regression test.

Tests (E2E Borrow Graph, +3):
- `slice_b_outer_assign_releases_old.affine` — `r = &y` then
  `x = 10` is now Ok (the old borrow on `x` was released).
- `slice_b_nll_expires_new.affine` — after `r = &y` and `r`'s last
  use, NLL expires the NEW borrow (on `y`), so subsequent writes to
  BOTH `x` and `y` succeed. Without the re-bind, NLL would expire
  the wrong borrow and `y = 10` would fail.
- `slice_b_new_borrow_still_protects.affine` — anti-regression:
  after `r = &y`, while `r` is still live, `y = 10` must still
  fail (MoveWhileBorrowed). Pins that the new borrow IS tracked.

Existing tests audited and remain green by construction:
- `borrow_return_refparam_ok.affine`: no re-assignment in scope; no
  Slice B trigger. Unaffected.
- `borrow_return_escape_{param,local}.affine`: `let r = &x` then
  `return r` (no reassignment); same path as before.
- `borrow_nll_still_rejects_live_borrow.affine`: no `&p`-form RHS
  in scope; Slice B doesn't fire.
- All existing borrow / quantity / linear-arrow fixtures are
  untouched by the new path — the assignment branch only deviates
  when LHS is a ref-binder AND RHS is a direct `&p`/`&mut p`.

Deferred (Slices C–D residual):
- Reborrow through indirection: `r = some_other_ref_var` (RHS not
  a direct `&place`) still leaves the ref-binding stale. Same
  limitation as `record_ref_binding`'s let-graph path; would need
  symmetric ref-to-ref binding for both let and assign.
- Origin/region variables (Polonius surface) + loan-live-at-point
  dataflow across CFG joins for `ExprHandle`/`ExprTry`/loops.
- Tighter quantity-checker integration for captured linears.

Docs updated: `STATE.a2ml` borrow-checker → "Slices A and B
landed"; `CAPABILITY-MATRIX.adoc` borrow-checker row records
Slice B; `TECH-DEBT.adoc` CORE-01 row records Slice B + narrows
residual to Slices C–D.

NOTE: this container has no OCaml toolchain; `dune build` /
`dune runtest` were not run locally. CI is the source of truth.
Mechanically scoped to one branch of `StmtAssign`'s `Some place →
None`-conflict-on-LHS arm; all other code paths are unchanged.
@hyperpolymath hyperpolymath self-assigned this May 24, 2026
@hyperpolymath hyperpolymath enabled auto-merge May 24, 2026 20:17
hyperpolymath added a commit that referenced this pull request May 24, 2026
#356)

… STATE.a2ml)

Extracted doc-only changes from PR #351 to avoid merge conflicts.
TECH-DEBT.adoc renamed to TECH-DEBT-alt.adoc per user request.
auto-merge was automatically disabled May 24, 2026 20:36

Pull request was closed

@hyperpolymath hyperpolymath deleted the claude/bold-brahmagupta-0T7Bh branch May 24, 2026 20:36
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.

2 participants