Vec improvements
#427
Vec improvements
#427
Conversation
src/vec.rs
Outdated
| fn from(buffer: [T; N]) -> Self { | ||
| Self { | ||
| // cast [T; N] into [MaybeUninit<T>; N] | ||
| buffer: unsafe { buffer.as_ptr().cast::<[MaybeUninit<T>; N]>().read() }, |
There was a problem hiding this comment.
this does a bitwise copy of the elements into the Vec, but will still drop them when buffer goes out of scope. This will cause use-after-frees if T is an owned type, like Box.
You can wrap the buffer into a ManuallyDrop to avoid this.
src/vec.rs
Outdated
| @@ -858,6 +888,17 @@ impl<T, const N: usize> Drop for Vec<T, N> { | |||
| } | |||
| } | |||
|
|
|||
| impl<T, const N: usize> From<[T; N]> for Vec<T, N> { | |||
There was a problem hiding this comment.
it feels a bit strange that the array length becomes the capacity. Intuitively I'd say it should be possible to convert a [u8; 10] into a Vec<u8; 20> with 10 filled elements.
This would fail if CAP < N, and there's no way to add where CAP >= N in const generics, so tihs'd have to be TryFrom which perhaps is not that ergonomic?
There was a problem hiding this comment.
There is a way to generate a compile error in that case. This PR does this: #352
There was a problem hiding this comment.
I thought doing it this way would allow me to "move" the array into the Vec without copying, but it turns out that copying will happen either way, so might as well allow N < CAP.
| heapless::Vec::<_, $cap>::new() | ||
| }; | ||
|
|
||
| ($($elem:expr),+ ; $cap:expr) => { |
There was a problem hiding this comment.
this should be consistent with std.
As it's written now, vec![42; 5] in std it would create a vec with five 42's, while in heapless it creates a vec with a single 42 with a capacity of 5.
IMO the macro should behave like this:
vec![1,2,3]: 3 elements, capacity is inferred.vec![42; 3]: element repeated 3 times, capacity is inferred.
and then invent some extra syntax for specifying capacity explicitly that doesn't conflict with that.
In #369 we had the same issue, we went with format!(256; "foo {}", 42) where 256 is the capacity, but that doesn't work well here...
|
in the end i've merged #352 for the "from array" conversion because it was a bit more complete (more tests). Could you open separate PRs for the two remaining features so they can be reviewed/discussed separately? (the |
Add
spare_capacity_mutandFromimplementation forVec, as well asvec!macro.Resolves #426 and #344