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 borrow check in async function #74068

Open
Nugine opened this issue Jul 5, 2020 · 8 comments
Open

Inconsistent borrow check in async function #74068

Nugine opened this issue Jul 5, 2020 · 8 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. NLL-polonius Issues related for using Polonius in the borrow checker P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Nugine
Copy link
Contributor

Nugine commented Jul 5, 2020

I tried this code:

use std::collections::HashMap;

fn f1(map: &mut HashMap<i32, String>, k: i32) -> &String {
    if let Some(s) = map.get(&k) {
        return s;
    }
    map.insert(k, k.to_string());
    map.get(&k).unwrap()
}

async fn f2(map: &mut HashMap<i32, String>, k: i32) -> &String {
    if let Some(s) = map.get(&k) {
        return s;
    }
    map.insert(k, k.to_string());
    map.get(&k).unwrap()
}

f2 seems to be correct but can not be compiled.

   Compiling playground v0.0.1 (/playground)
error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable
  --> src/lib.rs:15:5
   |
12 |     if let Some(s) = map.get(&k) {
   |                      --- immutable borrow occurs here
13 |         return s;
   |                - returning this value requires that `*map` is borrowed for `'1`
14 |     }
15 |     map.insert(k, k.to_string());
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
16 |     map.get(&k).unwrap()
17 | }
   | - return type of generator is &'1 std::string::String

error: aborting due to previous error

For more information about this error, try `rustc --explain E0502`.
error: could not compile `playground`.

To learn more, run the command again with --verbose.

Meta

According to the playground, the problem exists on 1.44.1 and 1.46.0-nightly (2020-07-04 0cd7ff7ddfb75a38dca8).

Other versions may be the same.

@Nugine Nugine added the C-bug Category: This is a bug. label Jul 5, 2020
@jonas-schievink jonas-schievink added A-async-await Area: Async & Await A-borrow-checker Area: The borrow checker T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 5, 2020
@SNCPlay42
Copy link
Contributor

SNCPlay42 commented Jul 5, 2020

If f2 is commented out then f1 errors the same way (playground):

   Compiling playground v0.0.1 (/playground)
error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable
 --> src/lib.rs:7:5
  |
3 | fn f1(map: &mut HashMap<i32, String>, k: i32) -> &String {
  |            - let's call the lifetime of this reference `'1`
4 |     if let Some(s) = map.get(&k) {
  |                      --- immutable borrow occurs here
5 |         return s;
  |                - returning this value requires that `*map` is borrowed for `'1`
6 |     }
7 |     map.insert(k, k.to_string());
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here

error: aborting due to previous error

For more information about this error, try `rustc --explain E0502`.
error: could not compile `playground`.

To learn more, run the command again with --verbose.

In rustc 1.42.0 both errors are reported simultaneously (godbolt).

I'm not sure whether this is a bug or not - rustc need not report everything wrong with a crate in one go, but if it can report these simultaneously then it probably should.

@SNCPlay42
Copy link
Contributor

cargo bisect-rustc results for when the compiler stopped reporting both errors simultaneously:

searched nightlies: from nightly-2020-02-06 to nightly-2020-07-05
regressed nightly: nightly-2020-02-16
searched commits: from 433aae9 to 61d9231
regressed commit: 19288dd

@Nugine
Copy link
Contributor Author

Nugine commented Jul 6, 2020

I have found the answer.
http://smallcultfollowing.com/babysteps/blog/2018/06/15/mir-based-borrow-check-nll-status-update/
https://nikomatsakis.github.io/rust-belt-rust-2019/#87

How can I work around it without Polonius?
Just use Entry API.

@tmandry
Copy link
Member

tmandry commented Jul 7, 2020

Thanks for digging into this, @SNCPlay42! This doesn't appear to be async-related, so removing that tag.

@tmandry tmandry added I-prioritize Issue: Indicates that prioritization has been requested for this issue. and removed A-async-await Area: Async & Await labels Jul 7, 2020
@nikomatsakis
Copy link
Contributor

If @SNCPlay42's bisection is correct (thanks!), then this was triggered by #67681 (cc @matthewjasper). It is indeed true that the code example is not expected to compile, though you could make it compile by either using entry API or adding something like

if map.contains(..) {
   if let Some(...) = ... {
      ...
   }
}

I'm not sure exactly what caused this to occur, but it makes sense that #67681 might be related since that affected how we manage opaque type inference (and the async fn implicitly uses an opaque return type).

@matthewjasper
Copy link
Contributor

The error is being suppressed because rustc_typeck::check_crate is (effectively) aborting compilation early.

@spastorino spastorino added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jul 8, 2020
@spastorino
Copy link
Member

Prioritized as P-medium as per Zulip discussion.

@compiler-errors
Copy link
Member

Both errors are reported now. This is a polonius problem, I believe.

@compiler-errors compiler-errors added NLL-polonius Issues related for using Polonius in the borrow checker fixed-by-polonius Compiling with `-Zpolonius` fixes this issue. labels Dec 31, 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. NLL-polonius Issues related for using Polonius in the borrow checker P-medium Medium priority 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

8 participants