From 75f066dc687e9a40e193ff059491b208a98291aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 7 Apr 2020 13:26:09 -0700 Subject: [PATCH] Handle binop on unbound type param When encountering a binary operation involving a type parameter that has no bindings, suggest adding the appropriate bound. --- src/librustc_hir/hir.rs | 30 +++++++ src/librustc_typeck/check/op.rs | 81 ++++++++++++++++--- src/test/ui/issues/issue-6738.stderr | 5 +- .../type/type-check/missing_trait_impl.stderr | 10 ++- 4 files changed, 110 insertions(+), 16 deletions(-) diff --git a/src/librustc_hir/hir.rs b/src/librustc_hir/hir.rs index 258428d77da10..941430afaba86 100644 --- a/src/librustc_hir/hir.rs +++ b/src/librustc_hir/hir.rs @@ -2626,8 +2626,38 @@ impl Node<'_> { match self { Node::TraitItem(TraitItem { generics, .. }) | Node::ImplItem(ImplItem { generics, .. }) + | Node::Item(Item { kind: ItemKind::Trait(_, _, generics, ..), .. }) + | Node::Item(Item { kind: ItemKind::Impl { generics, .. }, .. }) | Node::Item(Item { kind: ItemKind::Fn(_, generics, _), .. }) => Some(generics), _ => None, } } + + pub fn hir_id(&self) -> Option { + match self { + Node::Item(Item { hir_id, .. }) + | Node::ForeignItem(ForeignItem { hir_id, .. }) + | Node::TraitItem(TraitItem { hir_id, .. }) + | Node::ImplItem(ImplItem { hir_id, .. }) + | Node::Field(StructField { hir_id, .. }) + | Node::AnonConst(AnonConst { hir_id, .. }) + | Node::Expr(Expr { hir_id, .. }) + | Node::Stmt(Stmt { hir_id, .. }) + | Node::Ty(Ty { hir_id, .. }) + | Node::Binding(Pat { hir_id, .. }) + | Node::Pat(Pat { hir_id, .. }) + | Node::Arm(Arm { hir_id, .. }) + | Node::Block(Block { hir_id, .. }) + | Node::Local(Local { hir_id, .. }) + | Node::MacroDef(MacroDef { hir_id, .. }) + | Node::Lifetime(Lifetime { hir_id, .. }) + | Node::Param(Param { hir_id, .. }) + | Node::GenericParam(GenericParam { hir_id, .. }) => Some(*hir_id), + Node::TraitRef(TraitRef { hir_ref_id, .. }) => Some(*hir_ref_id), + Node::PathSegment(PathSegment { hir_id, .. }) => *hir_id, + Node::Variant(Variant { id, .. }) => Some(*id), + Node::Ctor(variant) => variant.ctor_hir_id(), + Node::Crate(_) | Node::Visibility(_) => None, + } + } } diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs index cac9113fd5d30..4cc68e7e54dea 100644 --- a/src/librustc_typeck/check/op.rs +++ b/src/librustc_typeck/check/op.rs @@ -13,6 +13,7 @@ use rustc_middle::ty::TyKind::{Adt, Array, Char, FnDef, Never, Ref, Str, Tuple, use rustc_middle::ty::{self, Ty, TypeFoldable}; use rustc_span::Span; use rustc_trait_selection::infer::InferCtxtExt; +use rustc_trait_selection::traits::error_reporting::suggest_constraining_type_param; impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// Checks a `a = b` @@ -253,6 +254,50 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // error types are considered "builtin" if !lhs_ty.references_error() { let source_map = self.tcx.sess.source_map(); + + let suggest_constraining_param = + |mut err: &mut DiagnosticBuilder<'_>, + missing_trait: &str, + p: ty::ParamTy, + set_output: bool| { + let hir = self.tcx.hir(); + let msg = + &format!("`{}` might need a bound for `{}`", lhs_ty, missing_trait); + if let Some(def_id) = hir + .find(hir.get_parent_item(expr.hir_id)) + .and_then(|node| node.hir_id()) + .and_then(|hir_id| hir.opt_local_def_id(hir_id)) + { + let generics = self.tcx.generics_of(def_id); + let param_def_id = generics.type_param(&p, self.tcx).def_id; + if let Some(generics) = hir + .as_local_hir_id(param_def_id) + .and_then(|id| hir.find(hir.get_parent_item(id))) + .as_ref() + .and_then(|node| node.generics()) + { + let output = if set_output { + format!("", rhs_ty) + } else { + String::new() + }; + suggest_constraining_type_param( + self.tcx, + generics, + &mut err, + &format!("{}", lhs_ty), + &format!("{}{}", missing_trait, output), + None, + ); + } else { + let span = self.tcx.def_span(param_def_id); + err.span_label(span, msg); + } + } else { + err.note(&msg); + } + }; + match is_assign { IsAssign::Yes => { let mut err = struct_span_err!( @@ -317,12 +362,8 @@ 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(_) = lhs_ty.kind { - // FIXME: point to span of param - err.note(&format!( - "`{}` might need a bound for `{}`", - lhs_ty, missing_trait - )); + } else if let ty::Param(p) = lhs_ty.kind { + suggest_constraining_param(&mut err, missing_trait, p, false); } else if !suggested_deref { suggest_impl_missing(&mut err, lhs_ty, &missing_trait); } @@ -330,46 +371,56 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err.emit(); } IsAssign::No => { - let (message, missing_trait) = match op.node { + let (message, missing_trait, use_output) = match op.node { hir::BinOpKind::Add => ( format!("cannot add `{}` to `{}`", rhs_ty, lhs_ty), Some("std::ops::Add"), + true, ), hir::BinOpKind::Sub => ( format!("cannot subtract `{}` from `{}`", rhs_ty, lhs_ty), Some("std::ops::Sub"), + true, ), hir::BinOpKind::Mul => ( format!("cannot multiply `{}` to `{}`", rhs_ty, lhs_ty), Some("std::ops::Mul"), + true, ), hir::BinOpKind::Div => ( format!("cannot divide `{}` by `{}`", lhs_ty, rhs_ty), Some("std::ops::Div"), + true, ), hir::BinOpKind::Rem => ( format!("cannot mod `{}` by `{}`", lhs_ty, rhs_ty), Some("std::ops::Rem"), + true, ), hir::BinOpKind::BitAnd => ( format!("no implementation for `{} & {}`", lhs_ty, rhs_ty), Some("std::ops::BitAnd"), + true, ), hir::BinOpKind::BitXor => ( format!("no implementation for `{} ^ {}`", lhs_ty, rhs_ty), Some("std::ops::BitXor"), + true, ), hir::BinOpKind::BitOr => ( format!("no implementation for `{} | {}`", lhs_ty, rhs_ty), Some("std::ops::BitOr"), + true, ), hir::BinOpKind::Shl => ( format!("no implementation for `{} << {}`", lhs_ty, rhs_ty), Some("std::ops::Shl"), + true, ), hir::BinOpKind::Shr => ( format!("no implementation for `{} >> {}`", lhs_ty, rhs_ty), Some("std::ops::Shr"), + true, ), hir::BinOpKind::Eq | hir::BinOpKind::Ne => ( format!( @@ -378,6 +429,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { lhs_ty ), Some("std::cmp::PartialEq"), + false, ), hir::BinOpKind::Lt | hir::BinOpKind::Le @@ -389,6 +441,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { lhs_ty ), Some("std::cmp::PartialOrd"), + false, ), _ => ( format!( @@ -397,6 +450,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { lhs_ty ), None, + false, ), }; let mut err = struct_span_err!( @@ -459,12 +513,13 @@ 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(_) = lhs_ty.kind { - // FIXME: point to span of param - err.note(&format!( - "`{}` might need a bound for `{}`", - lhs_ty, missing_trait - )); + } else if let ty::Param(p) = lhs_ty.kind { + suggest_constraining_param( + &mut err, + missing_trait, + p, + use_output, + ); } else if !suggested_deref && !involves_fn { suggest_impl_missing(&mut err, lhs_ty, &missing_trait); } diff --git a/src/test/ui/issues/issue-6738.stderr b/src/test/ui/issues/issue-6738.stderr index 82b670bd03bc5..a428ff7e91fad 100644 --- a/src/test/ui/issues/issue-6738.stderr +++ b/src/test/ui/issues/issue-6738.stderr @@ -6,7 +6,10 @@ LL | self.x += v.x; | | | cannot use `+=` on type `T` | - = note: `T` might need a bound for `std::ops::AddAssign` +help: consider restricting type parameter `T` + | +LL | impl Foo { + | ^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/type/type-check/missing_trait_impl.stderr b/src/test/ui/type/type-check/missing_trait_impl.stderr index 7186d6a542dc9..30df1261cefa1 100644 --- a/src/test/ui/type/type-check/missing_trait_impl.stderr +++ b/src/test/ui/type/type-check/missing_trait_impl.stderr @@ -6,7 +6,10 @@ LL | let z = x + y; | | | T | - = note: `T` might need a bound for `std::ops::Add` +help: consider restricting type parameter `T` + | +LL | fn foo>(x: T, y: T) { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0368]: binary assignment operation `+=` cannot be applied to type `T` --> $DIR/missing_trait_impl.rs:9:5 @@ -16,7 +19,10 @@ LL | x += x; | | | cannot use `+=` on type `T` | - = note: `T` might need a bound for `std::ops::AddAssign` +help: consider restricting type parameter `T` + | +LL | fn bar(x: T) { + | ^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 2 previous errors