Skip to content

allow types to specify the kind of serializer they should be used with or carry state #3740

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

Closed
nikomatsakis opened this issue Oct 12, 2012 · 8 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@nikomatsakis
Copy link
Contributor

Right now, the serialization interface requires that the type to be serialized be generic with respect to any sort of serializer:

trait Serializable { fn serialize<S: Serializer>(&self, s: &s); }

Unfortunately, some types require context to be serialized, such as ty::t or span. When doing the original design I had intended that this state would be carried in the serializer, but of course if you must write the serialization function generically that didn't work, so we ended up with ast_encode.rs which does some manual serialization for any type that touches ty::t. At first this wasn't a lot of stuff but of course it's growing and it's annoying.

One way to solve this is to modify the Serializable trait as follows:

trait Serializable<S: Serializer> { fn serialize(&self, s: &s); }

This allows a type to implement Serializable only for specific serializers:

impl ty::t: Serializable<RustcSerializer> { ... }

To really make this work, though, it is also necessary for the auto_serialize code to permit generation of an impl that is specialized to a particular serializer, since any struct which contains ty::t values will also only work with RustcSerializer.

Another option would be to extend the Serializable trait with some kind of state and carry that around, but I think that winds up just being more type parameters without really adding any sort of flexibility.

@erickt has done preliminary work but it's hitting an ICE somewhere in the static methods implementation. I haven't tracked down the cause.

@nikomatsakis
Copy link
Contributor Author

This is a prereq to #3738, since the RustcSerializer could store a reference to the interner.

@nikomatsakis
Copy link
Contributor Author

Also a prereq to fixing #1972.

@pnkfelix
Copy link
Member

It is a little tricky to be sure given the alpha-renaming that has occurred since this bug was filed, but I think the design proposed by Niko in the description has been implemented.

The version of src/libstd/serialization.rs from 8 months ago had the old Serializable trait for stage0, and the Serializable<S: Serializer> trait for stages > 0.

The current version of src/libextra/serialize.rs has the latter renamed as Encodable<S:Encoder> trait. It is available in all stages.

So I think we should be able to close this.

I guess I'll want to double check whether the feature is actually being tested. And whether we can indeed fix #3738 and #1972 now. :)

@nikomatsakis
Copy link
Contributor Author

This is not fully implemented because the deriving mode does not allow you to specify the serializer.

@huonw
Copy link
Member

huonw commented Jun 19, 2013

@nikomatsakis I opened #7229 about that (it suggests a possible solution).

@emberian
Copy link
Member

emberian commented Aug 5, 2013

Visiting for triage; nothing to add.

@catamorphism
Copy link
Contributor

De-milestoning, just a bug

@alexcrichton
Copy link
Member

Closing, this appears to be the current design of the Encodable trait

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

No branches or pull requests

6 participants