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 find_equiv_mut to HashMap #15721

Closed
wants to merge 1 commit into from
Closed

Add find_equiv_mut to HashMap #15721

wants to merge 1 commit into from

Conversation

aochagavia
Copy link
Contributor

No description provided.

@alexcrichton
Copy link
Member

This has been proposed many times before, see: #13205, #11074, #11691, #12135, #14968, #15220, and #15389.

I would like to continue to push back on the basis that we don't have a clear design moving forward to deal with this case in general other than add a duplicate copy of all methods.

@pcwalton
Copy link
Contributor

Just FYI so it doesn't seem like the team is stonewalling this: I think the right thing to do is to nuke the Equiv trait in favor of a find that provides a hash value (or a value implementing Hash) and a closure to compare equality. That's the simplest possible solution that doesn't result in an explosion of trait complexity.

@aturon
Copy link
Member

aturon commented Jul 17, 2014

@pcwalton To me, the problem isn't so much trait complexity (the Equiv trait is pretty simple), but rather that this technique doubles the surface area of a given API.

In some sense, methods like find_equiv are generalizations of find, but the current trait system makes it so that you cannot usefully do impl<T> Equiv<T> for T.

If @nikomatsakis's where proposal lands, I wonder if we could provide such an impl using a multi-parameter version of Equiv. That way, we could give only methods that use the Equiv approach, but they would work even when you're using the key type itself, eliminating the API redundancy.

@aturon
Copy link
Member

aturon commented Jul 17, 2014

To be clear, the signature would look something like:

fn find<'a, Q>(&'a self, k: &Q) -> Option<&'a V>
    where Q: Hash<S>, () : Equiv<K, Q>

using the trick of encoding multiparameter typeclasses via impls on the unit type.

We'd also have impl<T> Equiv<T, T> for (), so that this find is usable in the same way the current find is -- thus merging the find and find_equiv functionality.

@pcwalton
Copy link
Contributor

I worry that that solution makes the type signature of find stray too far from the "obvious" type signature. The solution of duplicating APIs adds more surface area, but it's simple and easy to understand.

@aochagavia
Copy link
Contributor Author

Thanks for commenting! I understand the problem with this and hope that it can be solved in the future.

@aochagavia aochagavia closed this Jul 17, 2014
@aochagavia aochagavia deleted the hashmap branch October 13, 2014 18:16
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 13, 2023
Shrink PatPtr by swapping its AstPtr and Either wrap order

Will have neglible perf results I imagine, but it cleans up some code
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