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

Improve diagnostic for missing IndexMut: HashMap #100873

Closed
estebank opened this issue Aug 22, 2022 · 4 comments · Fixed by #100906
Closed

Improve diagnostic for missing IndexMut: HashMap #100873

estebank opened this issue Aug 22, 2022 · 4 comments · Fixed by #100906
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@estebank
Copy link
Contributor

estebank commented Aug 22, 2022

The following is invalid

let mut unique_things: HashMap<usize, bool>  = HashMap::new();
unique_things[&5] = false;
error[E0594]: cannot assign to data in an index of `HashMap<usize, bool>`
   --> src/main.rs:147:2
    |
147 |     unique_things[&5] = false;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^ cannot assign
    |
    = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<usize, bool>`

Because the entry &5 must exist before it can be assigned to. We should propose the use of the Entry APIs or .insert method.

https://users.rust-lang.org/t/problematic-errormessage-indexmut-is-required-to-modify-indexed-content/80158

@estebank estebank added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. labels Aug 22, 2022
@ChayimFriedman2
Copy link
Contributor

@rustbot claim

@ChayimFriedman2
Copy link
Contributor

The code responsible for generating this error is in compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs, but I don't feel like polluting it with detection of HashMap (and BTreeMap). Do you have a better idea?

@estebank
Copy link
Contributor Author

estebank commented Aug 22, 2022

It is not that uncommon for us to have weird one-off detection of types, although we do try to avoid it. Given that this is about HashMap and something a newcomer might reasonably attempt, I feel it is reasonable to do so. You should move the logic to its own method to avoid cluttering the happy path, though.

An option would be to check if the involved type has an .insert method that takes the relevant types, and try to rely on that convention so it will work with third party types as well, but even then I'd still want to tell people about Entry so you'd need to check for HashMap anyways.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 24, 2022
…avidtwco

Suggest alternatives when trying to mutate a `HashMap`/`BTreeMap` via indexing

The error can be quite confusing to newcomers.

Fixes rust-lang#100873.

I'm not so sure about the message, open to wording suggestions.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 24, 2022
…avidtwco

Suggest alternatives when trying to mutate a `HashMap`/`BTreeMap` via indexing

The error can be quite confusing to newcomers.

Fixes rust-lang#100873.

I'm not so sure about the message, open to wording suggestions.
@estebank
Copy link
Contributor Author

estebank commented Aug 24, 2022

CC #52788, #45491

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants