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

"immutable field" errors are confusing when the handle is mutable, should describe why it is immutable #18150

Closed
huonw opened this issue Oct 19, 2014 · 9 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@huonw
Copy link
Member

huonw commented Oct 19, 2014

struct Foo { a: usize }
fn main() {
    let mut x = std::rc::Rc::new(Foo { a: 1 });
    x.a += 1;

    let mut y = &(Foo { a: 1 });
    y.a += 1;
}
<anon>:4:5: 4:13 error: cannot assign to immutable field
<anon>:4     x.a += 1;
             ^~~~~~~~
<anon>:7:5: 7:13 error: cannot assign to immutable field `y.a`
<anon>:7     y.a += 1;
             ^~~~~~~~

Gives no indication that the field is immutable because it is stored inside an Rc/&. These example are somewhat obvious, since the initialiser is directly above, but it's certainly possible to be matching deep inside some data structure and meet this error even though there are muts on all the appropriate variables.

The error message could include: "note: this field is immutable because it is being accessed via an & reference [created by the autoderef of a Rc]".

@huonw huonw added the A-diagnostics Area: Messages for errors, warnings, and lints label Oct 19, 2014
@steveklabnik
Copy link
Member

steveklabnik commented Oct 27, 2015

Triage: same problem, but some updated code:

struct Foo { a: usize }
fn main() {
    let mut x = std::rc::Rc::new(Foo { a: 1 });
    x.a += 1;

    let mut y = &(Foo { a: 1 });
    y.a += 1;
}

@steveklabnik
Copy link
Member

Triage: no change

@steveklabnik steveklabnik added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 9, 2017
@Mark-Simulacrum Mark-Simulacrum added C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed I-papercut labels Jul 22, 2017
@estebank
Copy link
Contributor

Related: #47774.

@estebank
Copy link
Contributor

Current output:

error[E0594]: cannot assign to field of immutable binding
 --> src/main.rs:4:5
  |
4 |     x.a += 1;
  |     ^^^^^^^^ cannot mutably borrow field of immutable binding

error[E0594]: cannot assign to field `y.a` of immutable binding
 --> src/main.rs:7:5
  |
7 |     y.a += 1;
  |     ^^^^^^^^ cannot mutably borrow field of immutable binding

@estebank
Copy link
Contributor

Current output:

error[E0594]: cannot assign to data in a `&` reference
 --> src/main.rs:4:5
  |
4 |     x.a += 1;
  |     ^^^^^^^^ cannot assign

error[E0594]: cannot assign to `y.a` which is behind a `&` reference
 --> src/main.rs:7:5
  |
6 |     let mut y = &(Foo { a: 1 });
  |                 --------------- help: consider changing this to be a mutable reference: `&mut (Foo { a: 1 })`
7 |     y.a += 1;
  |     ^^^^^^^^ `y` is a `&` reference, so the data it refers to cannot be written

@estebank estebank added E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. P-high High priority labels May 25, 2019
@estebank
Copy link
Contributor

Case from #52941:

use std::rc::Rc;
use std::cell::RefCell;

#[derive(Debug)]
struct TestStruct {
    u: u32,
}

fn main() {
    let mut val = TestStruct{ u: 1 };
    val.u += 2;
    println!("val {:?} ", val);
    //prints: val TestStruct { u: 3 }
    
    let mut val_rc = Rc::new(TestStruct{ u: 10 });    
    //above I mistakenly assumed "mut" would let me mutate the subcontents of the Rc.
    val_rc.u += 2;
    //above creates compile error: error[E0594]: cannot assign to data in a `&` reference
    println!("val_rc {:?} ", val_rc);    

    //The solution I used
    let val_rc_refcell = Rc::new(RefCell::new(TestStruct{ u: 20 }));
    (*val_rc_refcell).borrow_mut().u += 2;
    println!("val_rc_refcell {:?} ", val_rc_refcell);        
}

@pnkfelix
Copy link
Member

If I understand correctly, it seems like the main thing that is currently missing from the desiderata here is something in the diagnostic connecting the Rc type in the users code with the &-type that the compiler gets via auto-deref of the Rc

@pnkfelix
Copy link
Member

I'll nominate this for discussion at the T-compiler meeting: I do not think the P-high tag is warranted (it was added to this issue based on feedback in #52941), but I want to make sure we at least discuss this issue as a group before I revise its priority.

@pnkfelix
Copy link
Member

pnkfelix commented Jul 4, 2019

after discussing at T-compiler meeting, closing this issue (which tracked an old problem that is much improved) and reopening #52941. Marking #52941 as P-high for now, but that might change in near term. (Cc @eddyb)

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-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. P-high High 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

5 participants