From 8f40dae93b3a66a9cdd0f940244da7f602618fba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 24 Jun 2020 16:17:04 -0700 Subject: [PATCH] Suggest type param trait bound for binop only when appropriate Verify that the binop trait *is* implemented for the types *if* all the involved type parameters are replaced with fresh inferred types. When this is the case, it means that the type parameter was indeed missing a trait bound. If this is not the case, provide a generic `note` refering to the type that doesn't implement the expected trait. --- src/librustc_typeck/check/op.rs | 67 +++++++++++++------ src/test/ui/issues/issue-35668.stderr | 1 - src/test/ui/suggestions/invalid-bin-op.rs | 7 ++ src/test/ui/suggestions/invalid-bin-op.stderr | 13 ++++ .../missing-trait-bound-for-op.fixed | 12 +--- .../suggestions/missing-trait-bound-for-op.rs | 12 +--- .../missing-trait-bound-for-op.stderr | 15 ++--- .../trait-resolution-in-overloaded-op.stderr | 1 - 8 files changed, 78 insertions(+), 50 deletions(-) create mode 100644 src/test/ui/suggestions/invalid-bin-op.rs create mode 100644 src/test/ui/suggestions/invalid-bin-op.stderr diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs index 0184c00c475b5..e333b706e745d 100644 --- a/src/librustc_typeck/check/op.rs +++ b/src/librustc_typeck/check/op.rs @@ -8,6 +8,7 @@ use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKi use rustc_middle::ty::adjustment::{ Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, }; +use rustc_middle::ty::fold::TypeFolder; 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, TypeVisitor, @@ -436,29 +437,36 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // we don't want the note in the else clause to be emitted } 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 blanket - // 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 { + // Check if the method would be found if the type param wasn't + // involved. If so, it means that adding a trait bound to the param is + // enough. Otherwise we do not give the suggestion. + let mut eraser = TypeParamEraser(&self, expr.span); + let needs_bound = self + .lookup_op_method( + eraser.fold_ty(lhs_ty), + &[eraser.fold_ty(rhs_ty)], + Op::Binary(op, is_assign), + ) + .is_ok(); + if needs_bound { + suggest_constraining_param( + self.tcx, + self.body_id, + &mut err, + ty, + rhs_ty, + missing_trait, + p, + use_output, + ); + } else if *ty != lhs_ty { + // When we know that a missing bound is responsible, we don't show + // this note as it is redundant. err.note(&format!( "the trait `{}` is not implemented for `{}`", missing_trait, lhs_ty )); } - suggest_constraining_param( - self.tcx, - self.body_id, - &mut err, - ty, - rhs_ty, - missing_trait, - p, - use_output, - ); } else { bug!("type param visitor stored a non type param: {:?}", ty.kind); } @@ -656,10 +664,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); err.span_label( ex.span, - format!( - "cannot apply unary operator `{}`", - op.as_str() - ), + format!("cannot apply unary operator `{}`", op.as_str()), ); match actual.kind { Uint(_) if op == hir::UnOp::UnNeg => { @@ -954,3 +959,21 @@ impl<'tcx> TypeVisitor<'tcx> for TypeParamVisitor<'tcx> { ty.super_visit_with(self) } } + +struct TypeParamEraser<'a, 'tcx>(&'a FnCtxt<'a, 'tcx>, Span); + +impl TypeFolder<'tcx> for TypeParamEraser<'_, 'tcx> { + fn tcx(&self) -> TyCtxt<'tcx> { + self.0.tcx + } + + fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> { + match ty.kind { + ty::Param(_) => self.0.next_ty_var(TypeVariableOrigin { + kind: TypeVariableOriginKind::MiscVariable, + span: self.1, + }), + _ => ty.super_fold_with(self), + } + } +} diff --git a/src/test/ui/issues/issue-35668.stderr b/src/test/ui/issues/issue-35668.stderr index c8f1f88a4708e..600cacc23aef5 100644 --- a/src/test/ui/issues/issue-35668.stderr +++ b/src/test/ui/issues/issue-35668.stderr @@ -6,7 +6,6 @@ LL | a.iter().map(|a| a*a) | | | &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 { diff --git a/src/test/ui/suggestions/invalid-bin-op.rs b/src/test/ui/suggestions/invalid-bin-op.rs new file mode 100644 index 0000000000000..bea1b91558646 --- /dev/null +++ b/src/test/ui/suggestions/invalid-bin-op.rs @@ -0,0 +1,7 @@ +pub fn foo(s: S, t: S) { + let _ = s == t; //~ ERROR binary operation `==` cannot be applied to type `S` +} + +struct S(T); + +fn main() {} diff --git a/src/test/ui/suggestions/invalid-bin-op.stderr b/src/test/ui/suggestions/invalid-bin-op.stderr new file mode 100644 index 0000000000000..7668eddf6070a --- /dev/null +++ b/src/test/ui/suggestions/invalid-bin-op.stderr @@ -0,0 +1,13 @@ +error[E0369]: binary operation `==` cannot be applied to type `S` + --> $DIR/invalid-bin-op.rs:2:15 + | +LL | let _ = s == t; + | - ^^ - S + | | + | S + | + = note: the trait `std::cmp::PartialEq` is not implemented for `S` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0369`. diff --git a/src/test/ui/suggestions/missing-trait-bound-for-op.fixed b/src/test/ui/suggestions/missing-trait-bound-for-op.fixed index 02886bb845cf7..6b24375e41503 100644 --- a/src/test/ui/suggestions/missing-trait-bound-for-op.fixed +++ b/src/test/ui/suggestions/missing-trait-bound-for-op.fixed @@ -1,13 +1,7 @@ // 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 +pub fn foo(s: &[T], t: &[T]) { + let _ = s == t; //~ ERROR binary operation `==` cannot be applied to type `&[T]` } + 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 index aa4ef467360d9..df47be070c9ea 100644 --- a/src/test/ui/suggestions/missing-trait-bound-for-op.rs +++ b/src/test/ui/suggestions/missing-trait-bound-for-op.rs @@ -1,13 +1,7 @@ // 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 +pub fn foo(s: &[T], t: &[T]) { + let _ = s == t; //~ ERROR binary operation `==` cannot be applied to type `&[T]` } + 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 index dab4e575be1fa..0e0d397d6fc15 100644 --- a/src/test/ui/suggestions/missing-trait-bound-for-op.stderr +++ b/src/test/ui/suggestions/missing-trait-bound-for-op.stderr @@ -1,16 +1,15 @@ error[E0369]: binary operation `==` cannot be applied to type `&[T]` - --> $DIR/missing-trait-bound-for-op.rs:7:17 + --> $DIR/missing-trait-bound-for-op.rs:4:15 | -LL | if head == prefix { - | ---- ^^ ------ &[T] - | | - | &[T] +LL | let _ = s == t; + | - ^^ - &[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]> { - | ^^^^^^^^^^^^^^^^^^^^^ +LL | pub fn foo(s: &[T], t: &[T]) { + | ^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error 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 1c424ce7da669..507d53dc07c4c 100644 --- a/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr +++ b/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr @@ -6,7 +6,6 @@ LL | a * b | | | &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 {