Rollup merge of #114288 - Urgau:fix-issue-109352, r=b-naber

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 https://github.com/rust-lang/rust/issues/109352
This commit is contained in:
Matthias Krüger 2023-08-01 17:39:11 +02:00 committed by GitHub
commit c726dcb962
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 326 additions and 31 deletions

View File

@ -4,7 +4,7 @@ use super::method::MethodCallee;
use super::{has_expected_num_generic_args, FnCtxt};
use crate::Expectation;
use rustc_ast as ast;
use rustc_errors::{self, struct_span_err, Applicability, Diagnostic};
use rustc_errors::{self, struct_span_err, Applicability, Diagnostic, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_infer::traits::ObligationCauseCode;
@ -380,33 +380,93 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
};
let mut suggest_deref_binop = |lhs_deref_ty: Ty<'tcx>| {
if self
.lookup_op_method(
lhs_deref_ty,
Some((rhs_expr, rhs_ty)),
Op::Binary(op, is_assign),
expected,
)
.is_ok()
{
let msg = format!(
"`{}{}` can be used on `{}` if you dereference the left-hand side",
op.node.as_str(),
match is_assign {
IsAssign::Yes => "=",
IsAssign::No => "",
},
lhs_deref_ty,
);
err.span_suggestion_verbose(
lhs_expr.span.shrink_to_lo(),
msg,
"*",
rustc_errors::Applicability::MachineApplicable,
);
}
};
let suggest_deref_binop =
|err: &mut DiagnosticBuilder<'_, _>, lhs_deref_ty: Ty<'tcx>| {
if self
.lookup_op_method(
lhs_deref_ty,
Some((rhs_expr, rhs_ty)),
Op::Binary(op, is_assign),
expected,
)
.is_ok()
{
let msg = format!(
"`{}{}` can be used on `{}` if you dereference the left-hand side",
op.node.as_str(),
match is_assign {
IsAssign::Yes => "=",
IsAssign::No => "",
},
lhs_deref_ty,
);
err.span_suggestion_verbose(
lhs_expr.span.shrink_to_lo(),
msg,
"*",
rustc_errors::Applicability::MachineApplicable,
);
}
};
let suggest_different_borrow =
|err: &mut DiagnosticBuilder<'_, _>,
lhs_adjusted_ty,
lhs_new_mutbl: Option<ast::Mutability>,
rhs_adjusted_ty,
rhs_new_mutbl: Option<ast::Mutability>| {
if self
.lookup_op_method(
lhs_adjusted_ty,
Some((rhs_expr, rhs_adjusted_ty)),
Op::Binary(op, is_assign),
expected,
)
.is_ok()
{
let op_str = op.node.as_str();
err.note(format!("an implementation for `{lhs_adjusted_ty} {op_str} {rhs_adjusted_ty}` exists"));
if let Some(lhs_new_mutbl) = lhs_new_mutbl
&& let Some(rhs_new_mutbl) = rhs_new_mutbl
&& lhs_new_mutbl.is_not()
&& rhs_new_mutbl.is_not() {
err.multipart_suggestion_verbose(
"consider reborrowing both sides",
vec![
(lhs_expr.span.shrink_to_lo(), "&*".to_string()),
(rhs_expr.span.shrink_to_lo(), "&*".to_string())
],
rustc_errors::Applicability::MachineApplicable,
);
} else {
let mut suggest_new_borrow = |new_mutbl: ast::Mutability, sp: Span| {
// Can reborrow (&mut -> &)
if new_mutbl.is_not() {
err.span_suggestion_verbose(
sp.shrink_to_lo(),
"consider reborrowing this side",
"&*",
rustc_errors::Applicability::MachineApplicable,
);
// Works on &mut but have &
} else {
err.span_help(
sp,
"consider making this expression a mutable borrow",
);
}
};
if let Some(lhs_new_mutbl) = lhs_new_mutbl {
suggest_new_borrow(lhs_new_mutbl, lhs_expr.span);
}
if let Some(rhs_new_mutbl) = rhs_new_mutbl {
suggest_new_borrow(rhs_new_mutbl, rhs_expr.span);
}
}
}
};
let is_compatible_after_call = |lhs_ty, rhs_ty| {
self.lookup_op_method(
@ -429,15 +489,60 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
} else if is_assign == IsAssign::Yes
&& let Some(lhs_deref_ty) = self.deref_once_mutably_for_diagnostic(lhs_ty)
{
suggest_deref_binop(lhs_deref_ty);
suggest_deref_binop(&mut err, lhs_deref_ty);
} else if is_assign == IsAssign::No
&& let Ref(_, lhs_deref_ty, _) = lhs_ty.kind()
&& let Ref(region, lhs_deref_ty, mutbl) = lhs_ty.kind()
{
if self.type_is_copy_modulo_regions(
self.param_env,
*lhs_deref_ty,
) {
suggest_deref_binop(*lhs_deref_ty);
suggest_deref_binop(&mut err, *lhs_deref_ty);
} else {
let lhs_inv_mutbl = mutbl.invert();
let lhs_inv_mutbl_ty = Ty::new_ref(
self.tcx,
*region,
ty::TypeAndMut {
ty: *lhs_deref_ty,
mutbl: lhs_inv_mutbl,
},
);
suggest_different_borrow(
&mut err,
lhs_inv_mutbl_ty,
Some(lhs_inv_mutbl),
rhs_ty,
None,
);
if let Ref(region, rhs_deref_ty, mutbl) = rhs_ty.kind() {
let rhs_inv_mutbl = mutbl.invert();
let rhs_inv_mutbl_ty = Ty::new_ref(
self.tcx,
*region,
ty::TypeAndMut {
ty: *rhs_deref_ty,
mutbl: rhs_inv_mutbl,
},
);
suggest_different_borrow(
&mut err,
lhs_ty,
None,
rhs_inv_mutbl_ty,
Some(rhs_inv_mutbl),
);
suggest_different_borrow(
&mut err,
lhs_inv_mutbl_ty,
Some(lhs_inv_mutbl),
rhs_inv_mutbl_ty,
Some(rhs_inv_mutbl),
);
}
}
} else if self.suggest_fn_call(&mut err, lhs_expr, lhs_ty, |lhs_ty| {
is_compatible_after_call(lhs_ty, rhs_ty)

View File

@ -0,0 +1,27 @@
struct Bar;
impl std::ops::Mul for &mut Bar {
type Output = Bar;
fn mul(self, _rhs: Self) -> Self::Output {
unimplemented!()
}
}
fn main() {
let ref_mut_bar: &mut Bar = &mut Bar;
let ref_bar: &Bar = &Bar;
let owned_bar: Bar = Bar;
let _ = ref_mut_bar * ref_mut_bar;
// FIXME: we should be able to suggest borrowing both side
let _ = owned_bar * owned_bar;
//~^ ERROR cannot multiply
let _ = ref_bar * ref_bar;
//~^ ERROR cannot multiply
let _ = ref_bar * ref_mut_bar;
//~^ ERROR cannot multiply
let _ = ref_mut_bar * ref_bar;
//~^ ERROR mismatched types
}

View File

@ -0,0 +1,64 @@
error[E0369]: cannot multiply `Bar` by `Bar`
--> $DIR/borrow-suggestion-109352-2.rs:19:23
|
LL | let _ = owned_bar * owned_bar;
| --------- ^ --------- Bar
| |
| Bar
|
note: an implementation of `Mul` might be missing for `Bar`
--> $DIR/borrow-suggestion-109352-2.rs:1:1
|
LL | struct Bar;
| ^^^^^^^^^^ must implement `Mul`
note: the trait `Mul` must be implemented
--> $SRC_DIR/core/src/ops/arith.rs:LL:COL
error[E0369]: cannot multiply `&Bar` by `&Bar`
--> $DIR/borrow-suggestion-109352-2.rs:21:21
|
LL | let _ = ref_bar * ref_bar;
| ------- ^ ------- &Bar
| |
| &Bar
|
= note: an implementation for `&mut Bar * &mut Bar` exists
help: consider making this expression a mutable borrow
--> $DIR/borrow-suggestion-109352-2.rs:21:13
|
LL | let _ = ref_bar * ref_bar;
| ^^^^^^^
help: consider making this expression a mutable borrow
--> $DIR/borrow-suggestion-109352-2.rs:21:23
|
LL | let _ = ref_bar * ref_bar;
| ^^^^^^^
error[E0369]: cannot multiply `&Bar` by `&mut Bar`
--> $DIR/borrow-suggestion-109352-2.rs:23:21
|
LL | let _ = ref_bar * ref_mut_bar;
| ------- ^ ----------- &mut Bar
| |
| &Bar
|
= note: an implementation for `&mut Bar * &mut Bar` exists
help: consider making this expression a mutable borrow
--> $DIR/borrow-suggestion-109352-2.rs:23:13
|
LL | let _ = ref_bar * ref_mut_bar;
| ^^^^^^^
error[E0308]: mismatched types
--> $DIR/borrow-suggestion-109352-2.rs:25:27
|
LL | let _ = ref_mut_bar * ref_bar;
| ^^^^^^^ types differ in mutability
|
= note: expected mutable reference `&mut Bar`
found reference `&Bar`
error: aborting due to 4 previous errors
Some errors have detailed explanations: E0308, E0369.
For more information about an error, try `rustc --explain E0308`.

View File

@ -0,0 +1,27 @@
// run-rustfix
struct Foo;
impl std::ops::Mul for &Foo {
type Output = Foo;
fn mul(self, _rhs: Self) -> Self::Output {
unimplemented!()
}
}
fn main() {
let ref_mut_foo: &mut Foo = &mut Foo;
let ref_foo: &Foo = &Foo;
let owned_foo: Foo = Foo;
let _ = ref_foo * ref_foo;
let _ = ref_foo * ref_mut_foo;
let _ = &*ref_mut_foo * ref_foo;
//~^ ERROR cannot multiply
let _ = &*ref_mut_foo * &*ref_mut_foo;
//~^ ERROR cannot multiply
let _ = &*ref_mut_foo * &owned_foo;
//~^ ERROR cannot multiply
}

View File

@ -0,0 +1,27 @@
// run-rustfix
struct Foo;
impl std::ops::Mul for &Foo {
type Output = Foo;
fn mul(self, _rhs: Self) -> Self::Output {
unimplemented!()
}
}
fn main() {
let ref_mut_foo: &mut Foo = &mut Foo;
let ref_foo: &Foo = &Foo;
let owned_foo: Foo = Foo;
let _ = ref_foo * ref_foo;
let _ = ref_foo * ref_mut_foo;
let _ = ref_mut_foo * ref_foo;
//~^ ERROR cannot multiply
let _ = ref_mut_foo * ref_mut_foo;
//~^ ERROR cannot multiply
let _ = ref_mut_foo * &owned_foo;
//~^ ERROR cannot multiply
}

View File

@ -0,0 +1,45 @@
error[E0369]: cannot multiply `&mut Foo` by `&Foo`
--> $DIR/borrow-suggestion-109352.rs:21:25
|
LL | let _ = ref_mut_foo * ref_foo;
| ----------- ^ ------- &Foo
| |
| &mut Foo
|
= note: an implementation for `&Foo * &Foo` exists
help: consider reborrowing this side
|
LL | let _ = &*ref_mut_foo * ref_foo;
| ++
error[E0369]: cannot multiply `&mut Foo` by `&mut Foo`
--> $DIR/borrow-suggestion-109352.rs:23:25
|
LL | let _ = ref_mut_foo * ref_mut_foo;
| ----------- ^ ----------- &mut Foo
| |
| &mut Foo
|
= note: an implementation for `&Foo * &Foo` exists
help: consider reborrowing both sides
|
LL | let _ = &*ref_mut_foo * &*ref_mut_foo;
| ++ ++
error[E0369]: cannot multiply `&mut Foo` by `&Foo`
--> $DIR/borrow-suggestion-109352.rs:25:25
|
LL | let _ = ref_mut_foo * &owned_foo;
| ----------- ^ ---------- &Foo
| |
| &mut Foo
|
= note: an implementation for `&Foo * &Foo` exists
help: consider reborrowing this side
|
LL | let _ = &*ref_mut_foo * &owned_foo;
| ++
error: aborting due to 3 previous errors
For more information about this error, try `rustc --explain E0369`.