-
Notifications
You must be signed in to change notification settings - Fork 95
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 implementation of Decode for Box<str> and Box<[T]> #565
base: master
Are you sure you want to change the base?
Conversation
src/codec.rs
Outdated
@@ -1058,6 +1058,18 @@ impl Encode for str { | |||
} | |||
} | |||
|
|||
impl Decode for Box<str> { | |||
fn decode<I: Input>(input: &mut I) -> Result<Self, Error> { | |||
Ok(String::decode(input)?.into_boxed_str()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you check the already existing Box
Decode
implementation. It should be in the same way as this implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think that decoded String
will have some excess capacity or will it have capacity
== len
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've taken a bit different route here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should I maybe use WrapperTypeDecode
like this:
impl WrapperTypeDecode for Box<str> {
type Wrapped = String;
fn decode_wrapped<I: Input>(input: &mut I) -> Result<Self, Error> {
// Guaranteed to create a Vec with capacity == len
let vec = Vec::from(Box::<[u8]>::decode(input)?);
// Guaranteed not to reallocate the vec, only transmute to String
let str = String::from_utf8(vec).map_err(|_| "Invalid utf8 sequence")?;
// At this point we have String with capacity == len,
// therefore String::into_boxed_str will not reallocate
Ok(str.into_boxed_str())
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can I get a response here?
984c99c
to
c1b1067
Compare
23abd83
to
e86febe
Compare
hi, this PR should be ready for merge. Can someone approve it now? |
Would be great to get an impl for |
Signed-off-by: Marin Veršić <[email protected]>
I don't want to complicate this PR further as it's not being reviewed as it is. |
Closes #564
I can't think of a way to implement
Decode
for all unsized types