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 support for the Rust's unstable allocator-api on Vec #14

Closed
wants to merge 1 commit into from

Conversation

aabizri
Copy link

@aabizri aabizri commented Aug 21, 2022

This PR adds feature-gated support for rust's new(ish) allocator api on Vec (see rust-lang/rust#32838 & https://doc.rust-lang.org/unstable-book/library-features/allocator-api.html).

This enables support for custom allocators, which are often needed in video games & high performance projects, as well as make it possible for libraries supporting the API to use encase with user-provided Vecs. I haven't added support for Box<T> which now can also take a custom allocator, as that feature still sometime causes ICEs.

As for the feature name, I chose allocator_api, as that's also the name of Rust's feature (and it's disabled by default).

This takes the form of an additional bound on Vecs, which now look like Vec<T, A> where A: std::alloc::Allocator.

@aabizri
Copy link
Author

aabizri commented Aug 21, 2022

It seems that clippy & MSRV checks are failing as they enable all features, and allocator_api is unstable (and thus not usable on stable). If @teoxoy would like to go-ahead with this new feature, those checks will need to swap --all-features with --features mint,cgmath,glam,nalgebra,ultraviolet,vek,smallvec,arrayvec,tinyvec,ndarray,rpds,archery,im,im-rc,imbl,static-rc.

@teoxoy
Copy link
Owner

teoxoy commented Aug 23, 2022

Thanks for the PR!

To avoid the code duplication could you try something like this:

impl<#[cfg(feature = "allocator_api")] A: std::alloc::Allocator>
ByteVecExt for Vec<u8, #[cfg(feature = "allocator_api")] A>

Also, I feel like we should also extend this PR to cover all traits implemented for Vec (ShaderType, ShaderSize, ...)

On a related note, do you know how close this is to stabilization?

@teoxoy
Copy link
Owner

teoxoy commented Dec 9, 2022

@aabizri are you still interested in pursuing this?

@teoxoy teoxoy closed this Feb 12, 2023
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.

None yet

2 participants