Introduce Serializer::collect_str (fixes #786) #789
Conversation
|
For |
|
We can optimize this with a statically allocated buffer in a separate commit. This API looks reasonable to me. @nox could you also implement the serde_json side of this? |
|
My main concern is that the |
|
@dtolnay Done. |
|
@clarcharr I will change the error message tomorrow. Any suggestions? |
|
@nox Perhaps "collect_str is currently unimplemented for no_std builds" ? |
|
I changed the error message and used |
serde/src/ser/mod.rs
Outdated
| where T: Display, | ||
| { | ||
| let mut string = String::new(); | ||
| write!(string, "{}", value).unwrap(); |
There was a problem hiding this comment.
Any advantages of this over value.to_string()? I guess in case there is a specialized ToString impl to make this default implementation behave consistently with a serializer that overrides collect_str with a more efficient Display-based implementation? I think people deserve what they get if they implement ToString and Display in a way that is not consistent with each other.
ToString may be faster rust-lang/rust#18404 (not that I expect this method to be used with T=str).
There was a problem hiding this comment.
I would like to use ToString, but unfortunately, I have to hate it for now.
serde/src/ser/mod.rs
Outdated
| /// The default implementation serializes the given value as a string with | ||
| /// `ToString::to_string`. | ||
| #[cfg(any(feature = "std", feature = "collections"))] | ||
| fn collect_str<T>(self, value: &T) -> Result<Self::Ok, Self::Error> |
|
Would it be reasonable to add a token to |
|
I don't think so, semantically it is no different from a string. |
The default implementation collects the Display value into a String and then passes that to Serializer::serialize_str when the std or collections features are enabled, otherwise it unconditionally returns an error.
The previous message was targeted toward Serializer implementors, which is not the group that will be seeing this message most often.
|
Very cool idea — I was just looking for something like this! Thanks! |
No description provided.