Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

helpers: Add AlignedBuffer #1600

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

seijikun
Copy link
Contributor

@seijikun seijikun commented Apr 1, 2025

AlignedBuffer is a helper class that manages the livetime of a memory region, allocated using a certain alignment. Like Box, it handles deallocation when the object isn't used anymore.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

@nicholasbishop
Copy link
Member

As mentioned in #1595 (comment), let's move to the mem module.

@seijikun seijikun force-pushed the mr-alignedbuffer branch 2 times, most recently from 9263616 to bffd748 Compare April 1, 2025 22:37
Copy link
Member

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

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

Thanks for your many contributions and your patience! I left a few remarks.

If you resolve them, we are good to go

@phip1611 phip1611 requested a review from nicholasbishop April 2, 2025 11:03
@seijikun seijikun force-pushed the mr-alignedbuffer branch 2 times, most recently from c37c461 to 812a105 Compare April 2, 2025 12:29
@seijikun seijikun requested a review from phip1611 April 2, 2025 12:34
AlignedBuffer is a helper class that manages the livetime of a memory region, allocated using
a certain alignment. Like Box, it handles deallocation when the object isn't used anymore.
@seijikun
Copy link
Contributor Author

seijikun commented Apr 5, 2025

Hm. The Lint failures seem to be outside the code I changed in this PR

@nicholasbishop
Copy link
Member

Probably due to 1.86 going stable recently. If you want to, please put up a separate pr with fixes for those lints. Or I can do it sometime this weekend probably.

@phip1611 phip1611 mentioned this pull request Apr 5, 2025
2 tasks
@phip1611
Copy link
Member

phip1611 commented Apr 5, 2025

I've set up #1602.

Thanks for your patience, @seijikun. We are almost there!

Copy link
Member

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

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

LGTM now!

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