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 HashMap.entry_or_clone() method #1203

Open
dnspies opened this issue Jul 11, 2015 · 8 comments
Open

Add HashMap.entry_or_clone() method #1203

dnspies opened this issue Jul 11, 2015 · 8 comments
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.

Comments

@dnspies
Copy link

dnspies commented Jul 11, 2015

Sometimes I have a key type which implements Clone, but for efficiency reasons I don't want to clone it unless it's absolutely necessary.

In this case it would be nice to have a method

my_hash_map.entry_or_clone(&k)

which behaves identically to

my_hash_map.entry(k.clone())

but only bothers to clone k if the key doesn't already exist in my_hash_map.

@Gankra
Copy link
Contributor

Gankra commented Jul 11, 2015

See also https://internals.rust-lang.org/t/head-desking-on-entry-api-4-0/2156

This is possibly a viable solution in that it's back-compat to add a new method that requires a Clone bound.

@Gankra Gankra self-assigned this Jul 11, 2015
@Gankra Gankra added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Jul 11, 2015
@futile
Copy link

futile commented Jul 20, 2015

Just hit this as well. The entry_or_clone idea sounds good. One thing to consider would probably be that it might end up cluttering the API if a method with a clone bound is found soon after adding this.

Asides from that, the name entry_or_clone makes clear what's happening, and should not be too confusing(until the API is possibly expanded later on).

@gereeter gereeter mentioned this issue Mar 6, 2016
@gereeter
Copy link

gereeter commented Mar 6, 2016

I just posted another alternative to this in #1533.

@orlp
Copy link
Contributor

orlp commented Dec 24, 2018

Just passing by to give a +1, I also just ran into this and am annoyed.

@orlp orlp unassigned Gankra Dec 24, 2018
@orlp
Copy link
Contributor

orlp commented Dec 26, 2018

Eh... is that a Github bug? It says I unassigned Gankro, but I did no such thing nor do I have permissions to do so.

@Timmmm
Copy link

Timmmm commented Nov 11, 2020

Given that it has been 5 years and apparently nobody has found a more elegant solution perhaps it is time to accept that entry_or_clone() is the best option?

I'm really surprised this hasn't been given more attention given that it is a pretty basic flaw and Rust is very focused on performance.

@thomcc
Copy link
Member

thomcc commented Nov 12, 2020

On nightly rust (or stable hashbrown) you can use the raw_entry API which provides this and more.

It needs another design pass though before it can be stabilized. It provides useful functionality, but the very clean initial design became a little mangled when it ran into reality.

You can do what you want as

some_map
    .raw_entry_mut()
    .from_key(&key)
    .or_insert_with(|| (key.clone(), 42));

Note that unlike entry_or_clone this also works for cases like looking up a &str but inserting String (e.g. ToOwned not Clone, or anything else). That said, a buggy impl can corrupt your map IIUC, although not in a memory unsafe way (and no more than a buggy Hash/Eq impl combo, I think).

@dekellum
Copy link

HashMap::raw_entry tracking issue is rust-lang/rust#56167 (for those following along).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests

8 participants