Skip to content

Commit 5aab1a9

Browse files
estebankestebank
committed
Tweak binop errors
* Suggest potentially missing binop trait bound (fix #73416) * Use structured suggestion for dereference in binop
1 parent a39c778 commit 5aab1a9

10 files changed

+169
-40
lines changed

‎src/librustc_typeck/check/op.rs‎

Lines changed: 98 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ use rustc_middle::ty::adjustment::{
99
Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability,
1010
};
1111
use rustc_middle::ty::TyKind::{Adt, Array, Char, FnDef, Never, Ref, Str, Tuple, Uint};
12-
use rustc_middle::ty::{self, suggest_constraining_type_param, Ty, TyCtxt, TypeFoldable};
12+
use rustc_middle::ty::{
13+
self, suggest_constraining_type_param, Ty, TyCtxt, TypeFoldable, TypeVisitor,
14+
};
1315
use rustc_span::symbol::Ident;
1416
use rustc_span::Span;
1517
use rustc_trait_selection::infer::InferCtxtExt;
@@ -254,6 +256,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
254256
if !lhs_ty.references_error() && !rhs_ty.references_error() {
255257
let source_map = self.tcx.sess.source_map();
256258

259+
let note = |err: &mut DiagnosticBuilder<'_>, missing_trait| {
260+
err.note(&format!(
261+
"the trait `{}` is not implemented for `{}`",
262+
missing_trait, lhs_ty
263+
));
264+
};
257265
match is_assign {
258266
IsAssign::Yes => {
259267
let mut err = struct_span_err!(
@@ -286,10 +294,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
286294
rty.peel_refs(),
287295
lstring,
288296
);
289-
err.span_suggestion(
290-
lhs_expr.span,
297+
err.span_suggestion_verbose(
298+
lhs_expr.span.shrink_to_lo(),
291299
msg,
292-
format!("*{}", lstring),
300+
"*".to_string(),
293301
rustc_errors::Applicability::MachineApplicable,
294302
);
295303
suggested_deref = true;
@@ -310,6 +318,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
310318
_ => None,
311319
};
312320
if let Some(missing_trait) = missing_trait {
321+
let mut visitor = TypeParamVisitor(vec![]);
322+
visitor.visit_ty(lhs_ty);
323+
324+
let mut sugg = false;
313325
if op.node == hir::BinOpKind::Add
314326
&& self.check_str_addition(
315327
lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, true, op,
@@ -318,18 +330,33 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
318330
// This has nothing here because it means we did string
319331
// concatenation (e.g., "Hello " += "World!"). This means
320332
// we don't want the note in the else clause to be emitted
321-
} else if let ty::Param(p) = lhs_ty.kind {
322-
suggest_constraining_param(
323-
self.tcx,
324-
self.body_id,
325-
&mut err,
326-
lhs_ty,
327-
rhs_ty,
328-
missing_trait,
329-
p,
330-
false,
331-
);
332-
} else if !suggested_deref {
333+
sugg = true;
334+
} else if let [ty] = &visitor.0[..] {
335+
if let ty::Param(p) = ty.kind {
336+
// FIXME: This *guesses* that constraining the type param
337+
// will make the operation available, but this is only true
338+
// when the corresponding trait has a blanked
339+
// implementation, like the following:
340+
// `impl<'a> PartialEq for &'a [T] where T: PartialEq {}`
341+
// The correct thing to do would be to verify this
342+
// projection would hold.
343+
if *ty != lhs_ty {
344+
note(&mut err, missing_trait);
345+
}
346+
suggest_constraining_param(
347+
self.tcx,
348+
self.body_id,
349+
&mut err,
350+
ty,
351+
rhs_ty,
352+
missing_trait,
353+
p,
354+
false,
355+
);
356+
sugg = true;
357+
}
358+
}
359+
if !sugg && !suggested_deref {
333360
suggest_impl_missing(&mut err, lhs_ty, &missing_trait);
334361
}
335362
}
@@ -458,18 +485,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
458485
.is_ok()
459486
} {
460487
if let Ok(lstring) = source_map.span_to_snippet(lhs_expr.span) {
461-
err.help(&format!(
462-
"`{}` can be used on '{}', you can \
463-
dereference `{2}`: `*{2}`",
464-
op.node.as_str(),
465-
rty.peel_refs(),
466-
lstring
467-
));
488+
err.span_suggestion_verbose(
489+
lhs_expr.span.shrink_to_lo(),
490+
&format!(
491+
"`{}` can be used on `{}`, you can dereference \
492+
`{}`",
493+
op.node.as_str(),
494+
rty.peel_refs(),
495+
lstring,
496+
),
497+
"*".to_string(),
498+
Applicability::MachineApplicable,
499+
);
468500
suggested_deref = true;
469501
}
470502
}
471503
}
472504
if let Some(missing_trait) = missing_trait {
505+
let mut visitor = TypeParamVisitor(vec![]);
506+
visitor.visit_ty(lhs_ty);
507+
508+
let mut sugg = false;
473509
if op.node == hir::BinOpKind::Add
474510
&& self.check_str_addition(
475511
lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, false, op,
@@ -478,18 +514,33 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
478514
// This has nothing here because it means we did string
479515
// concatenation (e.g., "Hello " + "World!"). This means
480516
// we don't want the note in the else clause to be emitted
481-
} else if let ty::Param(p) = lhs_ty.kind {
482-
suggest_constraining_param(
483-
self.tcx,
484-
self.body_id,
485-
&mut err,
486-
lhs_ty,
487-
rhs_ty,
488-
missing_trait,
489-
p,
490-
use_output,
491-
);
492-
} else if !suggested_deref && !involves_fn {
517+
sugg = true;
518+
} else if let [ty] = &visitor.0[..] {
519+
if let ty::Param(p) = ty.kind {
520+
// FIXME: This *guesses* that constraining the type param
521+
// will make the operation available, but this is only true
522+
// when the corresponding trait has a blanked
523+
// implementation, like the following:
524+
// `impl<'a> PartialEq for &'a [T] where T: PartialEq {}`
525+
// The correct thing to do would be to verify this
526+
// projection would hold.
527+
if *ty != lhs_ty {
528+
note(&mut err, missing_trait);
529+
}
530+
suggest_constraining_param(
531+
self.tcx,
532+
self.body_id,
533+
&mut err,
534+
ty,
535+
rhs_ty,
536+
missing_trait,
537+
p,
538+
use_output,
539+
);
540+
sugg = true;
541+
}
542+
}
543+
if !sugg && !suggested_deref && !involves_fn {
493544
suggest_impl_missing(&mut err, lhs_ty, &missing_trait);
494545
}
495546
}
@@ -928,8 +979,7 @@ fn suggest_impl_missing(err: &mut DiagnosticBuilder<'_>, ty: Ty<'_>, missing_tra
928979
if let Adt(def, _) = ty.peel_refs().kind {
929980
if def.did.is_local() {
930981
err.note(&format!(
931-
"an implementation of `{}` might \
932-
be missing for `{}`",
982+
"an implementation of `{}` might be missing for `{}`",
933983
missing_trait, ty
934984
));
935985
}
@@ -975,3 +1025,14 @@ fn suggest_constraining_param(
9751025
err.span_label(span, msg);
9761026
}
9771027
}
1028+
1029+
struct TypeParamVisitor<'tcx>(Vec<Ty<'tcx>>);
1030+
1031+
impl<'tcx> TypeVisitor<'tcx> for TypeParamVisitor<'tcx> {
1032+
fn visit_ty(&mut self, ty: Ty<'tcx>) -> bool {
1033+
if let ty::Param(_) = ty.kind {
1034+
self.0.push(ty);
1035+
}
1036+
ty.super_visit_with(self)
1037+
}
1038+
}

‎src/test/ui/binary-op-on-double-ref.fixed‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// run-rustfix
2+
fn main() {
3+
let v = vec![1, 2, 3, 4, 5, 6, 7, 8, 9];
4+
let vr = v.iter().filter(|x| {
5+
*x % 2 == 0
6+
//~^ ERROR cannot mod `&&{integer}` by `{integer}`
7+
});
8+
println!("{:?}", vr);
9+
}

‎src/test/ui/binary-op-on-double-ref.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// run-rustfix
12
fn main() {
23
let v = vec![1, 2, 3, 4, 5, 6, 7, 8, 9];
34
let vr = v.iter().filter(|x| {

‎src/test/ui/binary-op-on-double-ref.stderr‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
error[E0369]: cannot mod `&&{integer}` by `{integer}`
2-
--> $DIR/binary-op-on-double-ref.rs:4:11
2+
--> $DIR/binary-op-on-double-ref.rs:5:11
33
|
44
LL | x % 2 == 0
55
| - ^ - {integer}
66
| |
77
| &&{integer}
88
|
9-
= help: `%` can be used on '{integer}', you can dereference `x`: `*x`
9+
help: `%` can be used on `{integer}`, you can dereference `x`
10+
|
11+
LL | *x % 2 == 0
12+
| ^
1013

1114
error: aborting due to previous error
1215

‎src/test/ui/issues/issue-35668.stderr‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ LL | a.iter().map(|a| a*a)
55
| -^- &T
66
| |
77
| &T
8+
|
9+
= note: the trait `std::ops::Mul` is not implemented for `&T`
10+
help: consider restricting type parameter `T`
11+
|
12+
LL | fn func<'a, T: std::ops::Mul<Output = &T>>(a: &'a [T]) -> impl Iterator<Item=&'a T> {
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
814

915
error: aborting due to previous error
1016

‎src/test/ui/issues/issue-5239-1.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ LL | let x = |ref x: isize| { x += 1; };
99
help: `+=` can be used on 'isize', you can dereference `x`
1010
|
1111
LL | let x = |ref x: isize| { *x += 1; };
12-
| ^^
12+
| ^
1313

1414
error: aborting due to previous error
1515

‎src/test/ui/suggestions/missing-trait-bound-for-op.fixed‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// run-rustfix
2+
3+
pub fn strip_prefix<'a, T: std::cmp::PartialEq>(s: &'a [T], prefix: &[T]) -> Option<&'a [T]> {
4+
let n = prefix.len();
5+
if n <= s.len() {
6+
let (head, tail) = s.split_at(n);
7+
if head == prefix { //~ ERROR binary operation `==` cannot be applied to type `&[T]`
8+
return Some(tail);
9+
}
10+
}
11+
None
12+
}
13+
fn main() {}

‎src/test/ui/suggestions/missing-trait-bound-for-op.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// run-rustfix
2+
3+
pub fn strip_prefix<'a, T>(s: &'a [T], prefix: &[T]) -> Option<&'a [T]> {
4+
let n = prefix.len();
5+
if n <= s.len() {
6+
let (head, tail) = s.split_at(n);
7+
if head == prefix { //~ ERROR binary operation `==` cannot be applied to type `&[T]`
8+
return Some(tail);
9+
}
10+
}
11+
None
12+
}
13+
fn main() {}

‎src/test/ui/suggestions/missing-trait-bound-for-op.stderr‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
error[E0369]: binary operation `==` cannot be applied to type `&[T]`
2+
--> $DIR/missing-trait-bound-for-op.rs:7:17
3+
|
4+
LL | if head == prefix {
5+
| ---- ^^ ------ &[T]
6+
| |
7+
| &[T]
8+
|
9+
= note: the trait `std::cmp::PartialEq` is not implemented for `&[T]`
10+
help: consider restricting type parameter `T`
11+
|
12+
LL | pub fn strip_prefix<'a, T: std::cmp::PartialEq>(s: &'a [T], prefix: &[T]) -> Option<&'a [T]> {
13+
| ^^^^^^^^^^^^^^^^^^^^^
14+
15+
error: aborting due to previous error
16+
17+
For more information about this error, try `rustc --explain E0369`.

‎src/test/ui/traits/trait-resolution-in-overloaded-op.stderr‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ LL | a * b
55
| - ^ - f64
66
| |
77
| &T
8+
|
9+
= note: the trait `std::ops::Mul` is not implemented for `&T`
10+
help: consider further restricting this bound
11+
|
12+
LL | fn foo<T: MyMul<f64, f64> + std::ops::Mul<Output = f64>>(a: &T, b: f64) -> f64 {
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
814

915
error: aborting due to previous error
1016

0 commit comments

Comments
 (0)