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

Repeated "mutable/immutable borrow" error messages #42106

Closed
tommyip opened this issue May 19, 2017 · 8 comments · Fixed by #47146
Closed

Repeated "mutable/immutable borrow" error messages #42106

tommyip opened this issue May 19, 2017 · 8 comments · Fixed by #47146
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. WG-diagnostics Working group: Diagnostics

Comments

@tommyip
Copy link
Contributor

tommyip commented May 19, 2017

Minimal example:

fn do_something<T>(collection: &mut Vec<T>) {
    let a = &collection;
    collection.swap(1, 2);
}

Error message (ran in playground):

rustc 1.17.0 (56124baa9 2017-04-24)
error[E0502]: cannot borrow `*collection` as mutable because `collection` is also borrowed as immutable
 --> <anon>:3:5
  |
2 |     let a = &collection;
  |              ---------- immutable borrow occurs here
3 |     collection.swap(1, 2);
  |     ^^^^^^^^^^ mutable borrow occurs here
4 | }
  | - immutable borrow ends here

error[E0502]: cannot borrow `*collection` as mutable because `collection` is also borrowed as immutable
 --> <anon>:3:5
  |
2 |     let a = &collection;
  |              ---------- immutable borrow occurs here
3 |     collection.swap(1, 2);
  |     ^^^^^^^^^^ mutable borrow occurs here
4 | }
  | - immutable borrow ends here

error: aborting due to 2 previous errors

Compilation failed.

Tested on nightly as well (rustc 1.19.0-nightly (0ed1ec9f9 2017-05-18))

@Mark-Simulacrum Mark-Simulacrum added the A-diagnostics Area: Messages for errors, warnings, and lints label May 20, 2017
@est31
Copy link
Member

est31 commented May 21, 2017

Related: #40067

@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 26, 2017
@estebank
Copy link
Contributor

estebank commented Oct 2, 2017

report_error_if_loans_conflict calls report_error_if_loan_conflicts_with_restriction twice without checking the return value. Modify it to return Result<(), DiagnosticBuilder<'a>> and check where it is being called ether both calls have returned an (cancel one, emit the other, return false), wether only one was created (emit it, return false) or none has (return true).

@estebank estebank added E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. WG-diagnostics Working group: Diagnostics labels Oct 2, 2017
@estebank estebank changed the title Repeated error messages Repeated "mutable/immutable borrow" error messages Oct 3, 2017
@estebank estebank added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Oct 3, 2017
@tommyip
Copy link
Contributor Author

tommyip commented Oct 3, 2017

I'll fix this, thanks for the instruction!

@lochsh
Copy link
Contributor

lochsh commented Oct 19, 2017

I would happily have a go at this, if that's ok with @tommyip 😄

@tommyip
Copy link
Contributor Author

tommyip commented Oct 19, 2017

Sure @lochsh go for it!

@joshleeb
Copy link

@lochsh have you had time to work on this? If not could I give it a go? :)

@lochsh
Copy link
Contributor

lochsh commented Oct 28, 2017

@joshleeb I totally thought I was going to have time, but I had a crazy week at work this week. Go ahead and give it a go :) I'll find another bug to work on when I definitely have time 👍

bors added a commit that referenced this issue Oct 30, 2017
Fix duplicate display of error E0502

Ref. Repeated "mutable/immutable borrow" error messages #42106.

This PR modifies the return type of [`report_error_if_loan_conflicts_with_restriction`](https://github.com/rust-lang/rust/blob/0f0f5db465de96b6c12e71f0c7d3e475f618b104/src/librustc_borrowck/borrowck/check_loans.rs#L398-L403) so the result can be checked in [`report_error_if_loans_conflict`](https://github.com/rust-lang/rust/blob/0f0f5db465de96b6c12e71f0c7d3e475f618b104/src/librustc_borrowck/borrowck/check_loans.rs#L377-L396). This is done to prevent displaying a duplicate of the error message E0502 which is referenced in #42106.

The output of compiling:

```rust
fn do_something<T>(collection: &mut Vec<T>) {
    let _a = &collection;
    collection.swap(1, 2);
}

fn main() {}
```

is now

```bash
$ rustc src/test/compile-fail/issue-42106.rs
error[E0502]: cannot borrow `*collection` as mutable because `collection` is also borrowed as immutable
  --> src/test/compile-fail/issue-42106.rs:13:5
   |
12 |     let _a = &collection;
   |               ---------- immutable borrow occurs here
13 |     collection.swap(1, 2);
   |     ^^^^^^^^^^ mutable borrow occurs here
14 | }
   | - immutable borrow ends here

error: aborting due to 2 previous errors
```

r? @estebank
@dsheets
Copy link

dsheets commented Nov 17, 2017

It looks like the PR to fix this was merged but has a few dangling threads. There seems to be some confusion about short-circuiting functionality and the necessity of the patch and also the error count still reads 2 rather than 1. Should a new issue be opened with those issues and this issue closed?

bors added a commit that referenced this issue Jan 3, 2018
Only bump error count when we are sure that the diagnostic is not a repetition

This ensures that if we emit the same diagnostic twice, the error count will
match the real number of errors shown to the user.

Fixes #42106

This is a followup of #45603 as stated in #42106 (comment).

This program, for example:

```rust
fn do_something<T>(collection: &mut Vec<T>) {
    let _a = &collection;
    collection.swap(1, 2);
}

fn main() {}
```

without this patch, produces:

```
error[E0502]: cannot borrow `*collection` as mutable because `collection` is also borrowed as immutable
  --> $DIR/issue-42106.rs:13:5
   |
12 |     let _a = &collection;
   |               ---------- immutable borrow occurs here
13 |     collection.swap(1, 2); //~ ERROR also borrowed as immutable
   |     ^^^^^^^^^^ mutable borrow occurs here
14 | }
   | - immutable borrow ends here

error: aborting due to 2 previous errors
```

The number of errors do not match the diagnostics reported. This PR fixes this problem. The output is now in this case:

```
error[E0502]: cannot borrow `*collection` as mutable because `collection` is also borrowed as immutable
  --> $DIR/issue-42106.rs:13:5
   |
12 |     let _a = &collection;
   |               ---------- immutable borrow occurs here
13 |     collection.swap(1, 2); //~ ERROR also borrowed as immutable
   |     ^^^^^^^^^^ mutable borrow occurs here
14 | }
   | - immutable borrow ends here

error: aborting due to previous error
```

Also, some other tests outputs have been adapted because their count didn't really match the number of diagnostics reported.

As an aside, an outdated comment has been removed (`Handler::cancel` will only call to the `Diagnostic::cancel` method and will not decrease the count of errors).

All tests are passing with this PR (`x.py test` is successful).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. WG-diagnostics Working group: Diagnostics
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants