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

Cannot use or_insert_with for HashMap::<usize, HashMap<usize, i64>> - is this supposed to be supported? #595

Open
FelixZY opened this issue Dec 6, 2024 · 4 comments

Comments

@FelixZY
Copy link

FelixZY commented Dec 6, 2024

I have a program like this:

use hashbrown::HashMap;

fn main() {
    let mut value_by_col_by_row = HashMap::<usize, HashMap<usize, i64>>::new();
    value_by_col_by_row.entry_ref(&0).or_insert_with(HashMap::new).insert(0, 1);
}

Unfortunately, cargo build produces the following error:

   Compiling rust v0.1.0 (/my/project/path)
error[E0277]: the trait bound `usize: From<&usize>` is not satisfied
    --> src/main.rs:5:39
     |
5    |     value_by_col_by_row.entry_ref(&0).or_insert_with(HashMap::new).insert(0, 1);
     |                                       ^^^^^^^^^^^^^^ the trait `From<&usize>` is not implemented for `usize`
     |
note: required by a bound in `EntryRef::<'a, 'b, K, Q, V, S, A>::or_insert_with`
    --> /my/home/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hashbrown-0.15.2/src/map.rs:4197:19
     |
4195 |     pub fn or_insert_with<F: FnOnce() -> V>(self, default: F) -> &'a mut V
     |            -------------- required by a bound in this associated function
4196 |     where
4197 |         K: Hash + From<&'b Q>,
     |                   ^^^^^^^^^^^ required by this bound in `EntryRef::<'a, 'b, K, Q, V, S, A>::or_insert_with`
help: consider borrowing here
     |
5    |     (&value_by_col_by_row.entry_ref(&0)).or_insert_with(HashMap::new).insert(0, 1);
     |     ++                                 +

For more information about this error, try `rustc --explain E0277`.
error: could not compile `rust` (bin "rust") due to 1 previous error

I'm not sure if I'm doing something wrong or if this is not supposed to be a supported use case?

$ cargo --version
cargo 1.83.0 (5ffbef321 2024-10-29)
$ rustc --version
rustc 1.83.0 (90b35a623 2024-11-26)

#####

[dependencies]
hashbrown = "0.15.2"
@clarfonthey
Copy link
Contributor

So, the EntryRef API is very weird, and the fact that it requires K: From<&Q> is very weird, but this is intended for cases like &str, where you're converting a borrowed objecting into an owned copy.

For integer keys, you should be using entry instead, and passing the integer key directly, since it can be copied. The or_insert_with method on entry(0) should work, even though entry_ref(&0) won't.

@FelixZY
Copy link
Author

FelixZY commented Dec 8, 2024

Alright, thank you @clarfonthey ! I'm fine with closing this unless anyone wants to e.g. add an extra snippet of documentation somewhere or further discuss the "weirdness" of the EntryRef API.

@clarfonthey
Copy link
Contributor

Up to you— I think that documentation can always be improved, although I also was able to figure this out mostly by just checking the docs for EntryRef::or_insert_with to see the weird From bound more explicitly.

@FelixZY
Copy link
Author

FelixZY commented Dec 9, 2024

I guess I would have liked to see a reference to entry() in the entry_ref() docs (and vice versa) with a short "use X instead if ...".

To me, it's not very clear what the difference between these two are, going only by the description text:

entry

Gets the given key’s corresponding entry in the map for in-place manipulation.

entry_ref

Gets the given key’s corresponding entry by reference in the map for in-place manipulation.

In what way isn't entry providing a reference? I mean, the docs clearly state that it operates in-place! (Question I would have liked the docs to make more clear).

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

No branches or pull requests

2 participants