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

Inconsistent rejection of early return of borrow in loops #124254

Closed
jamesmunns opened this issue Apr 22, 2024 · 2 comments
Closed

Inconsistent rejection of early return of borrow in loops #124254

jamesmunns opened this issue Apr 22, 2024 · 2 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-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@jamesmunns
Copy link
Member

I tried this code:

fn find_lowest_or_first_empty_pos(list: &mut [Option<u8>]) -> &mut Option<u8> {
    let mut low_pos_val: Option<(usize, u8)> = None;
    for (idx, i) in list.iter_mut().enumerate() {
        // Can't do this!
        //
        // > cannot borrow `self.items[_]` as mutable more than once at a time
        // > returning this value requires that `self.items` is borrowed for `'1`
        let Some(s) = i else {
            return i;
        };
        
        // Can do this, lol
        // let Some(s) = i else {
        //     return &mut list[idx];
        // };
    
        // Or this:
        // let Some(s) = i else {
        //     low_pos_val = Some((idx, 0));
        //     break;
        // };
        low_pos_val = match low_pos_val {
            Some((_oidx, oval)) if oval > *s => Some((idx, *s)),
            Some(old) => Some(old),
            None => Some((idx, *s)),
        };
    }
    let Some((lowest_idx, _)) = low_pos_val else {
        unreachable!("Can't have zero length list!");
    };
    &mut list[lowest_idx]
}

fn main() {
    let mut list = [Some(1), Some(2), None, Some(3)];
    let v = find_lowest_or_first_empty_pos(&mut list);
    assert!(v.is_none());
    assert_eq!(v as *mut _ as usize, list.as_ptr().wrapping_add(2) as usize);
    
    let mut list = [Some(1), Some(2), Some(3), Some(0)];
    let v = find_lowest_or_first_empty_pos(&mut list);
    assert_eq!(v, &mut Some(0));
    assert_eq!(v as *mut _ as usize, list.as_ptr().wrapping_add(3) as usize);
    
    println!("pass");
}

I expected to see this happen: Code works, prints "pass"

Instead, this happened: Compiler error:

error[E0499]: cannot borrow `list[_]` as mutable more than once at a time
  --> src/main.rs:31:5
   |
1  | fn find_lowest_or_first_empty_pos(list: &mut [Option<u8>]) -> &mut Option<u8> {
   |                                         - let's call the lifetime of this reference `'1`
2  |     let mut low_pos_val: Option<(usize, u8)> = None;
3  |     for (idx, i) in list.iter_mut().enumerate() {
   |                     ---- first mutable borrow occurs here
...
9  |             return i;
   |                    - returning this value requires that `*list` is borrowed for `'1`
...
31 |     &mut list[lowest_idx]
   |     ^^^^^^^^^^^^^^^^^^^^^ second mutable borrow occurs here

error[E0503]: cannot use `*list` because it was mutably borrowed
  --> src/main.rs:31:10
   |
1  | fn find_lowest_or_first_empty_pos(list: &mut [Option<u8>]) -> &mut Option<u8> {
   |                                         - let's call the lifetime of this reference `'1`
2  |     let mut low_pos_val: Option<(usize, u8)> = None;
3  |     for (idx, i) in list.iter_mut().enumerate() {
   |                     ---- `*list` is borrowed here
...
9  |             return i;
   |                    - returning this value requires that `*list` is borrowed for `'1`
...
31 |     &mut list[lowest_idx]
   |          ^^^^^^^^^^^^^^^^ use of borrowed `*list`

Some errors have detailed explanations: E0499, E0503.
For more information about an error, try `rustc --explain E0499`.
error: could not compile `playground` (bin "playground") due to 2 previous errors

In general, it feels like the borrow checker SHOULD be able to tell that the return is divergent and allow the return of i which comes from the iter_mut borrow. The compiler CAN tell that the branch is divergent, as it allows the use of IndexMut to invalidate the old borrow (the first commented solution) for early return.

This might be a known limitation of the current borrow checker, however it surprised me and the diagnostics were not particularly helpful, especially as the first commented option DOES work.

Meta

Repros on 1.77.2 stable, as well as 2024-04-21 nightly

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01c4d55ee75c67771912532d87a69a6d

@jamesmunns jamesmunns added the C-bug Category: This is a bug. label Apr 22, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 22, 2024
@fmease fmease added A-borrow-checker Area: The borrow checker T-types Relevant to the types team, which will review and decide on the PR/issue. fixed-by-polonius Compiling with `-Zpolonius` fixes this issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Apr 22, 2024
@fmease
Copy link
Member

fmease commented Apr 22, 2024

Likely a dupe of one of the many fixed-by-polonius Compiling with `-Zpolonius` fixes this issue. issues.

@jamesmunns
Copy link
Member Author

For what its worth, I tried it with -Zpolonius, and it worked.

RUSTFLAGS=-Zpolonius cargo +nightly run
   Compiling polo v0.1.0 (/private/tmp/polo)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.15s
     Running `target/debug/polo`
pass

Going to go ahead and close, but feel free to re-open if y'all think its worth tracking.

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-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants