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

Unclear error E0594 #52941

Closed
StyMaar opened this issue Aug 1, 2018 · 19 comments · Fixed by #62468
Closed

Unclear error E0594 #52941

StyMaar opened this issue Aug 1, 2018 · 19 comments · Fixed by #62468
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@StyMaar
Copy link

StyMaar commented Aug 1, 2018

When attempting to mutate a field of a borrowed struct (with a shared reference &) we get the unhelpful E5094 error :

error[E0594]: cannot assign to field reagent.unreserved_volume of immutable binding [emphasis mine]

Sample code :

fn main() {
    let foo = &Bar{x: 5}; // or any function that returns a shared reference
    foo.x -= 12;
}

struct Bar {
    x: i32
}

Error :

| foo.x -= 12;
| ^^^^^^^^^^^ cannot mutably borrow field of immutable binding

Since the compiler complains about a “binding” being immutable, my reflex was to add a mut at foo's declaration site :

fn main() {
    let mut foo = &Bar{x: 5}; // or any function that returns a shared reference
    foo.x -= 12;
}

struct Bar {
    x: i32
}

Which didn't change anything. (except adding a warning that I didn't notice)

The problem here, is that the error message is complaining about an immutable binding whereas it should complain about an immutable (or shared) reference.

To add insult to injury, the error E0594 doesn't exist in the error index and rustc --explain E0594 returns the laconic

error: no extended information for E0594

Meta

rustc --version --verbose:

rustc 1.29.0-nightly (e94df4a 2018-07-31)
binary: rustc
commit-hash: e94df4a
commit-date: 2018-07-31
host: x86_64-unknown-linux-gnu
release: 1.29.0-nightly
LLVM version: 7.0

The behavior is the same on Stable 1.27.2 though.

@StyMaar
Copy link
Author

StyMaar commented Aug 1, 2018

Additional note : it's not just a problem with the error message of E0594, since the same error is also raised when it's actually a immutable binding problem :

fn main() {
 let y = 0f32;
 let foo = Bar{x: 0f32};
 foo.x= y;
}

struct Bar {
    x: f32
}

It should just be another error kind raised in the situation described by this issue. (And also E0594 should probably appear in the error index and as a result of rustc --explain E0594)

@csmoe csmoe added the A-diagnostics Area: Messages for errors, warnings, and lints label Aug 1, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Aug 1, 2018

I think NLL does a better job with error message here.

Look at the output when you run with 2018 edition enabled (playground):

error[E0594]: cannot assign to `foo.x` which is behind a `&` reference
 --> src/main.rs:3:5
  |
2 |     let foo = &Bar{x: 5}; // or any function that returns a shared reference
  |               ---------- help: consider changing this to be a mutable reference: `&mut Bar{x: 5}`
3 |     foo.x -= 12;
  |     ^^^^^^^^^^^ `foo` is a `&` reference, so the data it refers to cannot be written

@pnkfelix
Copy link
Member

pnkfelix commented Aug 1, 2018

(as in, it may be reasonable to tag this as NLL-fixed-by-NLL...)

@pnkfelix
Copy link
Member

pnkfelix commented Aug 1, 2018

Likewise, here's what NLL does for the second provided example (playground):

error[E0384]: cannot assign twice to immutable variable `foo`
 --> src/main.rs:4:2
  |
3 |  let foo = Bar{x: 0f32};
  |      ---
  |      |
  |      first assignment to `foo`
  |      consider changing this to `mut foo`
4 |  foo.x = y;
  |  ^^^^^^^^^ cannot assign twice to immutable variable

which I think also satisfies the request written in the comment above?

@StyMaar
Copy link
Author

StyMaar commented Aug 1, 2018

You're right, that's way better with NLL !

I'm a bit surprised that enabling 2018 edition activates NLL: does that mean that NLL will be limited to the 2018 edition ?

@StyMaar
Copy link
Author

StyMaar commented Aug 1, 2018

That also means that with NLL, the meaning a E0594 is changing (from «immutable binding error» also triggered in case of «immutable reference error» to «immutable reference error» only).

@pnkfelix
Copy link
Member

pnkfelix commented Aug 1, 2018

I'm a bit surprised that enabling 2018 edition activates NLL: does that mean that NLL will be limited to the 2018 edition ?

No, not in the long term.

For the short term, we are connecting the enabling of NLL to the 2018 edition as a way to try to get it tested by people who are willing to deal with the headaches NLL might imply in its current state.

  • These headaches include various diagnostic regressions and compiler performance regressions, all of which we hope to resolve.
  • And to be honest, I also am happy to attach NLL to the 2018 edition for the short term, so that we can add it to the list of cool stuff that will encourage people to try out the 2018 edition ASAP.

In the long term, we are planning to move everybody to NLL, regardless of edition. The schedule for when that will happen hasn't been established yet; I suspect it will depend on the feedback we get from 2018 edition preview 2.

  • But I wouldn't be surprised if we turn NLL (at least in its current migration mode) on across the board at the same time that we release the final version of the 2018 edition.

@StyMaar
Copy link
Author

StyMaar commented Aug 2, 2018

Thanks for your answers. I think I can close the issue.

@StyMaar StyMaar closed this as completed Aug 2, 2018
@dustinfreeman
Copy link

dustinfreeman commented Dec 20, 2018

I just want to chime in – I found E0594's message to still be confusing even when using Rust 2018 Edition, version 1.31. I'm only 1 month into using Rust, and in the code below I was mis-using Rc:

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

Can we hope for a clearer error message than error[E0594]: cannot assign to data in a & reference ?

@StyMaar
Copy link
Author

StyMaar commented Apr 9, 2019

I'm re-opening this issue because of what @dustinfreeman said, and because E0594 is still not part of the error index.

@StyMaar StyMaar reopened this Apr 9, 2019
@cumbreras
Copy link

agreed, this would be helpful, as I've found myself last couple of days falling to find it.

@estebank
Copy link
Contributor

The error index work is tracked in #61137. Extending E0594 to be more useful is tracked in #18150.

@pnkfelix
Copy link
Member

pnkfelix commented Jul 4, 2019

reopening as the modern, more narrowly focused version of #18150

(though really, an entirely new issue might be warranted at this point, to avoid the discussion of NLL and what not...)

@pnkfelix
Copy link
Member

pnkfelix commented Jul 4, 2019

marking P-high to reflect desired prioritization of @estebank , and assigning to self.

@pnkfelix pnkfelix added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. P-high High priority labels Jul 4, 2019
@pnkfelix pnkfelix self-assigned this Jul 4, 2019
bors added a commit that referenced this issue Jul 13, 2019
…bank

Improve diagnostics for invalid mutation through overloaded operators

Closes #58864
Closes #52941
Closes #57839
@TitanThinktank

This comment has been minimized.

@00benallen

This comment has been minimized.

@Qqwy
Copy link

Qqwy commented Jan 10, 2020

Note that as of January 2020, this E0594 still is not part of the error index.

@estebank
Copy link
Contributor

@Qqwy that work is tracked in #61137.

@estebank
Copy link
Contributor

@Qqwy that work is tracked in #61137. This error code has been added to the index.

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 P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants