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 some missing traits for SmallIntMap and TrieMap/TrieSet. #15963

Closed
wants to merge 3 commits into from

Conversation

nham
Copy link
Contributor

@nham nham commented Jul 24, 2014

Implements PartialEq/Eq/Clone/Hash/FromIterator/Extendable for SmallIntMap and Clone/Show for TrieMap/TrieSet. cc #15294

@nham
Copy link
Contributor Author

nham commented Jul 25, 2014

@alexcrichton I'm assuming you wanted me to squash that.

@alexcrichton
Copy link
Member

Sure!

@nham
Copy link
Contributor Author

nham commented Jul 25, 2014

I'm not sure I understand the error here.

error: type `vec::Vec<core::option::Option<T>>` does not implement any method in scope named `hash`

note: in expansion of #[deriving(Hash)]

Any idea?

@alexcrichton
Copy link
Member

Inside of the libcollections tests you'll have to make sure you're using the right version of traits and vectors and such. In that example it looks like you're using the libcollections Vec but the already-compiled libcore's Option while attempting to use someone's version of Hash, but the two don't match up.

In general you can just fiddle with imports until it compiles.

@nham
Copy link
Contributor Author

nham commented Jul 26, 2014

I don't think I really changed anything, but it passes Travis now so maybe it works?

@nham
Copy link
Contributor Author

nham commented Jul 26, 2014

I'm missing something important here. I can see that Hash is implemented for Option in libcollections::hash, but apparently smallintmap can't see that implementation? How do you import an implementation from another module?

@nham
Copy link
Contributor Author

nham commented Jul 26, 2014

I've decided to remove the Hash implementation for SmallIntMap and either try to add it later or let someone who knows more try to implement it. This now passes all tests on my machine.

@alexcrichton
Copy link
Member

The problem is that when you're testing libcollections there are two versions of libcollections, one with libstd and one that you're testing. This means that there are two versions of the Hash trait in play. What you're seeing is that you're using the tested Vec, but the libstd Option. Note that the libstd Hash trait is implemented for Option, but not for the tested Vec.

The fix would be to implement the tested Hash for the libstd Option, or some similar munging of the names or fiddling of imports to make sure you're importing the right thing. Another alternative would not be to test the tested Vec, but rather test the libstd Vec.

@nham
Copy link
Contributor Author

nham commented Jul 26, 2014

@alexcrichton Thanks for the explanation! This seems to work if I remove the #[deriving(Hash)] on SmallIntMap and just manually implement it. I've gone ahead and done that.

bors added a commit that referenced this pull request Jul 27, 2014
Implements PartialEq/Eq/Clone/Hash/FromIterator/Extendable for SmallIntMap and Clone/Show for TrieMap/TrieSet. cc #15294
@bors bors closed this Jul 27, 2014
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