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

Add suggestion/explanation to error on trying to mutably borrow immutable reference #45405

Open
oli-obk opened this issue Oct 20, 2017 · 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. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-diagnostics Working group: Diagnostics

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Oct 20, 2017

    let mut mem_buffer : &[u8] = &b"foo"[..];
    let mut reader = &mut mem_buffer as &std::io::Read;
    let mut read_buffer = [0u8, 10];
    reader.read(&mut read_buffer);

reports

error[E0596]: cannot borrow immutable borrowed content `*reader` as mutable
 --> src/main.rs:5:5
  |
5 |     reader.read(&mut read_buffer);
  |     ^^^^^^ cannot borrow as mutable

Which is correct, but the fix is to modify the creation of the reader variable. While this case is easy, it's probably less so in the general case. Possible avenues of improvement:

  1. The easy way this error message can be improved is to mention that the type is &T, but should be &mut T.
  2. If there are explicit type annotations on the variable, a note should point to it
  3. If the variable's type is inferred, a note should point to its initializer and maybe even do some primitive checks that can lead the developer towards the solution.
@kennytm kennytm added the A-diagnostics Area: Messages for errors, warnings, and lints label Oct 20, 2017
@TimNN TimNN added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Oct 22, 2017
@estebank estebank added the WG-diagnostics Working group: Diagnostics label Oct 31, 2017
@estebank
Copy link
Contributor

Current output:

warning: variable does not need to be mutable
 --> src/main.rs:3:9
  |
3 |     let mut reader = &mut mem_buffer as &std::io::Read;
  |         ----^^^^^^
  |         |
  |         help: remove this `mut`
  |
  = note: #[warn(unused_mut)] on by default

error[E0596]: cannot borrow `*reader` as mutable, as it is behind a `&` reference
 --> src/main.rs:5:5
  |
3 |     let mut reader = &mut mem_buffer as &std::io::Read;
  |                      --------------------------------- help: consider changing this to be a mutable reference: `&mut mut mem_buffer as &std::io::Read`
4 |     let mut read_buffer = [0u8, 10];
5 |     reader.read(&mut read_buffer);
  |     ^^^^^^ `reader` is a `&` reference, so the data it refers to cannot be borrowed as mutable

Applying the suggestion is (correctly) a parse error. We need to change that suggestion to find the appropriate place for the mut: it should be &mut mem_buffer as &mut std::io::Read.

@estebank estebank added D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 14, 2020
@estebank
Copy link
Contributor

estebank commented Feb 3, 2023

Current output:

error[E0596]: cannot borrow `*reader` as mutable, as it is behind a `&` reference
 --> src/main.rs:5:5
  |
3 |     let mut reader = &mut mem_buffer as &dyn std::io::Read;
  |         ---------- consider changing this binding's type to be: `&mut dyn std::io::Read`
4 |     let mut read_buffer = [0u8, 10];
5 |     reader.read(&mut read_buffer);
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `reader` is a `&` reference, so the data it refers to cannot be borrowed as mutable

estebank added a commit to estebank/rust that referenced this issue Feb 3, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 4, 2023
…-errors

Provide structured suggestion for binding needing type on E0594

Partially address rust-lang#45405.
@estebank
Copy link
Contributor

estebank commented May 2, 2024

Current output:

error[E0596]: cannot borrow `*reader` as mutable, as it is behind a `&` reference
 --> src/main.rs:5:5
  |
5 |     reader.read(&mut read_buffer);
  |     ^^^^^^ `reader` is a `&` reference, so the data it refers to cannot be borrowed as mutable
  |
help: consider specifying this binding's type
  |
3 |     let mut reader: &mut dyn std::io::Read = &mut mem_buffer as &dyn std::io::Read;
  |                   ++++++++++++++++++++++++

Applying the suggestion we get

error[E0308]: mismatched types
 --> src/main.rs:3:46
  |
3 |     let mut reader: &mut dyn std::io::Read = &mut mem_buffer as &dyn std::io::Read;
  |                     ----------------------   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ types differ in mutability
  |                     |
  |                     expected due to this
  |
  = note: expected mutable reference `&mut dyn std::io::Read`
                     found reference `&dyn std::io::Read`

Only thing left is to detect when an E0308 is pointing at an as casting expression and point only at the type part of it, and opportunistically suggest changing it to the right type. Having said that, I'm satisfied with the current output.

@estebank estebank added P-low Low priority and removed D-papercut Diagnostics: An error or lint that needs small tweaks. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. labels May 2, 2024
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. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-diagnostics Working group: Diagnostics
Projects
None yet
Development

No branches or pull requests

4 participants