Skip to content

"cannot assign to immutable field" could be confusing #35937

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.

Sign up for GitHub

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

Jump to bottom
Closed
durka opened this issue Aug 23, 2016 · 11 comments
Closed

"cannot assign to immutable field" could be confusing #35937

durka opened this issue Aug 23, 2016 · 11 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@durka
Copy link
Contributor

durka commented Aug 23, 2016 

struct S { x: i32 }
let s = S { x: 42 };
s.x += 1; //~ERROR cannot assign to immutable field

It occurred to me that this might be confusing to newcomers because fields don't have mutability in Rust, instead we have inherited mutability. So maybe it should say "cannot assign to field of immutable struct" or "cannot modify field due to immutable binding" or something.

cf #18150
@eddyb
Copy link
Member

eddyb commented Aug 23, 2016

cc @jonathandturner

@sfackler sfackler added A-docs A-diagnostics Area: Messages for errors, warnings, and lints and removed A-docs labels Aug 23, 2016
@Zoxc
Copy link
Contributor

Zoxc commented Dec 23, 2016

I got this error now:

error: cannot assign to immutable field
   --> C:\Users\John\Documents\rust-co\coroutines\future\src\lib.rs:128:21
    |
128 |                     self.timers[i].delta -= 1;
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^

Where self is &mut EventLoop which means I don't know the real cause of the error.

From:
https://github.com/Zoxc/coroutines/blob/e25d2b66a8cae8b03440ed4fdd2d32408fd71735/future/src/lib.rs#L128

@eddyb
Copy link
Member

eddyb commented Dec 23, 2016

@Zoxc My guess would be missing DerefMut or IndexMut impl.

@nikomatsakis @jonathandturner We should print the first element of the lvalue that causes the whole lvalue to be immutable only, either an immutable variable, reference or overloaded deref/indexing.

@Zoxc
Copy link
Contributor

Zoxc commented Dec 23, 2016

@eddyb I forgot I changed timers to Vec<Rc<Timer>>

@eddyb
Copy link
Member

eddyb commented Dec 23, 2016 •
edited
Loading

The error message would ideally contain "...because Rc<Timer> implements Deref but not DerefMut".

@nikomatsakis
Copy link
Contributor

@eddyb

@nikomatsakis @jonathandturner We should print the first element of the lvalue that causes the whole lvalue to be immutable only, either an immutable variable, reference or overloaded deref/indexing.

I was working with @caipre on a PR to do this at some point, but I think they lost steam. It was a bit trickier than I at first thought. In any case, there is an issue specific to this problem: #28419

@caipre
Copy link
Contributor

caipre commented Jan 6, 2017

Yeah, sorry about that. 😞 The issue turned out to be rather more difficult than I anticipated and I got caught up with some things where I wasn't able to put time into it any longer. If someone has a good idea of how to resolve that issue, I don't want to be a blocker.

@steveklabnik steveklabnik removed the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 9, 2017
@estebank estebank mentioned this issue Mar 23, 2017
Closed
estebank added a commit to estebank/rust that referenced this issue Mar 24, 2017
@estebank
Point to definition when modifying field of immutable variable
289772f 
Given a file

```rust
struct S {
    x: i32,
}
fn foo() {
    let s = S { x: 42 };
    s.x += 1;
}
fn bar(s: S) {
    s.x += 1;
}
```

Provide the following output:

```rust
error: cannot assign to immutable field `s.x`
 --> $DIR/issue-35937.rs:16:5
  |
5 |     let s = S { x: 42 };
  |         - consider changing this to `mut s`
6 |     s.x += 1;
  |     ^^^^^^^^ cannot mutably borrow immutable field

error: cannot assign to immutable field `s.x`
 --> $DIR/issue-35937.rs:20:5
  |
8 | fn bar(s: S) {
  |        - consider changing this to `mut s`
9 |     s.x += 1;
  |     ^^^^^^^^ cannot mutably borrow immutable field
```

Follow up to rust-lang#40445. Fix rust-lang#35937.
@arielb1 arielb1 mentioned this issue Mar 26, 2017
Merged
arielb1 added a commit to arielb1/rust that referenced this issue Mar 26, 2017
@arielb1
borrowck: consolidate mut suggestions
50728e5 
This converts all of borrowck's `mut` suggestions to a new
`mc::ImmutabilityBlame` API instead of the current mix of various hacks.

Fixes rust-lang#35937.
Fixes rust-lang#40823.
frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 29, 2017
@frewsxcv
Rollup merge of rust-lang#40841 - arielb1:immutable-blame, r=pnkfelix
a63b1df 
borrowck: consolidate `mut` suggestions

This converts all of borrowck's `mut` suggestions to a new
`mc::ImmutabilityBlame` API instead of the current mix of various hacks.

Fixes rust-lang#35937.
Fixes rust-lang#40823.
Fixes rust-lang#40859.

cc @estebank
r? @pnkfelix
@durka
Copy link
Contributor Author

durka commented Apr 19, 2017 •
edited
Loading

IMO this was not fixed by #40767. The note that was added (suggesting a change to the struct binding) is helpful, but the error still says "immutable field" which implies that individual fields can be mutable.

@steveklabnik steveklabnik reopened this Apr 19, 2017
@nikomatsakis
Copy link
Contributor

@durka I am inclined to agree. I'd like us to be less specific in many of these messages, since I think they imply things that are not true.

@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 26, 2017
@estebank estebank mentioned this issue Dec 31, 2017
Merged
bors added a commit that referenced this issue Jan 1, 2018
@bors
Auto merge of #47098 - estebank:immutable-field, r=nagisa
d5f2745 
Reword trying to operate in immutable fields

The previous message ("cannot assign/mutably borrow immutable field")
when trying to modify a field of an immutable binding gave the
(incorrect) impression that fields can be mutable independently of their
ADT's binding. Slightly reword the message to read "cannot
assign/mutably borrow field of immutable binding".

Re #35937.
@estebank
Copy link
Contributor

Current output:

error[E0594]: cannot assign to field `s.x` of immutable binding
 --> src/main.rs:4:1
  |
3 | let s = S { x: 42 };
  |     - consider changing this to `mut s`
4 | s.x += 1;
  | ^^^^^^^^ cannot mutably borrow field of immutable binding

@durka
Copy link
Contributor Author

durka commented Mar 20, 2018

LGTM.

@durka durka closed this as completed Mar 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@Zoxc @steveklabnik @durka @eddyb @nikomatsakis @caipre @sfackler @estebank @Mark-Simulacrum