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 deriving eq to asts #4922

Closed

Conversation

jbclements
Copy link
Contributor

r?

Apply deriving_eq to the data structures in ast.rs, and get rid of the custom definitions of eq that were everywhere. resulting ast.rs is about 400 lines shorter.

Also: add a few test cases and a bunch of comments.

Also: change ast_ty_to_ty_cache to use node ids rather than ast::ty's. I believe this was a suggestion related to my changes, and it appears to pass all tests.

Also: tiny doc fix, remove references to crate keywords.

Note that the replaced definition of equality on tokens
contains a *huge* shortcut on INTERPOLATED tokens (those
that contain ASTs), whereby any two INTERPOLATED tokens
are considered equal. This seems like a really broken
notion of equality, but it appears that the existing
test cases and the compiler don't depend on it. Niko
noticed this, BTW.

Replace long definition of Eq on tokens and binops
w
@catamorphism
Copy link
Contributor

Yay for deleting code!

bors added a commit that referenced this pull request Feb 14, 2013
…morphism

r?

Apply deriving_eq to the data structures in ast.rs, and get rid of the custom definitions of eq that were everywhere. resulting ast.rs is about 400 lines shorter.

Also: add a few test cases and a bunch of comments.

Also: change ast_ty_to_ty_cache to use node ids rather than ast::ty's. I believe this was a suggestion related to my changes, and it appears to pass all tests.

Also: tiny doc fix, remove references to crate keywords.
@bors bors closed this Feb 14, 2013
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.

3 participants