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

Implement Hash for TreeSet #15633

Merged
merged 1 commit into from
Jul 13, 2014
Merged

Implement Hash for TreeSet #15633

merged 1 commit into from
Jul 13, 2014

Conversation

nham
Copy link
Contributor

@nham nham commented Jul 12, 2014

cc #15294

@sfackler
Copy link
Member

It seems like we should be able to derive Hash instead.

@nham
Copy link
Contributor Author

nham commented Jul 12, 2014

I'm not really sure how #[deriving] works, but I'm guessing that would require that TreeMap implement Hash as well? (And the TreeNode structure that TreeMap uses as well). I'll try it.

@alexcrichton
Copy link
Member

This may actually have problems with #[deriving(Hash)] because you can have two equivalent trees with different structures (but they should have the same hash).

@sfackler
Copy link
Member

TreeMap would have to have a manual implementation, but TreeSet should be able to derive it since it'll defer to TreeMap's implementation.

@alexcrichton
Copy link
Member

Aha, indeed!

@nham
Copy link
Contributor Author

nham commented Jul 13, 2014

I'm having some difficulty understanding how to do this. Is there a way to manually implement Hash for TreeMap without using iter()? Because iter() is only implemented for keys that implement Ord, which means that Hash is only implemented for TreeMap's where the key implements Ord. This yields the following error when trying to derive Hash for TreeSet:

/home/travis/build/rust-lang/rust/src/libcollections/treemap.rs:592:5: 592:24 error: failed to find an implementation of trait core::cmp::Ord for T

/home/travis/build/rust-lang/rust/src/libcollections/treemap.rs:592 map: TreeMap<T, ()>

@sfackler
Copy link
Member

Ah, yeah, you may have to manually implement Hash for both TreeMap and TreeSet then.

@alexcrichton
Copy link
Member

Looks good to me! Could you squash the commits as well?

@nham
Copy link
Contributor Author

nham commented Jul 13, 2014

Done.

bors added a commit that referenced this pull request Jul 13, 2014
@bors bors closed this Jul 13, 2014
@bors bors merged commit a54dc54 into rust-lang:master Jul 13, 2014
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 13, 2023
scip: Allow customizing cargo config.

Re-use the LSP config json for simplicity.
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