Skip to content

fix(hugr-llvm): free borrow mask when converting to an array - #3322

Merged
qartik merged 1 commit into
codex-fix-borrow-array-discardfrom
codex-fix-borrow-array-conversion-leak
Nov 5, 2026
Merged

qartik merged 1 commit into
codex-fix-borrow-array-discardfrom
codex-fix-borrow-array-conversion-leak

Conversation

@qartik

@qartik qartik commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

Converting a borrow array to an ordinary array transfers the element buffer but leaves the borrow mask allocated. Free the mask after checking that no elements remain borrowed.

The borrow-array lowering was introduced in #2574.

Stacked on #3321.

Validation: the HUGR LLVM borrow-array tests pass, including a JIT allocation-accounting regression.

@qartik
qartik added this pull request to stack #3323 November 4, 2026 21:31
@qartik
qartik requested a balanced review from Copilot November 4, 2026 21:32
@qartik
qartik marked this pull request as ready for review November 4, 2026 21:32
@qartik
qartik requested a review from a team as a code owner November 4, 2026 21:32
@qartik
qartik requested a review from mark-koch November 4, 2026 21:32
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-04T21:33:27.711273Z b71c0bd Draft marked ready
🔒 Security Review ✅ Completed 2026-11-04T21:35:37.988770Z b71c0bd Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@codecov

codecov Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.87179% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.11%. Comparing base (f283f3b) to head (b71c0bd).

Files with missing lines Patch % Lines
...ugr-llvm/src/extension/collections/borrow_array.rs 94.87% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@                        Coverage Diff                         @@
##           codex-fix-borrow-array-discard    #3322      +/-   ##
==================================================================
+ Coverage                           82.10%   82.11%   +0.01%     
==================================================================
  Files                                 245      245              
  Lines                               47612    47651      +39     
  Branches                            41177    41216      +39     
==================================================================
+ Hits                                39090    39127      +37     
- Misses                               6617     6618       +1     
- Partials                             1905     1906       +1     
Flag Coverage Δ
python 89.72% <ø> (ø)
rust 80.92% <94.87%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues remain for the borrow-array conversion memory-leak fix.

No open issues 🎉

What changed in this PR

Fixes a memory leak in HUGR LLVM borrow-array conversion by freeing the borrow mask while transferring the element buffer to the ordinary array.

Changes:

  • Frees the mask after verifying that no elements remain borrowed.
  • Adds a JIT regression test that repeats conversion, access, and disposal 100 times, checking returned values, allocation counts, invalid frees, and outstanding allocations.
File Description
hugr-llvm/​src/​extension/​collections/​borrow_array.rs Frees the mask during conversion and adds allocation-accounting regression coverage.

Review effort: Balanced

@qartik
qartik added this pull request to the merge queue Nov 5, 2026
Merged via the queue into main with commit 7df7263 Nov 5, 2026
36 checks passed
@qartik
qartik deleted the codex-fix-borrow-array-conversion-leak branch November 5, 2026 13:39
@hugrbot hugrbot mentioned this pull request Nov 5, 2026
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.

3 participants