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

multiple mutable borrow error with get_mut #75112

Closed
95th opened this issue Aug 3, 2020 · 3 comments
Closed

multiple mutable borrow error with get_mut #75112

95th opened this issue Aug 3, 2020 · 3 comments
Labels
A-borrow-checker Area: The borrow checker C-bug Category: This is a bug. fixed-by-polonius Compiling with `-Zpolonius` fixes this issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@95th
Copy link
Contributor

95th commented Aug 3, 2020

I tried this code:

use std::collections::HashMap;

pub fn find<'a>(lookup: &HashMap<u32, usize>, items: &'a mut Vec<u32>, id: u32) -> Option<&'a mut u32> {
    if let Some(token) = lookup.get(&id) {
        return items.get_mut(*token);
    }

    if let Some(token) = lookup.get(&id.wrapping_add(1)) {
        let item = items.get_mut(*token)?;
        if *item == id {
            return Some(item);
        }
    }

    if let Some(token) = lookup.get(&(id.wrapping_sub(1))) {
        let item = items.get_mut(*token)?;
        if *item == id {
            return Some(item);
        }
    }

    None
}

I expected to see this happen: Code should compile because the latter get_mut calls don't overlap with preceding ones.

Instead, this happened: Got following error

error[E0499]: cannot borrow `*items` as mutable more than once at a time
  --> src/lib.rs:16:20
   |
3  | pub fn find<'a>(lookup: &HashMap<u32, usize>, items: &'a mut Vec<u32>, id: u32) -> Option<&'a mut u32> {
   |             -- lifetime `'a` defined here
...
9  |         let item = items.get_mut(*token)?;
   |                    ----- first mutable borrow occurs here
10 |         if *item == id {
11 |             return Some(item);
   |                    ---------- returning this value requires that `*items` is borrowed for `'a`
...
16 |         let item = items.get_mut(*token)?;
   |                    ^^^^^ second mutable borrow occurs here

It works if I comment the if condition inside the second if let:

use std::collections::HashMap;

pub fn find<'a>(lookup: &HashMap<u32, usize>, items: &'a mut Vec<u32>, id: u32) -> Option<&'a mut u32> {
    if let Some(token) = lookup.get(&id) {
        return items.get_mut(*token);
    }

    if let Some(token) = lookup.get(&id.wrapping_add(1)) {
        let item = items.get_mut(*token)?;
        // if *item == id {
            return Some(item);
        // }
    }

    if let Some(token) = lookup.get(&(id.wrapping_sub(1))) {
        let item = items.get_mut(*token)?;
        if *item == id {
            return Some(item);
        }
    }

    None
}

if I comment the last if let, then also it works:

use std::collections::HashMap;

pub fn find<'a>(lookup: &HashMap<u32, usize>, items: &'a mut Vec<u32>, id: u32) -> Option<&'a mut u32> {
    if let Some(token) = lookup.get(&id) {
        return items.get_mut(*token);
    }

    if let Some(token) = lookup.get(&id.wrapping_add(1)) {
        let item = items.get_mut(*token)?;
        if *item == id {
            return Some(item);
        }
    }

    // if let Some(token) = lookup.get(&(id.wrapping_sub(1))) {
    //     let item = items.get_mut(*token)?;
    //     if *item == id {
    //         return Some(item);
    //     }
    // }

    None
}

So there are still multiple mutable borrows but it works for those cases. But why not in the first case? Is it incorrect borrowing pattern somehow that I am not seeing?

Meta

rustc --version --verbose:

rustc 1.46.0-nightly (346aec9b0 2020-07-11)
binary: rustc
commit-hash: 346aec9b02f3c74f3fce97fd6bda24709d220e49
commit-date: 2020-07-11
host: x86_64-unknown-linux-gnu
release: 1.46.0-nightly
LLVM version: 10.0

Thanks

@95th 95th added the C-bug Category: This is a bug. label Aug 3, 2020
@nbdd0121
Copy link
Contributor

nbdd0121 commented Aug 3, 2020

use std::collections::HashMap;

pub fn find<'a>(lookup: &HashMap<u32, usize>, items: &'a mut Vec<u32>, id: u32) -> Option<&'a mut u32> {
    if let Some(token) = lookup.get(&id) {
        return items.get_mut(*token);
    }

    if let Some(token) = lookup.get(&id.wrapping_add(1)) {
        let item = items.get_mut(*token)?; // <- 2. Therefore this lifetime of this borrow of `items` is exactly 'a, not a shorter, temporary lifetime.
        if *item == id {
            return Some(item); // <- 1. This line requires item: 'a
        }
    }

    if let Some(token) = lookup.get(&(id.wrapping_sub(1))) {
        let item = items.get_mut(*token)?; // <- 3. Because 2 already requires the lifetime to be borrowed for entirety of 'a, this is no longer allowed.
        if *item == id {
            return Some(item);
        }
    }

    None
}

@fmease
Copy link
Member

fmease commented Sep 7, 2023

Fixed by Polonius (-Zpolonius).

@fmease fmease added A-borrow-checker Area: The borrow checker T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage-legacy labels Sep 7, 2023
@fmease fmease added the fixed-by-polonius Compiling with `-Zpolonius` fixes this issue. label Jan 25, 2024
@fmease
Copy link
Member

fmease commented Jan 26, 2024

Closing as duplicate of #54663. If you disagree with my assessment, I can of course reopen this issue.

@fmease fmease closed this as not planned Won't fix, can't repro, duplicate, stale Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-borrow-checker Area: The borrow checker C-bug Category: This is a bug. fixed-by-polonius Compiling with `-Zpolonius` fixes this issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants