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

Improve "previous borrow" location in borrow error message #29733

Closed
thibault-ml opened this issue Nov 10, 2015 · 1 comment · Fixed by #59114
Closed

Improve "previous borrow" location in borrow error message #29733

thibault-ml opened this issue Nov 10, 2015 · 1 comment · Fixed by #59114
Labels
A-borrow-checker Area: The borrow checker A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled.

Comments

@thibault-ml
Copy link

thibault-ml commented Nov 10, 2015

Given the following code:

use std::collections::HashMap;

fn main() {
    let mut vec = vec![0, 1, 2, 3, 4];
    let mut map = HashMap::<&i32, &str>::new();

    for key in &vec {
        map.insert(key, "value");
    }

    vec.push(5);
}

The compiler will complain with the following error message:

error[E0502]: cannot borrow `vec` as mutable because it is also borrowed as immutable
  --> /tmp/test_borrow_message.rs:11:5
   |
7  |     for key in &vec {
   |                 --- immutable borrow occurs here
...
11 |     vec.push(5);
   |     ^^^ mutable borrow occurs here
12 | }
   | - immutable borrow ends here

error: aborting due to previous error

While the borrow error is expected, the message seems pretty unclear to me.

Removing/commenting the map.insert(key, "value"); line will allow rustc to compile the file, which shows the problem isn't really exactly with the borrow happening at line 7 (for loop), but with the fact that the borrow is then extended to the lifetime of the map variable.
This can be verified with the following code which works because the map variable is gone by the time we need to mutable borrow again:

use std::collections::HashMap;

fn main() {
    let mut vec = vec![0, 1, 2, 3, 4];
    {
        let mut map = HashMap::<&i32, &str>::new();
        for key in &vec {
            map.insert(key, "value");
        }
    }
    vec.push(5);
}

It feels like the compiler should be clearer as to where the borrow is actually "stored". It's able to show that the borrow ends when map goes away, so I feel it should be clearer that the borrow issue stems from the map.insert(key, "value"); line, which extends the borrow's "lifetime" to the lifetime of map

(this error is output by rust 1.4.0)

@steveklabnik steveklabnik added the A-diagnostics Area: Messages for errors, warnings, and lints label Nov 11, 2015
@sanxiyn sanxiyn added the A-borrow-checker Area: The borrow checker label Nov 20, 2015
@Kixunil
Copy link
Contributor

Kixunil commented Feb 6, 2017

Seems like it's similar to one I've encountered. (Try commenting-out body of for loop.)

@steveklabnik steveklabnik removed the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 9, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 24, 2017
@estebank estebank added the NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled. label Oct 15, 2018
bors added a commit that referenced this issue Mar 11, 2019
Enable NLL migrate mode on the 2015 edition

Blocked on #58739

## What is in this PR?

* Remove the `-Zborrowck=ast` flag option from rustc.
* The default in the 2015 edition is now `-Zborrowck=migrate`.
* The 2018 edition default is unchanged: it's still `-Zborrowck=migrate`.
* Enable the `-Ztwo-phase-borrows` flag on all editions.
* Remove most dead code that handled these options.
* Update tests for the above changes.

## What is *not* in this PR?

These are left for future PRs

* Use `-Zborrowck=mir` in NLL compare mode tests
* Remove the `-Zborrowck=compare` option
* Remove the `-Ztwo-phase-borrows` flag. It's kept so that perf.rlo has time to stop using it (cc @Mark-Simulacrum)
* Remove MIR typeck as its own MIR pass - it's now run by NLL.
* Enabling `-Zborrowck=mir` by default

Soundness issues that are fixed by NLL will stay open until full NLL is emitting hard errors. However, these diagnostics and completeness issues can now be closed:

Closes #18330
Closes #22323
Closes #23591
Closes #26736
Closes #27487
Closes #28092
Closes #28970
Closes #29733
Closes #30104
Closes #38915
Closes #39908
Closes #43407
Closes #47524
Closes #48540
Closes #49073
Closes #52614
Closes #55085
Closes #56093
Closes #56496
Closes #57804

cc #43234

r? @pnkfelix
cc @rust-lang/lang
cc @rust-lang/wg-compiler-nll
bors added a commit that referenced this issue Apr 22, 2019
Enable NLL migrate mode on the 2015 edition

## What is in this PR?

* Remove the `-Zborrowck=ast` flag option from rustc.
* The default in the 2015 edition is now `-Zborrowck=migrate`.
* The 2018 edition default is unchanged: it's still `-Zborrowck=migrate`.
* Enable two-phase borrows (currently toggled via the `-Ztwo-phase-borrows` flag) on all editions.
* Remove most dead code that handled these options.
* Update tests for the above changes.

## What is *not* in this PR?

These are left for future PRs

* Use `-Zborrowck=mir` in NLL compare mode tests (#56993)
* Remove the `-Zborrowck=compare` option (#59193)
* Remove the `-Ztwo-phase-borrows` flag. It's kept, as a flag that does nothing so that perf.rlo has time to stop using it (cc @Mark-Simulacrum)
* Remove MIR typeck as its own MIR pass - it's now run by NLL.
* Enabling `-Zborrowck=mir` by default (#58781)
* Replace `allow_bind_by_move_patterns_with_guards` and `check_for_mutation_in_guard_via_ast_walk` with just using the feature gate. (#59192)

Soundness issues that are fixed by NLL will stay open until full NLL is emitting hard errors. However, these diagnostics and completeness issues can now be closed:

Closes #18330
Closes #22323
Closes #23591
Closes #26736
Closes #27487
Closes #28092
Closes #28970
Closes #29733
Closes #30104
Closes #38915
Closes #39908
Closes #43407
Closes #47524
Closes #48540
Closes #49073
Closes #52614
Closes #55085
Closes #56093
Closes #56496
Closes #57804

cc #43234

r? @pnkfelix
cc @rust-lang/lang
cc @rust-lang/wg-compiler-nll
bors added a commit that referenced this issue Apr 22, 2019
Enable NLL migrate mode on the 2015 edition

## What is in this PR?

* Remove the `-Zborrowck=ast` flag option from rustc.
* The default in the 2015 edition is now `-Zborrowck=migrate`.
* The 2018 edition default is unchanged: it's still `-Zborrowck=migrate`.
* Enable two-phase borrows (currently toggled via the `-Ztwo-phase-borrows` flag) on all editions.
* Remove most dead code that handled these options.
* Update tests for the above changes.

## What is *not* in this PR?

These are left for future PRs

* Use `-Zborrowck=mir` in NLL compare mode tests (#56993)
* Remove the `-Zborrowck=compare` option (#59193)
* Remove the `-Ztwo-phase-borrows` flag. It's kept, as a flag that does nothing so that perf.rlo has time to stop using it (cc @Mark-Simulacrum)
* Remove MIR typeck as its own MIR pass - it's now run by NLL.
* Enabling `-Zborrowck=mir` by default (#58781)
* Replace `allow_bind_by_move_patterns_with_guards` and `check_for_mutation_in_guard_via_ast_walk` with just using the feature gate. (#59192)

Soundness issues that are fixed by NLL will stay open until full NLL is emitting hard errors. However, these diagnostics and completeness issues can now be closed:

Closes #18330
Closes #22323
Closes #23591
Closes #26736
Closes #27487
Closes #28092
Closes #28970
Closes #29733
Closes #30104
Closes #38915
Closes #39908
Closes #43407
Closes #47524
Closes #48540
Closes #49073
Closes #52614
Closes #55085
Closes #56093
Closes #56496
Closes #57804

cc #43234

r? @pnkfelix
cc @rust-lang/lang
cc @rust-lang/wg-compiler-nll
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 A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants