libsyntax: Refactor deriving to reduce boilerplate required & derive Ord, TotalOrd and TotalEq #5640
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
With d19d353, there is no longer a quadratic slow down for the new Eq vs the old Eq implementation, but the conversion to using |
|
Not sure what your followup comment means. Do you want the most-recent change (d19d353) to land, or something else? |
|
(great work btw) |
|
Sorry, yes, d19d353 is the one to land. Before that change, the deriving Eq with this change was quadratic in the number of variants for an enum, rather than linear as with the old implementation. (I orphaned that comment by editing the part that talked about that out of the pull request, oops.) |
|
@graydon rebased onto incoming |
|
@graydon rebased again. |
…) interface. Pulls out many of the common patterns from the Eq and Clone deriving code (and invents a few of its own), so that deriving instances are very easy to write for a certain class of traits. (Basically, those which don't have parameters and where all methods only take arguments of type `&Self` and return either `Self` or types with no parameters.)
…ric deriving code. Closes rust-lang#4269, rust-lang#5588 and rust-lang#5589.
Allow a deriving instance using the generic code to short-circuit for any non-matching enum variants (grouping them all into a _ match), reducing the number of arms required. Use this to speed up the Eq & TotalEq implementations.
bors
added a commit
that referenced
this pull request
Apr 12, 2013
…inger This refactors much of the ast generation required for `deriving` instances into a common interface, so that new instances only need to specify what they do with the actual data, rather than worry about naming function arguments and extracting fields from structs and enum. (This all happens in `generic.rs`. I've tried to make sure it was well commented and explained, since it's a little abstract at points, but I'm sure it's still a little confusing.) It makes instances like the comparison traits and `Clone` short and easy to write. Caveats: - Not surprisingly, this slows the expansion pass (in some cases, dramatically, specifically deriving Ord or TotalOrd on enums with many variants). However, this shouldn't be too concerning, since in a more realistic case (compiling `core.rc`) the time increased by 0.01s, which isn't worth mentioning. And, it possibly slows type checking very slightly (about 2% worst case), but I'm having trouble measuring it (and I don't understand why this would happen). I think this could be resolved by using traits and encoding it all in the type system so that monomorphisation handles everything, but that would probably be a little tricky to arrange nicely, reduce flexibility and make compiling rustc take longer. (Maybe some judicious use of `#[inline(always)]` would help too; I'll have a bit of a play with it.) - The abstraction is not currently powerful enough for: - `IterBytes`: doesn't support arguments of type other than `&Self`. - `Encodable`/`Decodable` (#5090): doesn't support traits with parameters. - `Rand` & `FromStr`; doesn't support static functions and arguments of type other than `&Self`. - `ToStr`: I don't think it supports returning `~str` yet, but I haven't actually tried. (The last 3 are traits that might be nice to have: the derived `ToStr`/`FromStr` could just read/write the same format as `fmt!("%?", x)`, like `Show` and `Read` in Haskell.) I have ideas to resolve all of these, but I feel like it would essentially be a simpler version of the `mt` & `ty_` parts of `ast.rs`, and I'm not sure if the simplification is worth having 2 copies of similar code. Also, makes Ord, TotalOrd and TotalEq derivable (closes #4269, #5588 and #5589), although a snapshot is required before they can be used in the rust repo. If there is anything that is unclear (or incorrect) either here or in the code, I'd like to get it pointed out now, so I can explain/fix it while I'm still intimately familiar with the code.
This was referenced Apr 13, 2013
Closed
This was referenced May 1, 2013
bors
added a commit
that referenced
this pull request
May 8, 2013
…omatsakis This "finishes" the generic deriving code (which I started in #5640), in the sense it supports everything that I can think of being useful. (Including lifetimes and type parameters on methods and traits, arguments and return values of (almost) any type, static methods.) It closes #6149, but met with #6257, so the following doesn't work: ```rust #[deriving(TotalEq)] struct Foo<'self>(&'self int); ``` (It only fails for `TotalOrd`, `TotalEq` and `Clone`, since they are the only ones that call a method directly on sub-elements of the type, which means that the auto-deref interferes with the pointer.) It also makes `Rand` (chooses a random variant, fills the fields with random values, including recursively for recursive types) and `ToStr` (`x.to_str()` is the same as `fmt!("%?", x)`) derivable, as well as converting IterBytes to the generic code (which made the code 2.5x shorter, more robust and added support for tuple structs). ({En,De}codable are trickier, so I'll convert them over later.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This refactors much of the ast generation required for
derivinginstances into a common interface, so that new instances only need to specify what they do with the actual data, rather than worry about naming function arguments and extracting fields from structs and enum. (This all happens ingeneric.rs. I've tried to make sure it was well commented and explained, since it's a little abstract at points, but I'm sure it's still a little confusing.)It makes instances like the comparison traits and
Cloneshort and easy to write.Caveats:
Not surprisingly, this slows the expansion pass (in some cases, dramatically, specifically deriving Ord or TotalOrd on enums with many variants). However, this shouldn't be too concerning, since in a more realistic case (compiling
core.rc) the time increased by 0.01s, which isn't worth mentioning. And, it possibly slows type checking very slightly (about 2% worst case), but I'm having trouble measuring it (and I don't understand why this would happen). I think this could be resolved by using traits and encoding it all in the type system so that monomorphisation handles everything, but that would probably be a little tricky to arrange nicely, reduce flexibility and make compiling rustc take longer. (Maybe some judicious use of#[inline(always)]would help too; I'll have a bit of a play with it.)The abstraction is not currently powerful enough for:
IterBytes: doesn't support arguments of type other than&Self.Encodable/Decodable(Refactor deriving code between auto_encode.rs and deriving.rs #5090): doesn't support traits with parameters.Rand&FromStr; doesn't support static functions and arguments of type other than&Self.ToStr: I don't think it supports returning~stryet, but I haven't actually tried.(The last 3 are traits that might be nice to have: the derived
ToStr/FromStrcould just read/write the same format asfmt!("%?", x), likeShowandReadin Haskell.)I have ideas to resolve all of these, but I feel like it would essentially be a simpler version of the
mt&ty_parts ofast.rs, and I'm not sure if the simplification is worth having 2 copies of similar code.Also, makes Ord, TotalOrd and TotalEq derivable (closes #4269, #5588 and #5589), although a snapshot is required before they can be used in the rust repo.
If there is anything that is unclear (or incorrect) either here or in the code, I'd like to get it pointed out now, so I can explain/fix it while I'm still intimately familiar with the code.