Skip to content

Commit ad0729e

Browse files
UrgauUrgau
committed
Improve diagnostic for wrong borrow on binary operations
1 parent 0441150 commit ad0729e

File tree

6 files changed

+326
-31
lines changed

6 files changed

+326
-31
lines changed

‎compiler/rustc_hir_typeck/src/op.rs‎

Lines changed: 136 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use super::method::MethodCallee;
44
use super::{has_expected_num_generic_args, FnCtxt};
55
use crate::Expectation;
66
use rustc_ast as ast;
7-
use rustc_errors::{self, struct_span_err, Applicability, Diagnostic};
7+
use rustc_errors::{self, struct_span_err, Applicability, Diagnostic, DiagnosticBuilder};
88
use rustc_hir as hir;
99
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
1010
use rustc_infer::traits::ObligationCauseCode;
@@ -380,33 +380,93 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
380380
}
381381
};
382382

383-
let mut suggest_deref_binop = |lhs_deref_ty: Ty<'tcx>| {
384-
if self
385-
.lookup_op_method(
386-
lhs_deref_ty,
387-
Some((rhs_expr, rhs_ty)),
388-
Op::Binary(op, is_assign),
389-
expected,
390-
)
391-
.is_ok()
392-
{
393-
let msg = format!(
394-
"`{}{}` can be used on `{}` if you dereference the left-hand side",
395-
op.node.as_str(),
396-
match is_assign {
397-
IsAssign::Yes => "=",
398-
IsAssign::No => "",
399-
},
400-
lhs_deref_ty,
401-
);
402-
err.span_suggestion_verbose(
403-
lhs_expr.span.shrink_to_lo(),
404-
msg,
405-
"*",
406-
rustc_errors::Applicability::MachineApplicable,
407-
);
408-
}
409-
};
383+
let suggest_deref_binop =
384+
|err: &mut DiagnosticBuilder<'_, _>, lhs_deref_ty: Ty<'tcx>| {
385+
if self
386+
.lookup_op_method(
387+
lhs_deref_ty,
388+
Some((rhs_expr, rhs_ty)),
389+
Op::Binary(op, is_assign),
390+
expected,
391+
)
392+
.is_ok()
393+
{
394+
let msg = format!(
395+
"`{}{}` can be used on `{}` if you dereference the left-hand side",
396+
op.node.as_str(),
397+
match is_assign {
398+
IsAssign::Yes => "=",
399+
IsAssign::No => "",
400+
},
401+
lhs_deref_ty,
402+
);
403+
err.span_suggestion_verbose(
404+
lhs_expr.span.shrink_to_lo(),
405+
msg,
406+
"*",
407+
rustc_errors::Applicability::MachineApplicable,
408+
);
409+
}
410+
};
411+
412+
let suggest_different_borrow =
413+
|err: &mut DiagnosticBuilder<'_, _>,
414+
lhs_adjusted_ty,
415+
lhs_new_mutbl: Option<ast::Mutability>,
416+
rhs_adjusted_ty,
417+
rhs_new_mutbl: Option<ast::Mutability>| {
418+
if self
419+
.lookup_op_method(
420+
lhs_adjusted_ty,
421+
Some((rhs_expr, rhs_adjusted_ty)),
422+
Op::Binary(op, is_assign),
423+
expected,
424+
)
425+
.is_ok()
426+
{
427+
let op_str = op.node.as_str();
428+
err.note(format!("an implementation for `{lhs_adjusted_ty} {op_str} {rhs_adjusted_ty}` exists"));
429+
430+
if let Some(lhs_new_mutbl) = lhs_new_mutbl
431+
&& let Some(rhs_new_mutbl) = rhs_new_mutbl
432+
&& lhs_new_mutbl.is_not()
433+
&& rhs_new_mutbl.is_not() {
434+
err.multipart_suggestion_verbose(
435+
"consider reborrowing both sides",
436+
vec![
437+
(lhs_expr.span.shrink_to_lo(), "&*".to_string()),
438+
(rhs_expr.span.shrink_to_lo(), "&*".to_string())
439+
],
440+
rustc_errors::Applicability::MachineApplicable,
441+
);
442+
} else {
443+
let mut suggest_new_borrow = |new_mutbl: ast::Mutability, sp: Span| {
444+
// Can reborrow (&mut -> &)
445+
if new_mutbl.is_not() {
446+
err.span_suggestion_verbose(
447+
sp.shrink_to_lo(),
448+
"consider reborrowing this side",
449+
"&*",
450+
rustc_errors::Applicability::MachineApplicable,
451+
);
452+
// Works on &mut but have &
453+
} else {
454+
err.span_help(
455+
sp,
456+
"consider making this expression a mutable borrow",
457+
);
458+
}
459+
};
460+
461+
if let Some(lhs_new_mutbl) = lhs_new_mutbl {
462+
suggest_new_borrow(lhs_new_mutbl, lhs_expr.span);
463+
}
464+
if let Some(rhs_new_mutbl) = rhs_new_mutbl {
465+
suggest_new_borrow(rhs_new_mutbl, rhs_expr.span);
466+
}
467+
}
468+
}
469+
};
410470

411471
let is_compatible_after_call = |lhs_ty, rhs_ty| {
412472
self.lookup_op_method(
@@ -429,15 +489,60 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
429489
} else if is_assign == IsAssign::Yes
430490
&& let Some(lhs_deref_ty) = self.deref_once_mutably_for_diagnostic(lhs_ty)
431491
{
432-
suggest_deref_binop(lhs_deref_ty);
492+
suggest_deref_binop(&mut err, lhs_deref_ty);
433493
} else if is_assign == IsAssign::No
434-
&& let Ref(_, lhs_deref_ty, _) = lhs_ty.kind()
494+
&& let Ref(region, lhs_deref_ty, mutbl) = lhs_ty.kind()
435495
{
436496
if self.type_is_copy_modulo_regions(
437497
self.param_env,
438498
*lhs_deref_ty,
439499
) {
440-
suggest_deref_binop(*lhs_deref_ty);
500+
suggest_deref_binop(&mut err, *lhs_deref_ty);
501+
} else {
502+
let lhs_inv_mutbl = mutbl.invert();
503+
let lhs_inv_mutbl_ty = Ty::new_ref(
504+
self.tcx,
505+
*region,
506+
ty::TypeAndMut {
507+
ty: *lhs_deref_ty,
508+
mutbl: lhs_inv_mutbl,
509+
},
510+
);
511+
512+
suggest_different_borrow(
513+
&mut err,
514+
lhs_inv_mutbl_ty,
515+
Some(lhs_inv_mutbl),
516+
rhs_ty,
517+
None,
518+
);
519+
520+
if let Ref(region, rhs_deref_ty, mutbl) = rhs_ty.kind() {
521+
let rhs_inv_mutbl = mutbl.invert();
522+
let rhs_inv_mutbl_ty = Ty::new_ref(
523+
self.tcx,
524+
*region,
525+
ty::TypeAndMut {
526+
ty: *rhs_deref_ty,
527+
mutbl: rhs_inv_mutbl,
528+
},
529+
);
530+
531+
suggest_different_borrow(
532+
&mut err,
533+
lhs_ty,
534+
None,
535+
rhs_inv_mutbl_ty,
536+
Some(rhs_inv_mutbl),
537+
);
538+
suggest_different_borrow(
539+
&mut err,
540+
lhs_inv_mutbl_ty,
541+
Some(lhs_inv_mutbl),
542+
rhs_inv_mutbl_ty,
543+
Some(rhs_inv_mutbl),
544+
);
545+
}
441546
}
442547
} else if self.suggest_fn_call(&mut err, lhs_expr, lhs_ty, |lhs_ty| {
443548
is_compatible_after_call(lhs_ty, rhs_ty)

‎tests/ui/binop/borrow-suggestion-109352-2.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
struct Bar;
2+
3+
impl std::ops::Mul for &mut Bar {
4+
type Output = Bar;
5+
6+
fn mul(self, _rhs: Self) -> Self::Output {
7+
unimplemented!()
8+
}
9+
}
10+
11+
fn main() {
12+
let ref_mut_bar: &mut Bar = &mut Bar;
13+
let ref_bar: &Bar = &Bar;
14+
let owned_bar: Bar = Bar;
15+
16+
let _ = ref_mut_bar * ref_mut_bar;
17+
18+
// FIXME: we should be able to suggest borrowing both side
19+
let _ = owned_bar * owned_bar;
20+
//~^ ERROR cannot multiply
21+
let _ = ref_bar * ref_bar;
22+
//~^ ERROR cannot multiply
23+
let _ = ref_bar * ref_mut_bar;
24+
//~^ ERROR cannot multiply
25+
let _ = ref_mut_bar * ref_bar;
26+
//~^ ERROR mismatched types
27+
}

‎tests/ui/binop/borrow-suggestion-109352-2.stderr‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
error[E0369]: cannot multiply `Bar` by `Bar`
2+
--> $DIR/borrow-suggestion-109352-2.rs:19:23
3+
|
4+
LL | let _ = owned_bar * owned_bar;
5+
| --------- ^ --------- Bar
6+
| |
7+
| Bar
8+
|
9+
note: an implementation of `Mul` might be missing for `Bar`
10+
--> $DIR/borrow-suggestion-109352-2.rs:1:1
11+
|
12+
LL | struct Bar;
13+
| ^^^^^^^^^^ must implement `Mul`
14+
note: the trait `Mul` must be implemented
15+
--> $SRC_DIR/core/src/ops/arith.rs:LL:COL
16+
17+
error[E0369]: cannot multiply `&Bar` by `&Bar`
18+
--> $DIR/borrow-suggestion-109352-2.rs:21:21
19+
|
20+
LL | let _ = ref_bar * ref_bar;
21+
| ------- ^ ------- &Bar
22+
| |
23+
| &Bar
24+
|
25+
= note: an implementation for `&mut Bar * &mut Bar` exists
26+
help: consider making this expression a mutable borrow
27+
--> $DIR/borrow-suggestion-109352-2.rs:21:13
28+
|
29+
LL | let _ = ref_bar * ref_bar;
30+
| ^^^^^^^
31+
help: consider making this expression a mutable borrow
32+
--> $DIR/borrow-suggestion-109352-2.rs:21:23
33+
|
34+
LL | let _ = ref_bar * ref_bar;
35+
| ^^^^^^^
36+
37+
error[E0369]: cannot multiply `&Bar` by `&mut Bar`
38+
--> $DIR/borrow-suggestion-109352-2.rs:23:21
39+
|
40+
LL | let _ = ref_bar * ref_mut_bar;
41+
| ------- ^ ----------- &mut Bar
42+
| |
43+
| &Bar
44+
|
45+
= note: an implementation for `&mut Bar * &mut Bar` exists
46+
help: consider making this expression a mutable borrow
47+
--> $DIR/borrow-suggestion-109352-2.rs:23:13
48+
|
49+
LL | let _ = ref_bar * ref_mut_bar;
50+
| ^^^^^^^
51+
52+
error[E0308]: mismatched types
53+
--> $DIR/borrow-suggestion-109352-2.rs:25:27
54+
|
55+
LL | let _ = ref_mut_bar * ref_bar;
56+
| ^^^^^^^ types differ in mutability
57+
|
58+
= note: expected mutable reference `&mut Bar`
59+
found reference `&Bar`
60+
61+
error: aborting due to 4 previous errors
62+
63+
Some errors have detailed explanations: E0308, E0369.
64+
For more information about an error, try `rustc --explain E0308`.

‎tests/ui/binop/borrow-suggestion-109352.fixed‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// run-rustfix
2+
3+
struct Foo;
4+
5+
impl std::ops::Mul for &Foo {
6+
type Output = Foo;
7+
8+
fn mul(self, _rhs: Self) -> Self::Output {
9+
unimplemented!()
10+
}
11+
}
12+
13+
fn main() {
14+
let ref_mut_foo: &mut Foo = &mut Foo;
15+
let ref_foo: &Foo = &Foo;
16+
let owned_foo: Foo = Foo;
17+
18+
let _ = ref_foo * ref_foo;
19+
let _ = ref_foo * ref_mut_foo;
20+
21+
let _ = &*ref_mut_foo * ref_foo;
22+
//~^ ERROR cannot multiply
23+
let _ = &*ref_mut_foo * &*ref_mut_foo;
24+
//~^ ERROR cannot multiply
25+
let _ = &*ref_mut_foo * &owned_foo;
26+
//~^ ERROR cannot multiply
27+
}

‎tests/ui/binop/borrow-suggestion-109352.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// run-rustfix
2+
3+
struct Foo;
4+
5+
impl std::ops::Mul for &Foo {
6+
type Output = Foo;
7+
8+
fn mul(self, _rhs: Self) -> Self::Output {
9+
unimplemented!()
10+
}
11+
}
12+
13+
fn main() {
14+
let ref_mut_foo: &mut Foo = &mut Foo;
15+
let ref_foo: &Foo = &Foo;
16+
let owned_foo: Foo = Foo;
17+
18+
let _ = ref_foo * ref_foo;
19+
let _ = ref_foo * ref_mut_foo;
20+
21+
let _ = ref_mut_foo * ref_foo;
22+
//~^ ERROR cannot multiply
23+
let _ = ref_mut_foo * ref_mut_foo;
24+
//~^ ERROR cannot multiply
25+
let _ = ref_mut_foo * &owned_foo;
26+
//~^ ERROR cannot multiply
27+
}

‎tests/ui/binop/borrow-suggestion-109352.stderr‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
error[E0369]: cannot multiply `&mut Foo` by `&Foo`
2+
--> $DIR/borrow-suggestion-109352.rs:21:25
3+
|
4+
LL | let _ = ref_mut_foo * ref_foo;
5+
| ----------- ^ ------- &Foo
6+
| |
7+
| &mut Foo
8+
|
9+
= note: an implementation for `&Foo * &Foo` exists
10+
help: consider reborrowing this side
11+
|
12+
LL | let _ = &*ref_mut_foo * ref_foo;
13+
| ++
14+
15+
error[E0369]: cannot multiply `&mut Foo` by `&mut Foo`
16+
--> $DIR/borrow-suggestion-109352.rs:23:25
17+
|
18+
LL | let _ = ref_mut_foo * ref_mut_foo;
19+
| ----------- ^ ----------- &mut Foo
20+
| |
21+
| &mut Foo
22+
|
23+
= note: an implementation for `&Foo * &Foo` exists
24+
help: consider reborrowing both sides
25+
|
26+
LL | let _ = &*ref_mut_foo * &*ref_mut_foo;
27+
| ++ ++
28+
29+
error[E0369]: cannot multiply `&mut Foo` by `&Foo`
30+
--> $DIR/borrow-suggestion-109352.rs:25:25
31+
|
32+
LL | let _ = ref_mut_foo * &owned_foo;
33+
| ----------- ^ ---------- &Foo
34+
| |
35+
| &mut Foo
36+
|
37+
= note: an implementation for `&Foo * &Foo` exists
38+
help: consider reborrowing this side
39+
|
40+
LL | let _ = &*ref_mut_foo * &owned_foo;
41+
| ++
42+
43+
error: aborting due to 3 previous errors
44+
45+
For more information about this error, try `rustc --explain E0369`.

0 commit comments

Comments
 (0)