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

MIR borrowck: mutable use of immutable upvar causes duplicate errors #46559

Closed
arielb1 opened this issue Dec 7, 2017 · 4 comments
Closed

MIR borrowck: mutable use of immutable upvar causes duplicate errors #46559

arielb1 opened this issue Dec 7, 2017 · 4 comments
Labels
A-NLL Area: Non-lexical lifetimes (NLL) C-bug Category: This is a bug. NLL-diagnostics Working towards the "diagnostic parity" goal T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@arielb1
Copy link
Contributor

arielb1 commented Dec 7, 2017

e.g. the test compile-fail/unboxed-closures-mutate-upvar, or this code:

fn main() {
    let x = 0;
    || { &mut x; };
}

Causes a double error with MIR borrowck

$ ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc double-error.rs -Z borrowck=compare
error[E0595]: closure cannot assign to immutable local variable `x` (Ast)
 --> double-error.rs:3:5
  |
2 |     let x = 0;
  |         - consider changing this to `mut x`
3 |     || { &mut x; };
  |     ^^ cannot borrow mutably

error[E0596]: cannot borrow immutable item `x` as mutable (Mir)
 --> double-error.rs:3:5
  |
3 |     || { &mut x; };
  |     ^^^^^^^^^^^^^^ cannot borrow as mutable

error[E0596]: cannot borrow immutable item `x` as mutable (Mir)
 --> double-error.rs:3:10
  |
3 |     || { &mut x; };
  |          ^^^^^^ cannot borrow as mutable
  |
  = note: Value not mutable causing this error: `x`

error: aborting due to 3 previous errors

The reason is that the borrow of x is inferred as mutable, rather than unique, which means there's an error when creating the closure.

I think the best way to solve this would be to limit borrows of immutable upvars to unique - a mutable borrow can't succeed anyway, so this will only cause errors. We can't change the borrow to be unique, because otherwise AST borrowck will not report an error, but maybe we can do this in MIR construction?

@arielb1 arielb1 added this to the NLL prototype milestone Dec 7, 2017
@nikomatsakis nikomatsakis removed this from the NLL prototype milestone Dec 12, 2017
@XAMPPRocky XAMPPRocky added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-NLL Area: Non-lexical lifetimes (NLL) C-bug Category: This is a bug. labels Mar 26, 2018
@nikomatsakis
Copy link
Contributor

Still true:

error[E0596]: cannot borrow immutable item `x` as mutable
 --> src/main.rs:5:10
  |
5 |     || { &mut x; };
  |          ^^^^^^ cannot borrow as mutable
  |
  = note: Value not mutable causing this error: `x`

error[E0596]: cannot borrow immutable item `x` as mutable
 --> src/main.rs:5:5
  |
5 |     || { &mut x; };
  |     ^^^^^^^^^^^^^^ cannot borrow as mutable

@nikomatsakis nikomatsakis added NLL-diagnostics Working towards the "diagnostic parity" goal NLL-priority labels Apr 3, 2018
@nikomatsakis
Copy link
Contributor

Marking as release candidate: minor diagnostic defect.

@matthewjasper matthewjasper self-assigned this Jul 15, 2018
bors added a commit that referenced this issue Jul 21, 2018
[NLL] Mutability errors

cc #51028
cc #51170
cc #46559
Closes #46629

* Better explain why the place is immutable ("immutable item" is gone)
* Distinguish &T and *const T
* Use better spans when a mutable borrow is for a closure capture

r? @pnkfelix
@matthewjasper
Copy link
Contributor

The message is no longer duplicated in most cases. There are two errors when trying to capture an immutable variable that's behind a reference. I'm not sure if this is a problem, since if the user wants to keep the assignment, then fixing just one of these errors isn't sufficient.

fn fn_ref<F: Fn()>(f: F) -> F { f }

fn two_closures_ref(x: i32) {
    fn_ref(|| {
        || //~ ERROR
         x = 1;} //~ ERROR
    );
    fn_ref(move || {
        ||  //~ ERROR
    x = 1;}); //~ ERROR
}

@matthewjasper matthewjasper removed their assignment Jul 22, 2018
@nikomatsakis
Copy link
Contributor

I'm going to close this bug, seems "good enough".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NLL Area: Non-lexical lifetimes (NLL) C-bug Category: This is a bug. NLL-diagnostics Working towards the "diagnostic parity" goal 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