Skip to content

Commit e62ab64

Browse files
Claude/pr351 no techdebt (#355)
Co-authored-by: Claude <noreply@anthropic.com>
1 parent 79a5298 commit e62ab64

5 files changed

Lines changed: 163 additions & 8 deletions

File tree

lib/borrow.ml

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1233,8 +1233,41 @@ and check_stmt (ctx : context) (state : state) (symbols : Symbol.t) (stmt : stmt
12331233
(* Can't assign while borrowed *)
12341234
Error (MoveWhileBorrowed (place, borrow))
12351235
| None ->
1236-
(* Assignment is ok, check the RHS *)
1237-
check_expr ctx state symbols rhs
1236+
(* Slice B (CORE-01 pt3 / #177): flow-sensitive escape via
1237+
`outer = &y`. If LHS is a ref-binder symbol that already
1238+
holds a borrow and RHS is a fresh `&y`/`&mut y` reference,
1239+
the assignment *replaces* the held borrow. We pre-release
1240+
the old held borrow before checking the RHS so the same-
1241+
target reborrow case (`r = &mut x` while `r` already holds
1242+
`&mut x`) does not trip [ConflictingBorrow] on the
1243+
about-to-be-replaced exclusive borrow; we then re-bind the
1244+
ref-graph entry to the freshly-created borrow. Mirrors
1245+
[record_ref_binding]'s let-graph contract for the
1246+
assignment path so the NLL last-use + return-escape
1247+
analyses see the *current* referent after re-assignment,
1248+
not the stale original. *)
1249+
let pre_release =
1250+
match root_var place, ref_target symbols rhs with
1251+
| Some binder_sym, Some _
1252+
when List.mem_assoc binder_sym state.ref_bindings ->
1253+
let old_borrow = List.assoc binder_sym state.ref_bindings in
1254+
end_borrow state old_borrow;
1255+
state.ref_bindings <-
1256+
List.filter (fun (s, _) -> s <> binder_sym) state.ref_bindings;
1257+
Some binder_sym
1258+
| _ -> None
1259+
in
1260+
let* () = check_expr ctx state symbols rhs in
1261+
(match pre_release, ref_target symbols rhs with
1262+
| Some binder_sym, Some new_target ->
1263+
(match List.find_opt (fun b ->
1264+
places_overlap b.b_place new_target) state.borrows with
1265+
| Some new_b ->
1266+
state.ref_bindings <-
1267+
(binder_sym, new_b) :: state.ref_bindings
1268+
| None -> ())
1269+
| _ -> ());
1270+
Ok ()
12381271
end
12391272
| None ->
12401273
(* LHS is not a place (e.g., function call result), check both sides *)
@@ -1341,11 +1374,26 @@ let check_program (symbols : Symbol.t) (program : program) : unit result =
13411374
Outer-block ref-bindings are deliberately preserved — they expire
13421375
only at their own block's exit.
13431376
1344-
Still deferred — region-/flow-sensitive remainder:
1345-
- Flow-sensitive escape via assignment to an outer mutable
1346-
(`outer = &x`): the assignment must update the borrow graph the
1347-
way [let r = &x] already does.
1377+
CORE-01 pt3 Slice B (flow-sensitive escape via re-assignment):
1378+
`outer = &y` now updates the borrow graph the way `let outer = &y`
1379+
does. In StmtAssign, when LHS is a ref-binder symbol with a held
1380+
borrow and RHS is a direct `&p`/`&mut p`, the old borrow is
1381+
pre-released (so a same-target reborrow like `r = &mut x` while
1382+
`r` already holds `&mut x` does not trip ConflictingBorrow on the
1383+
about-to-be-replaced exclusive borrow), then after the RHS check
1384+
the (binder -> new_borrow) ref-graph entry is re-bound to the
1385+
freshly-created borrow. NLL last-use + return-escape now see the
1386+
*current* referent after re-assignment, not the stale original.
1387+
1388+
Still deferred:
1389+
- Reborrow through indirection: `r = some_other_ref_var` (RHS not a
1390+
direct `&place`) does not yet copy the other binder's graph
1391+
entry — the ref-binding stays stale. Same limitation as
1392+
[record_ref_binding] for the let-graph path; would require
1393+
symmetric let/assign handling for ref-to-ref binding.
13481394
- Origin/region variables (Polonius surface) + loan-live-at-point
1349-
dataflow across CFG joins (Slice C).
1350-
- Tighter integration with the quantity checker for captured linears.
1395+
dataflow across CFG joins for `ExprHandle`/`ExprTry`/loops
1396+
(Slice C).
1397+
- Tighter integration with the quantity checker for captured
1398+
linears (Slice D).
13511399
*)
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// SPDX-License-Identifier: MPL-2.0
2+
// Copyright (c) 2026 Jonathan D.A. Jewell
3+
//
4+
// CORE-01 pt3 Slice B / #177 anti-regression: after `r = &y`, the
5+
// NEW borrow on `y` must still protect `y` while `r` is live. A
6+
// buggy Slice B that dropped the ref-binding but failed to re-bind
7+
// to the new borrow would silently accept `y = 10` below — the
8+
// borrow-graph would lose track of who holds &y. With correct
9+
// Slice B, `(r, &y)` is in ref_bindings, the borrow on `y` is in
10+
// state.borrows, and the assignment to `y` is rejected as
11+
// MoveWhileBorrowed. Expected: Error MoveWhileBorrowed.
12+
13+
module SliceBNewBorrowStillProtects;
14+
15+
fn still_protected() -> Int {
16+
let mut x = 1;
17+
let mut y = 2;
18+
let mut r = &x;
19+
r = &y;
20+
y = 10;
21+
*r
22+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// SPDX-License-Identifier: MPL-2.0
2+
// Copyright (c) 2026 Jonathan D.A. Jewell
3+
//
4+
// CORE-01 pt3 Slice B / #177: after `r = &y`, the NLL last-use
5+
// machinery must track the NEW borrow (on `y`), not the stale old
6+
// one (on `x`). Without Slice B, the ref-binding entry stays
7+
// `(r, &x)`, so when NLL expires `r` after its last use it ends the
8+
// wrong borrow — leaving the new borrow on `y` alive and
9+
// spuriously rejecting the later `y = 10`. With Slice B, the entry
10+
// is `(r, &y)` post-reassignment, NLL expires the correct borrow,
11+
// and both `x = 5` and `y = 10` succeed. Expected: Ok.
12+
13+
module SliceBNllExpiresNew;
14+
15+
fn nll_after_reassign() -> Int {
16+
let mut x = 1;
17+
let mut y = 2;
18+
let mut r = &x;
19+
r = &y;
20+
let z = *r;
21+
x = 5;
22+
y = 10;
23+
x + y + z
24+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// SPDX-License-Identifier: MPL-2.0
2+
// Copyright (c) 2026 Jonathan D.A. Jewell
3+
//
4+
// CORE-01 pt3 Slice B / #177: flow-sensitive escape via `outer = &y`.
5+
// Pre-Slice-B, the borrow held by `r` on `x` was never released when
6+
// `r` was re-assigned to `&y` — so `x = 10` was rejected as
7+
// MoveWhileBorrowed even though `r` no longer pointed at `x`. With
8+
// Slice B, the assignment `r = &y` ends the held borrow on `x` and
9+
// re-binds `r` in the ref-graph to the freshly-created borrow on `y`,
10+
// so the subsequent write to `x` is legal. Expected: Ok.
11+
12+
module SliceBOuterAssignReleasesOld;
13+
14+
fn outer_reassign() -> Int {
15+
let mut x = 1;
16+
let mut y = 2;
17+
let mut r = &x;
18+
let z = *r;
19+
r = &y;
20+
x = 10;
21+
*r + x + z
22+
}

test/test_e2e.ml

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4354,6 +4354,39 @@ let test_borrow_nll_still_rejects_live_borrow () =
43544354
Alcotest.fail "NLL over-expired a still-live borrow — assignment \
43554355
to a borrowed owner was accepted"
43564356

4357+
(* CORE-01 pt3 Slice B / #177 — flow-sensitive escape via `outer = &y`.
4358+
The assignment `r = &y` (where `r` is an existing ref-binder) now
4359+
releases the old held borrow and re-binds `r` in the ref-graph to
4360+
the freshly-created borrow. Three tests pin: (1) the old target is
4361+
freed by the reassignment; (2) NLL last-use then correctly expires
4362+
the NEW borrow; (3) anti-regression — the new borrow still
4363+
protects its new target while the binder is live. *)
4364+
let test_slice_b_outer_assign_releases_old () =
4365+
match borrow_result (fixture "slice_b_outer_assign_releases_old.affine") with
4366+
| Ok () -> ()
4367+
| Error e ->
4368+
Alcotest.fail ("Slice B: `r = &y` did not release the old borrow on \
4369+
`x` — `x = …` after the reassignment was spuriously \
4370+
rejected: " ^ Borrow.format_borrow_error e)
4371+
4372+
let test_slice_b_nll_expires_new () =
4373+
match borrow_result (fixture "slice_b_nll_expires_new.affine") with
4374+
| Ok () -> ()
4375+
| Error e ->
4376+
Alcotest.fail ("Slice B: NLL last-use after `r = &y` did not expire \
4377+
the new borrow on `y`: " ^ Borrow.format_borrow_error e)
4378+
4379+
let test_slice_b_new_borrow_still_protects () =
4380+
match borrow_result (fixture "slice_b_new_borrow_still_protects.affine") with
4381+
| Error (Borrow.MoveWhileBorrowed _) -> ()
4382+
| Error e ->
4383+
Alcotest.fail ("Slice B anti-regression: expected MoveWhileBorrowed \
4384+
on `y = …` (the new borrow must still protect `y`), \
4385+
got: " ^ Borrow.format_borrow_error e)
4386+
| Ok () ->
4387+
Alcotest.fail "Slice B regressed: the new borrow on `y` was not \
4388+
tracked, so the write to `y` was silently accepted"
4389+
43574390
let borrow_tests = [
43584391
Alcotest.test_case "BorrowOutlivesOwner: &local escapes its block"
43594392
`Quick test_borrow_outlives_owner;
@@ -4379,6 +4412,12 @@ let borrow_tests = [
43794412
`Quick test_borrow_nll_read_after_mut_last_use;
43804413
Alcotest.test_case "NLL anti-regression: still-live borrow blocks assign (#177 pt3 Slice A)"
43814414
`Quick test_borrow_nll_still_rejects_live_borrow;
4415+
Alcotest.test_case "Slice B: `r = &y` releases old borrow on `x` (#177 pt3 Slice B)"
4416+
`Quick test_slice_b_outer_assign_releases_old;
4417+
Alcotest.test_case "Slice B: NLL expires the NEW borrow after `r = &y` (#177 pt3 Slice B)"
4418+
`Quick test_slice_b_nll_expires_new;
4419+
Alcotest.test_case "Slice B anti-regression: new borrow still protects `y` (#177 pt3 Slice B)"
4420+
`Quick test_slice_b_new_borrow_still_protects;
43824421
]
43834422

43844423
(* ============================================================================

0 commit comments

Comments
 (0)