Conversation
… and `FromIterator<&'a char>` for String and WString ros2-rust#246
nnmm
left a comment
nnmm
left a comment
There was a problem hiding this comment.
Thanks for your PR! I left a comment.
nnmm
left a comment
nnmm
left a comment
There was a problem hiding this comment.
LGTM, just some small changes to tidy up are remaining.
rosidl_runtime_rs/src/string.rs
Outdated
| } | ||
| } | ||
|
|
||
| impl From<&std::string::String> for $string { |
There was a problem hiding this comment.
Since the existing From<&str> impl is more general, I think this impl isn't necessary.
There was a problem hiding this comment.
So in *self = Self::from(&s);, &s is actually of type &String and From isn't implemented for that type. So I removed this impl by just using Self::from(s.as_str()) in the Extend impl
There was a problem hiding this comment.
That's correct, though using Self::from(&s) should work too due to Deref coercion.
| ) -> bool; | ||
| } | ||
|
|
||
| impl Default for $string { |
There was a problem hiding this comment.
Alphabetizing the trait impl's
nnmm
left a comment
nnmm
left a comment
There was a problem hiding this comment.
Thanks for making these changes!
Thanks for all the guidance :) |
2 participants
Implemented
Extend<char>,Extend<&'a char>,FromIterator<char>, andFromIterator<&'a char>for String and WString.Of note here is that with this change, String and WString now have interior mutability so they are no longer able to implement
Syncsafely.Original Issue: #246