Skip to content

Release v0.2.2#7

Merged
dnkats merged 1 commit into
mainfrom
ver0.2.2
Feb 3, 2026
Merged

Release v0.2.2#7
dnkats merged 1 commit into
mainfrom
ver0.2.2

Conversation

@dnkats

@dnkats dnkats commented Feb 3, 2026

Copy link
Copy Markdown
Member

Fixes scope issues in @buffer

Fixes scope issues in `@buffer`
Copilot AI review requested due to automatic review settings February 3, 2026 10:21
@dnkats dnkats merged commit f001256 into main Feb 3, 2026
6 checks passed
@dnkats dnkats deleted the ver0.2.2 branch February 3, 2026 10:23

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the @buffer macros to avoid the let/try...finally structure that introduced scope issues, and records the change in the changelog for the v0.2.2 release.

Changes:

  • Modify the single-argument @buffer(specs, ex) macro to allocate the MAllocBuffer directly in the caller scope, run ex, then free and clear the buffer variable without using let or try...finally.
  • Modify the two-buffer @buffer(specs, specs2, ex) macro analogously: allocate both buffers in the caller scope, execute ex, then free and clear both buffer variables, again without let or try...finally.
  • Add a v0.2.2 entry to CHANGELOG.md documenting that the scope-related changes to @buffer (use of let and try blocks) have been reverted.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/mbuffer.jl Adjusts both @buffer macro definitions to avoid let/try...finally blocks by assigning buffers directly in the caller scope, then freeing and nulling the buffer variables.
CHANGELOG.md Adds release notes for v0.2.2 describing the reversion of the earlier @buffer changes that caused scope issues.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/mbuffer.jl
Comment on lines +133 to +136
$(esc(buf)) = MAllocBuffer{$(esc(T))}($(esc(len)))
$(esc(ex))
free!($(esc(buf)))
$(esc(buf)) = nothing

Copilot AI Feb 3, 2026

Copy link

Choose a reason for hiding this comment

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

Removing the let/try...finally structure here means free! is no longer guaranteed to run if ex throws, so MAllocBuffer's underlying memory will leak on exceptions (there is no finalizer on MAllocBuffer, and free! is the only place that calls Libc.free). Please restore exception-safe cleanup (e.g., via a try...finally or equivalent pattern that still fixes the scope issue) so that @buffer truly frees the buffer after use even on error.

Copilot uses AI. Check for mistakes.
Comment thread src/mbuffer.jl
Comment on lines +150 to +156
$(esc(buf)) = MAllocBuffer{$(esc(T))}($(esc(len)))
$(esc(buf2)) = MAllocBuffer{$(esc(T2))}($(esc(len2)))
$(esc(ex))
free!($(esc(buf2)))
free!($(esc(buf)))
$(esc(buf2)) = nothing
$(esc(buf)) = nothing

Copilot AI Feb 3, 2026

Copy link

Choose a reason for hiding this comment

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

As with the single-buffer macro, dropping the let/try...finally wrapper means that if ex throws, neither free!($(esc(buf2))) nor free!($(esc(buf))) will run, leaking both MAllocBuffer instances. Consider reinstating an exception-safe cleanup pattern that still avoids the scope issues you observed, so both buffers are always freed even when ex fails.

Copilot uses AI. Check for mistakes.
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