Skip to content

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

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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();
}

# for free to join this conversation on GitHub. Already have an account? # 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