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

Idea: suggest coercing to &T from &mut T if it satisfies type check #109352

Closed
ErichDonGubler opened this issue Mar 19, 2023 · 3 comments · Fixed by #114288
Closed

Idea: suggest coercing to &T from &mut T if it satisfies type check #109352

ErichDonGubler opened this issue Mar 19, 2023 · 3 comments · Fixed by #114288
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ErichDonGubler
Copy link
Contributor

ErichDonGubler commented Mar 19, 2023

Finally getting around to filing this issue at @estebank's encouragement in this (rather embarrassing) Reddit thread. CC @WalterSmuts and @pali6.

Code

I've tried to capture things in this Playground link; to summarize:

struct Foo;

impl std::ops::Mul for &Foo {
    type Output = Foo;

    fn mul(self, _rhs: Self) -> Self::Output {
        unimplemented!()
    }
}

#[test]
fn it_works() {
    let ref_mut_foo: &mut Foo = &mut Foo;
    let ref_foo: &Foo = &Foo;
    let owned_foo: Foo = Foo;
    
    // The following assignments cause compiler errors that can be resolved by
    // getting immutable borrows of `ref_mut_foo`'s pointee (i.e.,
    // `&*ref_mut_foo`):
    let _ = ref_mut_foo * ref_foo;
    let _ = ref_mut_foo * ref_mut_foo;
    let _ = ref_mut_foo * &owned_foo;
    // …but we currently miss the opportunity to inform a user of the
    // possibility (😢).
}

Current output

error[E0369]: cannot multiply `&mut Foo` by `&Foo`
  --> src/lib.rs:20:25
   |
20 |     let _ = ref_mut_foo * ref_foo;
   |             ----------- ^ ------- &Foo
   |             |
   |             &mut Foo

error[E0369]: cannot multiply `&mut Foo` by `&mut Foo`
  --> src/lib.rs:21:25
   |
21 |     let _ = ref_mut_foo * ref_mut_foo;
   |             ----------- ^ ----------- &mut Foo
   |             |
   |             &mut Foo

error[E0369]: cannot multiply `&mut Foo` by `&Foo`
  --> src/lib.rs:22:25
   |
22 |     let _ = ref_mut_foo * &owned_foo;
   |             ----------- ^ ---------- &Foo
   |             |
   |             &mut Foo

Desired output

error[E0369]: cannot multiply `&mut Foo` by `&Foo`
  --> src/lib.rs:20:25
   |
20 |     let _ = ref_mut_foo * ref_foo;
   |             ----------- ^ ------- &Foo
   |             |
   |             &mut Foo
   |
   = help: consider re-borrowing here: `&*ref_mut_foo * ref_foo`

error[E0369]: cannot multiply `&mut Foo` by `&mut Foo`
  --> src/lib.rs:21:25
   |
21 |     let _ = ref_mut_foo * ref_mut_foo;
   |             ----------- ^ ----------- &mut Foo
   |             |
   |             &mut Foo
   |
   = help: consider re-borrowing here: `&*ref_mut_foo * &*ref_mut_foo`

error[E0369]: cannot multiply `&mut Foo` by `&Foo`
  --> src/lib.rs:22:25
   |
22 |     let _ = ref_mut_foo * &owned_foo;
   |             ----------- ^ ---------- &Foo
   |             |
   |             &mut Foo
   |
   = help: consider re-borrowing here: `&*ref_mut_foo * &owned_foo`

Rationale and extra context

It seems like this should generally be a valid suggestion, and we have to create a diagnostic for this; current error output doesn't help the user to discover the "right" solution at all.

Other cases

No response

Anything else?

No response

@ErichDonGubler ErichDonGubler added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 19, 2023
@WalterSmuts
Copy link

Hi Erich, thanks for tagging me :) Took me a couple of minutes to refresh my memory 😅 but glad to see there is something positive coming from that thread :)

Should the suggested help-message not read:

help: consider re-borrowing here: `&*ref_mut_foo

Since we're not just adding a & but rather a &*?

@ErichDonGubler
Copy link
Contributor Author

ErichDonGubler commented Mar 20, 2023

@WalterSmuts:

Should the suggested help-message not read:

help: consider re-borrowing here: `&*ref_mut_foo

Since we're not just adding a & but rather a &*?

I think your point is valid. Edited!

@workingjubilee workingjubilee added the A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` label Mar 20, 2023
@bors bors closed this as completed in c726dcb Aug 1, 2023
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Aug 2, 2023
Improve diagnostic for wrong borrow on binary operations

This PR improves the diagnostic for wrong borrow on binary operations by suggesting to reborrow on appropriate expressions.

```diff
+    = note: an implementation for `&Foo * &Foo` exist
+ help: consider reborrowing both sides
+    |
+ LL |     let _ = &*ref_mut_foo * &*ref_mut_foo;
+    |             ++              ++
```

Fixes rust-lang/rust#109352
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 A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` 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.

3 participants