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

Refactor deriving(Encodable, Decodable) and various bugfixes #6851

Closed
wants to merge 4 commits into from

Conversation

alexcrichton
Copy link
Member

Closes #5090 by using the excellent new generic deriving code

Promotes the unreachable code attribute to a lint attribute (instead of always being a warning)

Fixes some edge cases when creating hashmaps/hashsets and also when consuming them. (fixes #5998)

// Create the arms of the match in the method body.
let mut arms = do enum_definition.variants.mapi |i, variant| {
create_read_variant_arg(cx, span, i, variant)
_ => fail!("expected StaticEnum or StaticStruct")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could probably be cx.bug("expected StaticEnum or StaticStruct"). (And similarly in encodable.rs.)

@huonw
Copy link
Member

huonw commented May 31, 2013

Yay, thanks for rewriting {En,De}codable, it's wonderful how much shorter it is! (It's good to see that the generic deriving code makes sense to someone other than its author. :) )

@alexcrichton
Copy link
Member Author

Just updated, and you did a great job with the refactorings! It took me a bit to get used to, but I thought it was all really well done.

Now it uses the generic deriving code and should in theory work in all cases.
@huonw
Copy link
Member

huonw commented May 31, 2013

Thanks. :)

I think with this last rewrite, most of the stuff in deriving/mod.rs can be either removed or migrated to generic.rs, since a lot is just legacy stuff that is now called exactly once and can be inlined into the appropriate function. (I'm happy to do so after this patch lands.)

// We're not generating an AST that the borrow checker is expecting,
// so we need to generate a unique local variable to take the
// mutable loan out on, otherwise we get conflicts which don't
// actually exist.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unfortunate :(

bors added a commit that referenced this pull request Jun 1, 2013
Closes #5090 by using the excellent new generic deriving code

Promotes the unreachable code attribute to a lint attribute (instead of always being a warning)

Fixes some edge cases when creating hashmaps/hashsets and also when consuming them. (fixes #5998)
@bors bors closed this Jun 1, 2013
@huonw huonw mentioned this pull request Jun 7, 2013
bors added a commit that referenced this pull request Jun 7, 2013
Several minor changes:
 - The clean-up I mentioned in #6851 (moving functions from deriving/mod.rs to deriving/generic.rs)
 - Move `expand_generic_deriving` to a method
 - Reimplement `deriving(Ord)` with no dependence on `Eq`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants