Skip to content
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 traits w/ auto-deriving for soundly serializing/inspecting/transforming rustc types. #36588

Open
eddyb opened this issue Sep 19, 2016 · 2 comments
Labels
A-metadata Area: Crate metadata C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@eddyb
Copy link
Member

eddyb commented Sep 19, 2016

Currently, we're limited to the general-purpose Eq, Ord, Hash, Encodable and Decodable traits, even though the compiler often has more unconventional desires, involving contexts and custom behaviors.

There are several special-purpose traits already in the compiler, such as AST/HIR Folder/Visitor and the two-in-one TypeFoldable (which also does visiting).

Also, #36551 adds a specialization mechanism in the bundled rustc_serialize to work around the fact that Encodable and Decodable have their methods be generic over the encoder/decoder instead of the trait, but this abuses the yet-unclosed lifetime soundness hole in specialization (#31844):

impl<'a, 'tcx> SpecializedDecoder<Ty<'tcx>> for DecodeContext<'a, 'tcx> {...}

That impl requires that the 'tcx of the decoded Ty match the 'tcx of the decoder, but this cannot be enforced through specialization as the lifetime relationships do not exist before monomorphization.

A better solution would involve custom traits that can dispatch over the decoder w/o specialization, i.e.:

impl<'a, 'tcx> Decodable<DecodeContext<'a, 'tcx>> for Ty<'tcx> {...}

To make all of this ergonomic, auto-deriving is necessary, and I believe macros 1.1 (perhaps helped by a permanent switch to rustbuild) can make it happen.

Last but not least, it may be possible to generalize encoding/decoding and visiting/folding/relating into a single (or a pair of) abstraction(s), but I'd have to leave that to someone who understands tree folding, transducers, and perhaps Haskell lenses, although there's no guarantee they map nicely to Rust.

cc @rust-lang/compiler

@eddyb eddyb added C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-compiler labels Sep 19, 2016
@nikomatsakis
Copy link
Contributor

Nominating for discussion next week. In this week's compiler team meeting, @arielb1 raised concerns about leaving (ab)uses of unsound specialization in the compiler, which I am sympathetic to. It'd be good to figure out how we can prioritize moving away from that.

@arielb1 arielb1 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 24, 2016
@eddyb
Copy link
Member Author

eddyb commented Sep 3, 2017

I believe we can reduce our deriving needs to a single "ShapeReflect" trait with 4 operations:

  • Self -> (fields...)
  • &Self -> (&fields...)
  • &mut Self -> (&mut fields...)
  • (fields...) -> Self

Even without VG we can build implementations of all sorts of operations we need (such as Relate which does (&T, &T) -> Result<T, E>) out of this one trait + specialization.

EDITs:

bors added a commit that referenced this issue Nov 28, 2018
[WIP] serialize: rename to rustc_serialize.

In preparation for post-#49219 use of custom derives in rustc, we'll want to stop using `Rustc{Encodable,Decodable}` derives, and *at least* adapt the library to suit `rustc` better, before a full implementation of #36588 can be done.

This will break anyone using `extern crate serialize;` (on nightly) so let's do a crater run with a rename, just to see what things are like.

**EDIT**: I was going to test with `rustc_serialize` (which does work in terms of compiling) but I realized that `rustc-serialize` from crates.io might make that tricky if it ends up in our sysroot, so for the crater run let's go with something weirder (`rustc_ezilaires`).

**EDIT2**: had to remove `rustc_save_analysis` and `rls` to get rid of the crates.io `rustc-serialize` from Cargo.lock. Note that this is just for crater, just like the `rustc_ezilaires` rename.

r? @nikomatsakis
@jonas-schievink jonas-schievink added the A-metadata Area: Crate metadata label May 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-metadata Area: Crate metadata C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants