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

Error message about immutable binding is not actually caused by a binding #49839

Open
gdesmott opened this issue Apr 10, 2018 · 8 comments
Open
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@gdesmott
Copy link

See the following code:

use std::collections::HashMap;

struct Test {
    v: u32,
}

fn main() {
    let mut map = HashMap::new();
    map.insert("a", Test { v: 0 });

    let values = map.values();
    for mut t in values {
        t.v += 1;
    }
}

It raises this compiler error:

error[E0594]: cannot assign to field t.v of immutable binding
--> /tmp/test.rs:14:9
|
14 | t.v += 1;
| ^^^^^^^^ cannot mutably borrow field of immutable binding

The fix is to use values_mut() instead of values().
But the error message is confusing as the problem here is not the binding as t itself is mut making it harder to debug for newcomers.

@Kimundi Kimundi added the A-diagnostics Area: Messages for errors, warnings, and lints label Apr 10, 2018
@XAMPPRocky XAMPPRocky added 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. labels Jul 7, 2018
@mbrubeck
Copy link
Contributor

In Rust 1.23 and earlier, the error was:

error[E0594]: cannot assign to immutable field `t.v`
  --> diag.rs:13:9
   |
13 |         t.v += 1;
   |         ^^^^^^^^ cannot mutably borrow immutable field

error: aborting due to previous error

@estebank
Copy link
Contributor

estebank commented May 23, 2019

Current output is slightly more misleading, but I think improving on this will be hard:

error[E0594]: cannot assign to `t.v` which is behind a `&` reference
  --> src/main.rs:13:9
   |
12 |     for mut t in values {
   |         ----- help: consider changing this to be a mutable reference: `&mut Test`
13 |         t.v += 1;
   |         ^^^^^^^^ `t` is a `&` reference, so the data it refers to cannot be written

@estebank
Copy link
Contributor

This overlaps slightly with #18150, but is more targeted, leaving open.

@estebank estebank added E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-medium Medium priority labels May 25, 2019
@steveklabnik
Copy link
Member

steveklabnik commented May 12, 2020

Triage: no change

@sasurau4
Copy link
Contributor

I'll work on it. @rustbot calim

current output is following.

error[E0594]: cannot assign to `t.v` which is behind a `&` reference
  --> src/main.rs:13:9
   |
12 |     for mut t in values {
   |                  ------ this iterator yields `&` references
13 |         t.v += 1;
   |         ^^^^^^^^ `t` is a `&` reference, so the data it refers to cannot be written

@estebank
Copy link
Contributor

estebank commented Jan 18, 2021

@sasurau4 you might want to first focus on the case where the values() method has been called in the for loop as it will be easier to find the needed expressions and types to verify that values_mut is available. It would also be interesting to account for .iter() vs .iter_mut() as well as part of this work (it just adds another method to check for). For the exact case that is presented in the original report, you'll have to find where the binding that is in the iterator expression of the for loop comes from, which as things are today might be had (but not impossible for the common case) to do. I don't want to discourage you from tackling it if you desire to do so, but I do want you to know that so you don't get discouraged if you find it takes to long or becomes too hard. Handling the simpler cases has a lot of value on their own.

@sasurau4
Copy link
Contributor

@estebank Thanks for your advice! I was stuck on how to get the binding rhs expression for.
I'll focus on the case you suggest 👍

JohnTitor added a commit to JohnTitor/rust that referenced this issue Feb 10, 2021
…hod-for-loop, r=oli-obk

Add suggest mut method for loop

Part of rust-lang#49839

This PR focus on [the comment case](rust-lang#49839 (comment))
@estebank
Copy link
Contributor

Thanks to @sasurau4 the output for the "inlined" case is now:

error[E0594]: cannot assign to `t.v` which is behind a `&` reference
  --> src/main.rs:12:9
   |
11 |     for mut t in map.values() {
   |                  ------------
   |                  |   |
   |                  |   help: use mutable method: `values_mut()`
   |                  this iterator yields `&` references
12 |         t.v += 1;
   |         ^^^^^^^^ `t` is a `&` reference, so the data it refers to cannot be written

The let values = map.values(); case still needs to be handled.

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-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-medium Medium priority 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