-
-
Notifications
You must be signed in to change notification settings - Fork 847
Add support for tagged values. #408
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
Conversation
@@ -41,7 +41,7 @@ pub trait Serialize { | |||
/////////////////////////////////////////////////////////////////////////////// | |||
|
|||
/// A trait that describes a type that can serialize a stream of values into the underlying format. | |||
pub trait Serializer { | |||
pub trait Serializer: Sized { |
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.
is that necessary?
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.
The compiler says so (and is usually right 😬).
Compiling serde v0.7.7 (file:///serde/serde)
/serde/serde/src/ser/mod.rs:359:15: 359:24 error: the trait bound `Self: std::marker::Sized` is not satisfied [E0277]
/serde/serde/src/ser/mod.rs:359 value.serialize(self)
^~~~~~~~~
/serde/serde/src/ser/mod.rs:359:15: 359:24 help: run `rustc --explain E0277` to see a detailed explanation
/serde/serde/src/ser/mod.rs:359:15: 359:24 help: consider adding a `where Self: std::marker::Sized` bound
error: aborting due to previous error
error: Could not compile `serde`.
Maybe it can be done in a different way but I don't know how.
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.
well... if you look at all the other default method impls, none call methods on the value. I'm aware of the motivation, that formats not caring about tags don't need to implement a new method. But maybe we need something along the lines of MapVisitor
and SeqVisitor
.
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.
Yes adding a TaggedVisitor
is possible. In this case the Sized
bound can be removed.
I have added support for tagged values in a branch of my CBOR crate |
#[inline] | ||
fn serialize_tagged_value<T, V>(&mut self, | ||
_tag: T, | ||
value: V) -> Result<(), Self::Error> |
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.
Nit: can you update the indent here?
How do you handle tagging higher level types, like a datetime value? Would the idea be that the serializer calls Another thing to consider is that specialization is coming down the pipeline. How could that impact this design? It would be interesting if there was a design that eventually could enable serde to have a registry of generic tag names that then could be used by the different serialization formats without having to do a runtime check. |
Some serialization formats allow the addition of "tags" to values to add additional information. These formats include CBOR, BSON and YAML. This commit introduces serialization and deserialization of tags. It introduces a trait Tagger that resolves the tag for a specific format. It supports integer, binary, and string tags and is extensible to cover more yet unknown formats. By default tags are discarded at deserialization except when the user has implemented visit_tagged_value function. To serialize tags the user has to call serialize_tagged_value. Tagged values may be of any type. Thanks to @dtolnay, @oli-obk and @erickt for their comments on tags. Supersedes #301
I'll take the rust-url serialization as an example. Currently the URL is serialized as a string: impl serde::Serialize for Url {
fn serialize<S>(&self, serializer: &mut S) -> Result<(), S::Error> where S: serde::Serializer {
format!("{}", self).serialize(serializer)
}
} Since CBOR recommends an optional tagging of URIs with tag 32 this information could be added to the implementation of impl serde::Serialize for Url {
fn serialize<S>(&self, serializer: &mut S) -> Result<(), S::Error> where S: serde::Serializer {
struct Tagger;
impl serde::Tagger for Tagger {
fn u64_tag(&self, format: &'static str) -> Option<u64> {
if format == "cbor" {
// URI; see RFC7049 Section 2.4.4.3
return Some(32);
}
None
}
fn string_tag(&self, format: &'static str) -> Option<&str> {
if format == "serde-registry" {
return Some("serde-url");
}
None
}
}
serializer.serialize_tagged_value(Tagger, self.as_str())
}
} |
@dtolnay Why is this "WIP"? What needs to be changed? |
So I've been thinking some about this. And I still dislike the runtime decision about the tag type. For serialization I played with impl specialization, and (I think) it works Output for serializing with
Output for serializing with
The impl specialization magic happens here: struct Tagged(i32);
trait SerializeTag<S: Serialize>: Serializer {
fn serialize_tag(&mut self, value: &S) -> Result<(), ()>;
}
impl SerializeTag<Tagged> for MySerializer {
fn serialize_tag(&mut self, value: &Tagged) -> Result<(), ()> {
println!("MySerializer::serialize_tag");
self.serialize_tagged(42, value.0)
}
}
impl<S: Serializer> SerializeTag<Tagged> for S {
default fn serialize_tag(&mut self, value: &Tagged) -> Result<(), ()> {
println!("S::serialize_tag (default)");
value.0.serialize(self)
}
}
impl Serialize for Tagged {
fn serialize<S>(&self, serializer: &mut S) -> Result<(), ()>
where S: Serializer {
println!("Tagged::serialize");
serializer.serialize_tag(self)
}
} I haven't put any thought into deserialization yet, but I feel confident we can rig something. Note: this design does not touch the |
Ok, there's an issue with this. It works fine if the |
Ah, nevermind, it's just horrible error reporting. If the |
I implemented the serialization part in serde on a branch based on yours (+ I merged master into it) https://github.com/oli-obk/rust-serde/commits/tagged_values |
I got deserialization to mostly work. it's now failing on something really weird. I think syntex is eating my code. |
Closing in favor of #455 which seems more flexible and more promising. We can reopen later if it turns out otherwise. |
Some serialization formats allow the addition of "tags"
to values to add additional information. These formats
include CBOR, BSON and YAML.
This commit introduces serialization and deserialization
of tags. It introduces a trait Tagger that resolves
the tag for a specific format. It supports integer,
binary, and string tags and is extensible to cover
more yet unknown formats.
By default tags are discarded at deserialization
except when the user has implemented visit_tagged_value
function.
To serialize tags the user has to call
serialize_tagged_value.
Tagged values may be of any type.
Thanks to @dtolnay, @oli-obk and @erickt for their comments on tags.
Supersedes #301