Repository navigation
fix(hugr-llvm): free borrow array mask on discard - #3321
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are recorded for the borrow-array discard leak fix.
No open issues 🎉
What changed in this PR
Fixes the borrow-array discard leak in hugr-llvm reported in #3320 by releasing both the elements buffer and the borrow mask.
Changes:
- Frees both pointers in
emit_barray_discard. - Adds a JIT regression test that checks allocation accounting across 100 create/access/discard cycles.
- Updates LLVM 21 snapshots to include mask deallocation.
| File | Description |
|---|---|
hugr-llvm/src/extension/collections/snapshots/hugr_llvm__extension__collections__borrow_array__test__emit_get@pre-mem2reg@llvm21.snap |
Records mask deallocation before mem2reg for get/discard. |
hugr-llvm/src/extension/collections/snapshots/hugr_llvm__extension__collections__borrow_array__test__emit_get@llvm21.snap |
Records mask deallocation after mem2reg for get/discard. |
hugr-llvm/src/extension/collections/snapshots/hugr_llvm__extension__collections__borrow_array__test__emit_clone@pre-mem2reg@llvm21.snap |
Records both arrays’ mask deallocation before mem2reg. |
hugr-llvm/src/extension/collections/snapshots/hugr_llvm__extension__collections__borrow_array__test__emit_clone@llvm21.snap |
Records both arrays’ mask deallocation after mem2reg. |
hugr-llvm/src/extension/collections/borrow_array.rs |
Fixes discard cleanup and adds the allocation-accounting regression test. |
Review effort: Balanced
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3321 +/- ##
==========================================
+ Coverage 82.08% 82.10% +0.01%
==========================================
Files 245 245
Lines 47579 47612 +33
Branches 41144 41177 +33
==========================================
+ Hits 39056 39090 +34
+ Misses 6619 6617 -2
- Partials 1904 1905 +1
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:
|
BArrayDiscardfreed the elements buffer but left the borrow mask allocated. Free both pointers when discarding the array.The borrow-array lowering was introduced in #2574.
Add a JIT regression test that creates, accesses, and discards an array 100 times while accounting for allocations and frees. The test fails when the mask free is removed. Update the LLVM snapshots.
Closes #3320
Validation
cargo test -p hugr-llvm extension::collections::borrow_array::test --lib(84 passed, LLVM 21)cargo fmt --all -- --checkandgit diff --checkborrow_array.rs: no findings