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

Possibly confusing error message when deref/deref_mut are used. #58843

Open
real-felix opened this issue Mar 1, 2019 · 3 comments
Open

Possibly confusing error message when deref/deref_mut are used. #58843

real-felix opened this issue Mar 1, 2019 · 3 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@real-felix
Copy link

Beginners can be confused when deref and deref_mut are called implicitely:

In this given code:

struct Foo {
    a: Vec<i32>,
    b: Vec<i32>,
}

fn add(foo: std::sync::Mutex<Foo>) {
    let f = foo.lock().unwrap();

    for i in &mut f.a {
        for j in &f.b {
            *i += *j;
        }
    }
}

The error message is not the clearest:

error[E0502]: cannot borrow `f` as immutable because it is also borrowed as mutable
  --> src/lib.rs:10:19
   |
9  |     for i in &mut f.a {
   |              --------
   |              |    |
   |              |    mutable borrow occurs here
   |              mutable borrow used here, in later iteration of loop
10 |         for j in &f.b {
   |                   ^ immutable borrow occurs here

The users could not understand that the borrows are done by deref and deref_mut, since the calls are implicit. They could think that, for an unknown reason, the borrow checker does not understand that the fields are disjoint.

I think that it would be an improvement if the compiler added that the implicit call to deref/deref_mut is responsible of the borrowing:

error[E0502]: cannot borrow `f` as immutable because it is also borrowed as mutable
  --> src/lib.rs:10:19
   |
9  |     for i in &mut f.a {
   |              --------
   |              |    |
   |              |    mutable borrow occurs here due to a call to `Deref::deref`
   |              mutable borrow used here, in later iteration of loop
10 |         for j in &f.b {
   |                   ^ immutable borrow occurs here due to a call to `DerefMut::deref_mut`

This could be even better if the compiler gave the solution:

You can call `DerefMut::deref_mut` before borrowing the fields:
  --> src/lib.rs:8:1
7  |    let mut f = foo.lock().unwrap();
   |    let f = DerefMut::deref_mut(&mut f);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@jonas-schievink jonas-schievink added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 1, 2019
@matthewjasper matthewjasper added the NLL-diagnostics Working towards the "diagnostic parity" goal label Mar 14, 2019
@crlf0710 crlf0710 added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jun 11, 2020
@matklad
Copy link
Member

matklad commented May 20, 2021

+1, I've spend several minutes recently figuring this out (luckily, inlay hints in the IDE revealed the deref after some times. Otherwise, I might have been debugging this still :)

The suggestion should probably be &mut *f rather than DerefMut::deref_mut(&mut f)

@Aaron1011
Copy link
Member

I'm removing the NLL-diagnostics label, since this produces the same error message in both NLL and non-NLL mode. To fix this, we should be able to extend the support added in PR #75304 to cover these types of errors.

@Aaron1011 Aaron1011 removed the NLL-diagnostics Working towards the "diagnostic parity" goal label Oct 2, 2021
@jyn514
Copy link
Member

jyn514 commented Dec 22, 2021

Ran into this today on the community server - here's a slightly simpler version without loops:

use std::sync::Mutex;

struct Foo {
    bar: String,
    baz: String,
}

fn main() {
    // breaks
    let mutex = Mutex::new(Foo { bar: "hi".into(), baz: "hello".into() });
    let mut x = mutex.lock().unwrap();
    
    // works fine
    // let mut x = Foo { bar: "hi".into(), baz: "hello".into() };
    let bar = &mut x.bar;
    let baz = &mut x.baz;
    *bar = "new".into();
}

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. 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

7 participants