From 5aab1a9a88dd30d622a5303f26d9ad213c551473 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 23 Jun 2020 17:32:06 -0700 Subject: [PATCH] Tweak binop errors * Suggest potentially missing binop trait bound (fix #73416) * Use structured suggestion for dereference in binop --- src/librustc_typeck/check/op.rs | 135 +++++++++++++----- src/test/ui/binary-op-on-double-ref.fixed | 9 ++ src/test/ui/binary-op-on-double-ref.rs | 1 + src/test/ui/binary-op-on-double-ref.stderr | 7 +- src/test/ui/issues/issue-35668.stderr | 6 + src/test/ui/issues/issue-5239-1.stderr | 2 +- .../missing-trait-bound-for-op.fixed | 13 ++ .../suggestions/missing-trait-bound-for-op.rs | 13 ++ .../missing-trait-bound-for-op.stderr | 17 +++ .../trait-resolution-in-overloaded-op.stderr | 6 + 10 files changed, 169 insertions(+), 40 deletions(-) create mode 100644 src/test/ui/binary-op-on-double-ref.fixed create mode 100644 src/test/ui/suggestions/missing-trait-bound-for-op.fixed create mode 100644 src/test/ui/suggestions/missing-trait-bound-for-op.rs create mode 100644 src/test/ui/suggestions/missing-trait-bound-for-op.stderr diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs index fe508709116..7acf8843185 100644 --- a/src/librustc_typeck/check/op.rs +++ b/src/librustc_typeck/check/op.rs @@ -9,7 +9,9 @@ use rustc_middle::ty::adjustment::{ Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, }; use rustc_middle::ty::TyKind::{Adt, Array, Char, FnDef, Never, Ref, Str, Tuple, Uint}; -use rustc_middle::ty::{self, suggest_constraining_type_param, Ty, TyCtxt, TypeFoldable}; +use rustc_middle::ty::{ + self, suggest_constraining_type_param, Ty, TyCtxt, TypeFoldable, TypeVisitor, +}; use rustc_span::symbol::Ident; use rustc_span::Span; use rustc_trait_selection::infer::InferCtxtExt; @@ -254,6 +256,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if !lhs_ty.references_error() && !rhs_ty.references_error() { let source_map = self.tcx.sess.source_map(); + let note = |err: &mut DiagnosticBuilder<'_>, missing_trait| { + err.note(&format!( + "the trait `{}` is not implemented for `{}`", + missing_trait, lhs_ty + )); + }; match is_assign { IsAssign::Yes => { let mut err = struct_span_err!( @@ -286,10 +294,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { rty.peel_refs(), lstring, ); - err.span_suggestion( - lhs_expr.span, + err.span_suggestion_verbose( + lhs_expr.span.shrink_to_lo(), msg, - format!("*{}", lstring), + "*".to_string(), rustc_errors::Applicability::MachineApplicable, ); suggested_deref = true; @@ -310,6 +318,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { _ => None, }; if let Some(missing_trait) = missing_trait { + let mut visitor = TypeParamVisitor(vec![]); + visitor.visit_ty(lhs_ty); + + let mut sugg = false; if op.node == hir::BinOpKind::Add && self.check_str_addition( lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, true, op, @@ -318,18 +330,33 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // This has nothing here because it means we did string // concatenation (e.g., "Hello " += "World!"). This means // we don't want the note in the else clause to be emitted - } else if let ty::Param(p) = lhs_ty.kind { - suggest_constraining_param( - self.tcx, - self.body_id, - &mut err, - lhs_ty, - rhs_ty, - missing_trait, - p, - false, - ); - } else if !suggested_deref { + sugg = true; + } else if let [ty] = &visitor.0[..] { + if let ty::Param(p) = ty.kind { + // FIXME: This *guesses* that constraining the type param + // will make the operation available, but this is only true + // when the corresponding trait has a blanked + // implementation, like the following: + // `impl<'a> PartialEq for &'a [T] where T: PartialEq {}` + // The correct thing to do would be to verify this + // projection would hold. + if *ty != lhs_ty { + note(&mut err, missing_trait); + } + suggest_constraining_param( + self.tcx, + self.body_id, + &mut err, + ty, + rhs_ty, + missing_trait, + p, + false, + ); + sugg = true; + } + } + if !sugg && !suggested_deref { suggest_impl_missing(&mut err, lhs_ty, &missing_trait); } } @@ -458,18 +485,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .is_ok() } { if let Ok(lstring) = source_map.span_to_snippet(lhs_expr.span) { - err.help(&format!( - "`{}` can be used on '{}', you can \ - dereference `{2}`: `*{2}`", - op.node.as_str(), - rty.peel_refs(), - lstring - )); + err.span_suggestion_verbose( + lhs_expr.span.shrink_to_lo(), + &format!( + "`{}` can be used on `{}`, you can dereference \ + `{}`", + op.node.as_str(), + rty.peel_refs(), + lstring, + ), + "*".to_string(), + Applicability::MachineApplicable, + ); suggested_deref = true; } } } if let Some(missing_trait) = missing_trait { + let mut visitor = TypeParamVisitor(vec![]); + visitor.visit_ty(lhs_ty); + + let mut sugg = false; if op.node == hir::BinOpKind::Add && self.check_str_addition( lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, false, op, @@ -478,18 +514,33 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // This has nothing here because it means we did string // concatenation (e.g., "Hello " + "World!"). This means // we don't want the note in the else clause to be emitted - } else if let ty::Param(p) = lhs_ty.kind { - suggest_constraining_param( - self.tcx, - self.body_id, - &mut err, - lhs_ty, - rhs_ty, - missing_trait, - p, - use_output, - ); - } else if !suggested_deref && !involves_fn { + sugg = true; + } else if let [ty] = &visitor.0[..] { + if let ty::Param(p) = ty.kind { + // FIXME: This *guesses* that constraining the type param + // will make the operation available, but this is only true + // when the corresponding trait has a blanked + // implementation, like the following: + // `impl<'a> PartialEq for &'a [T] where T: PartialEq {}` + // The correct thing to do would be to verify this + // projection would hold. + if *ty != lhs_ty { + note(&mut err, missing_trait); + } + suggest_constraining_param( + self.tcx, + self.body_id, + &mut err, + ty, + rhs_ty, + missing_trait, + p, + use_output, + ); + sugg = true; + } + } + if !sugg && !suggested_deref && !involves_fn { suggest_impl_missing(&mut err, lhs_ty, &missing_trait); } } @@ -928,8 +979,7 @@ fn suggest_impl_missing(err: &mut DiagnosticBuilder<'_>, ty: Ty<'_>, missing_tra if let Adt(def, _) = ty.peel_refs().kind { if def.did.is_local() { err.note(&format!( - "an implementation of `{}` might \ - be missing for `{}`", + "an implementation of `{}` might be missing for `{}`", missing_trait, ty )); } @@ -975,3 +1025,14 @@ fn suggest_constraining_param( err.span_label(span, msg); } } + +struct TypeParamVisitor<'tcx>(Vec>); + +impl<'tcx> TypeVisitor<'tcx> for TypeParamVisitor<'tcx> { + fn visit_ty(&mut self, ty: Ty<'tcx>) -> bool { + if let ty::Param(_) = ty.kind { + self.0.push(ty); + } + ty.super_visit_with(self) + } +} diff --git a/src/test/ui/binary-op-on-double-ref.fixed b/src/test/ui/binary-op-on-double-ref.fixed new file mode 100644 index 00000000000..de9dc19af29 --- /dev/null +++ b/src/test/ui/binary-op-on-double-ref.fixed @@ -0,0 +1,9 @@ +// run-rustfix +fn main() { + let v = vec![1, 2, 3, 4, 5, 6, 7, 8, 9]; + let vr = v.iter().filter(|x| { + *x % 2 == 0 + //~^ ERROR cannot mod `&&{integer}` by `{integer}` + }); + println!("{:?}", vr); +} diff --git a/src/test/ui/binary-op-on-double-ref.rs b/src/test/ui/binary-op-on-double-ref.rs index 67e01b9327d..2616c560cbe 100644 --- a/src/test/ui/binary-op-on-double-ref.rs +++ b/src/test/ui/binary-op-on-double-ref.rs @@ -1,3 +1,4 @@ +// run-rustfix fn main() { let v = vec![1, 2, 3, 4, 5, 6, 7, 8, 9]; let vr = v.iter().filter(|x| { diff --git a/src/test/ui/binary-op-on-double-ref.stderr b/src/test/ui/binary-op-on-double-ref.stderr index 6c405333ec6..02b0488488c 100644 --- a/src/test/ui/binary-op-on-double-ref.stderr +++ b/src/test/ui/binary-op-on-double-ref.stderr @@ -1,12 +1,15 @@ error[E0369]: cannot mod `&&{integer}` by `{integer}` - --> $DIR/binary-op-on-double-ref.rs:4:11 + --> $DIR/binary-op-on-double-ref.rs:5:11 | LL | x % 2 == 0 | - ^ - {integer} | | | &&{integer} | - = help: `%` can be used on '{integer}', you can dereference `x`: `*x` +help: `%` can be used on `{integer}`, you can dereference `x` + | +LL | *x % 2 == 0 + | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-35668.stderr b/src/test/ui/issues/issue-35668.stderr index 98e8e6366b9..c8f1f88a470 100644 --- a/src/test/ui/issues/issue-35668.stderr +++ b/src/test/ui/issues/issue-35668.stderr @@ -5,6 +5,12 @@ LL | a.iter().map(|a| a*a) | -^- &T | | | &T + | + = note: the trait `std::ops::Mul` is not implemented for `&T` +help: consider restricting type parameter `T` + | +LL | fn func<'a, T: std::ops::Mul>(a: &'a [T]) -> impl Iterator { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-5239-1.stderr b/src/test/ui/issues/issue-5239-1.stderr index f4f0f17d001..d0c71e73463 100644 --- a/src/test/ui/issues/issue-5239-1.stderr +++ b/src/test/ui/issues/issue-5239-1.stderr @@ -9,7 +9,7 @@ LL | let x = |ref x: isize| { x += 1; }; help: `+=` can be used on 'isize', you can dereference `x` | LL | let x = |ref x: isize| { *x += 1; }; - | ^^ + | ^ error: aborting due to previous error diff --git a/src/test/ui/suggestions/missing-trait-bound-for-op.fixed b/src/test/ui/suggestions/missing-trait-bound-for-op.fixed new file mode 100644 index 00000000000..02886bb845c --- /dev/null +++ b/src/test/ui/suggestions/missing-trait-bound-for-op.fixed @@ -0,0 +1,13 @@ +// run-rustfix + +pub fn strip_prefix<'a, T: std::cmp::PartialEq>(s: &'a [T], prefix: &[T]) -> Option<&'a [T]> { + let n = prefix.len(); + if n <= s.len() { + let (head, tail) = s.split_at(n); + if head == prefix { //~ ERROR binary operation `==` cannot be applied to type `&[T]` + return Some(tail); + } + } + None +} +fn main() {} diff --git a/src/test/ui/suggestions/missing-trait-bound-for-op.rs b/src/test/ui/suggestions/missing-trait-bound-for-op.rs new file mode 100644 index 00000000000..aa4ef467360 --- /dev/null +++ b/src/test/ui/suggestions/missing-trait-bound-for-op.rs @@ -0,0 +1,13 @@ +// run-rustfix + +pub fn strip_prefix<'a, T>(s: &'a [T], prefix: &[T]) -> Option<&'a [T]> { + let n = prefix.len(); + if n <= s.len() { + let (head, tail) = s.split_at(n); + if head == prefix { //~ ERROR binary operation `==` cannot be applied to type `&[T]` + return Some(tail); + } + } + None +} +fn main() {} diff --git a/src/test/ui/suggestions/missing-trait-bound-for-op.stderr b/src/test/ui/suggestions/missing-trait-bound-for-op.stderr new file mode 100644 index 00000000000..dab4e575be1 --- /dev/null +++ b/src/test/ui/suggestions/missing-trait-bound-for-op.stderr @@ -0,0 +1,17 @@ +error[E0369]: binary operation `==` cannot be applied to type `&[T]` + --> $DIR/missing-trait-bound-for-op.rs:7:17 + | +LL | if head == prefix { + | ---- ^^ ------ &[T] + | | + | &[T] + | + = note: the trait `std::cmp::PartialEq` is not implemented for `&[T]` +help: consider restricting type parameter `T` + | +LL | pub fn strip_prefix<'a, T: std::cmp::PartialEq>(s: &'a [T], prefix: &[T]) -> Option<&'a [T]> { + | ^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0369`. diff --git a/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr b/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr index 29216f36f5f..1c424ce7da6 100644 --- a/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr +++ b/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr @@ -5,6 +5,12 @@ LL | a * b | - ^ - f64 | | | &T + | + = note: the trait `std::ops::Mul` is not implemented for `&T` +help: consider further restricting this bound + | +LL | fn foo + std::ops::Mul>(a: &T, b: f64) -> f64 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error