allow to_extend to work with reference to write#210
allow to_extend to work with reference to write#210e00E wants to merge 1 commit intojamesmunns:mainfrom
Conversation
Currently the function takes the collection by value. Taking by mutable reference is better because it is more flexible. Whenever you have a value, you can also pass by mutable reference, but sometimes you only have a mutable reference and cannot pass by value. This also fixes another problem with the current API: It keeps the collection on error. On success it returns it to the caller, but on error it does not. This makes the caller lose the collection. I implement this as a new function rather than by changing the existing to_extend so that this is not a breaking change. When postcard is willing to make breaking changes in the future, then it could decide to remove the existing to_extend.
✅ Deploy Preview for cute-starship-2d9c9b canceled.
|
|
Can we also have something like this for |
|
It's not necessary. The only difference is that on a vector you can call |
|
Do you think that compiler replaces |
|
If self is fn foo(extend: &mut impl Extend<u8>, iter: impl Iterator<Item = u8>) {
extend.extend(iter);
}
pub fn with_extend(a: &mut Vec<u8>, b: &[u8]) {
foo(a, b.iter().copied());
}
pub fn with_extend_from_slice(a: &mut Vec<u8>, b: &[u8]) {
a.extend_from_slice(b);
}Both of the pub functions will reserve the right amount of space in the vector and then call memcpy. https://godbolt.org/z/81o3hvj58 |
Shatur
left a comment
Shatur
left a comment
There was a problem hiding this comment.
Thanks, I copied the implementation into my project, works perfectly :)
I left a couple suggestions.
| /// | ||
| /// This is like [`ExtendFlavor`], but it takes the writer by reference instead of | ||
| /// by value. This allows you to remain in control of the writer. | ||
| pub struct ExtendRefFlavor<'a, T> { |
There was a problem hiding this comment.
Maybe use the word "mut" instead? When I see "ref", I usually expect &.
| pub struct ExtendRefFlavor<'a, T> { | |
| pub struct ExtendMutFlavor<'a, T> { |
| /// This is like [`ExtendFlavor`], but it takes the writer by reference instead of | ||
| /// by value. This allows you to remain in control of the writer. | ||
| pub struct ExtendRefFlavor<'a, T> { | ||
| iter: &'a mut T, |
There was a problem hiding this comment.
Maybe call it bytes? Extend is not a trait for iterator, it represent a collection that can accept an iterator to extend itself.
| iter: &'a mut T, | |
| bytes: &'a mut T, |
| where | ||
| T: core::iter::Extend<u8>, | ||
| { | ||
| type Output = (); |
jamesmunns
added
the
Q2 '25 Triage
3 participants
Currently the function takes the collection by value. Taking by mutable reference is better because it is more flexible. Whenever you have a value, you can also pass by mutable reference, but sometimes you only have a mutable reference and cannot pass by value.
This also fixes another problem with the current API: It keeps the collection on error. On success it returns it to the caller, but on error it does not. This makes the caller lose the collection.
I implement this as a new function rather than by changing the existing to_extend so that this is not a breaking change. When postcard is willing to make breaking changes in the future, then it could decide to remove the existing to_extend.
This is an alternative to #208 .