Skip to content

map_entry lint suggested by clippy gives compilation error when key is also used in else block #13306

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

Closed
prateekkumarweb opened this issue Aug 25, 2024 · 1 comment · Fixed by #14314
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@prateekkumarweb
Copy link

prateekkumarweb commented Aug 25, 2024

Summary

Clippy gives incorrect suggestion of entry api when using contains_key followed by insert. The suggestion given causes compilation error because of key being used by else block which has already got moved when entry method is called in the if condition.

Reproducer

When running clippy with the following input:

use std::collections::HashMap;

#[derive(Debug)]
struct Env {
    enclosing: Option<Box<Env>>,
    values: HashMap<String, usize>,
}

impl Env {
    fn new(enclosing: Option<Box<Env>>) -> Self {
        Self {
            enclosing,
            values: HashMap::new(),
        }
    }

    fn assign(&mut self, name: String, value: usize) -> bool {
        if self.values.contains_key(&name) {
            self.values.insert(name, value);
            true
        } else if let Some(enclosing) = &mut self.enclosing {
            enclosing.assign(name, value)
        } else {
            false
        }
    }
}

fn main() {
    let mut env = Env::new(None);
    env.assign("x".to_string(), 10);
    dbg!(&env);
}

gives:

warning: usage of `contains_key` followed by `insert` on a `HashMap`
  --> src/main.rs:18:9
   |
18 | /         if self.values.contains_key(&name) {
19 | |             self.values.insert(name, value);
20 | |             true
21 | |         } else if let Some(enclosing) = &mut self.enclosing {
...  |
24 | |             false
25 | |         }
   | |_________^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_entry
   = note: `#[warn(clippy::map_entry)]` on by default
help: try
   |
18 ~         if let std::collections::hash_map::Entry::Occupied(mut e) = self.values.entry(name) {
19 +             e.insert(value);
20 +             true
21 +         } else if let Some(enclosing) = &mut self.enclosing {
22 +             enclosing.assign(name, value)
23 +         } else {
24 +             false
25 +         }
   |

Accepting this suggestion using --fix gives error.

after fixes were automatically applied the compiler reported errors within these files:

  * src/main.rs

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust-clippy/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

The following errors were reported:
error[E0382]: use of moved value: `name`
  --> src/main.rs:22:30
   |
17 |     fn assign(&mut self, name: String, value: usize) -> bool {
   |                          ---- move occurs because `name` has type `std::string::String`, which does not implement the `Copy` trait
18 |         if let std::collections::hash_map::Entry::Occupied(mut e) = self.values.entry(name) {
   |                                                                                       ---- value moved here
...
22 |             enclosing.assign(name, value)
   |                              ^^^^ value used here after move

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0382`.
Original diagnostics will follow.

Version

rustc 1.80.1 (3f5fd8dd4 2024-08-06)
binary: rustc
commit-hash: 3f5fd8dd41153bc5fdca9427e9e05be2c767ba23
commit-date: 2024-08-06
host: aarch64-apple-darwin
release: 1.80.1
LLVM version: 18.1.7

Additional Labels

No response

@prateekkumarweb prateekkumarweb added the C-bug Category: Clippy is not doing the correct thing label Aug 25, 2024
@alex-semenyuk
Copy link
Member

@rustbot label I-suggestion-causes-error

@rustbot rustbot added the I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied label Aug 28, 2024
github-merge-queue bot pushed a commit that referenced this issue Mar 2, 2025
Closes #13306
Closes #9925
Closes #9470
Closes #9305

Clippy gives wrong suggestions when the key is not `Copy`. As suggested
in #9925, in such cases Clippy will simply warn but no fix.

changelog: [`map_entry`]: fix wrong suggestions when key is not `Copy`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants