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

Add uninit buffer ancillary APIs #1108

Merged
merged 4 commits into from
Feb 26, 2025

Conversation

SUPERCILEX
Copy link
Contributor

Pretty sure I managed to maintain backcompat via a conversion trait.

Closes #1103

@SUPERCILEX
Copy link
Contributor Author

Nevermind, I don't think backcompat is possible without implementing a trait for both DerefMut<Target=[u8]> and DerefMut<Target=[MaybeUninit<u8>]> which I don't think you can do. Targeting the staging branch instead.

@SUPERCILEX SUPERCILEX changed the base branch from main to 1.0-staging August 15, 2024 02:08
@SUPERCILEX
Copy link
Contributor Author

Staging needs a rebase and also #1107

@SUPERCILEX SUPERCILEX changed the base branch from 1.0-staging to main August 15, 2024 03:24
@SUPERCILEX
Copy link
Contributor Author

Annnnnd I decided to make it backwards compatible again. The public API should be identical except there's now a From impl for MaybeUninit. 1.0 should break the ::new impl to take uninits instead.

@SUPERCILEX
Copy link
Contributor Author

Not sure why the tests are so borked.

@SUPERCILEX SUPERCILEX changed the title Make all ancillary APIs use uninit buffers Add uninit buffer ancillary APIs Aug 15, 2024
@SUPERCILEX
Copy link
Contributor Author

Ok so the idea with this one is that the ::new methods would be swapped to use MaybeUninits for 1.0 and any way to construct a buffer with [u8] would be removed. I'm not really happy with the deprecation story as it feels like we'll just be springing this change on people, but I also don't see a way to allow seamless migration while preserving the name new.

@sunfishcode sunfishcode mentioned this pull request Jan 23, 2025
21 tasks
@sunfishcode
Copy link
Member

I've now rebased this and fixed the merge conflicts.

@SUPERCILEX
Copy link
Contributor Author

Can we remove the non MaybeUninit versions? I personally don't think you should be using u8s for these buffers since we take care of giving you a nice well typed iterator.

@sunfishcode
Copy link
Member

Yep, I was just reading your comment above about that too.

@SUPERCILEX
Copy link
Contributor Author

Brilliant, this looks great!

@sunfishcode sunfishcode merged commit 3b0e286 into bytecodealliance:main Feb 26, 2025
45 checks passed
@sunfishcode
Copy link
Member

Thanks!

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.

Make RecvAncillaryBuffer and SendAncillaryBuffer use MaybeUninits
2 participants