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

map_entry is incorrectly suggested when map is used between contains_key and insert #6196

Closed
khyperia opened this issue Oct 19, 2020 · 3 comments
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

@khyperia
Copy link

Related issue: #5696

Reproducing code:

use std::collections::HashMap;

// don't want to call this if contains_key==true
fn really_expensive_function(_map: &HashMap<u32, u32>) {}

fn main() {
    let mut map = HashMap::new();
    let key = 5;
    if !map.contains_key(&key) {
        really_expensive_function(&map);
        map.insert(key, 10);
    }
    // This doesn't compile:
    /*
    use std::collections::hash_map::Entry;
    match map.entry(key) {
        Entry::Vacant(entry) => {
            really_expensive_function(&map);
            entry.insert(10);
        }
        _ => (),
    }
    */
}

produces

warning: usage of `contains_key` followed by `insert` on a `HashMap`
  --> src/main.rs:9:5
   |
9  | /     if !map.contains_key(&key) {
10 | |         really_expensive_function(&map);
11 | |         map.insert(key, 10);
12 | |     }
   | |_____^ consider using `map.entry(key)`

However, the commented-out code (which is what I believe clippy is suggesting) fails to compile with

error[E0502]: cannot borrow `map` as immutable because it is also borrowed as mutable
  --> src/main.rs:17:39
   |
15 |     match map.entry(key) {
   |           --- mutable borrow occurs here
16 |         Entry::Vacant(entry) => {
17 |             really_expensive_function(&map);
   |                                       ^^^^ immutable borrow occurs here
18 |             entry.insert(10);
   |             ----- mutable borrow later used here

I believe the correct behavior here is to not emit a suggestion at all.

Meta

  • cargo clippy -V: clippy 0.0.212 (18bf6b4 2020-10-07)
  • rustc -Vv:
    rustc 1.47.0 (18bf6b4f0 2020-10-07)
    binary: rustc
    commit-hash: 18bf6b4f01a6feaf7259ba7cdae58031af1b7b39
    commit-date: 2020-10-07
    host: x86_64-unknown-linux-gnu
    release: 1.47.0
    LLVM version: 11.0
    
@khyperia khyperia added the C-bug Category: Clippy is not doing the correct thing label Oct 19, 2020
@giraffate
Copy link
Contributor

@rustbot modify labels: +L-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 Oct 27, 2020
@giraffate
Copy link
Contributor

related issue: #4674

@khyperia
Copy link
Author

related issue: #4674

Looks like this (my issue) is a duplicate, feel free to close this if you'd like.

@llogiq llogiq closed this as completed Oct 28, 2020
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

No branches or pull requests

4 participants