Skip to content

Commit

Permalink
Handle binop on unbound type param
Browse files Browse the repository at this point in the history
When encountering a binary operation involving a type parameter that has
no bindings, suggest adding the appropriate bound.
  • Loading branch information
estebank committed May 4, 2020
1 parent 6318d24 commit 75f066d
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 16 deletions.
30 changes: 30 additions & 0 deletions src/librustc_hir/hir.rs
Expand Up @@ -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<HirId> {
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,
}
}
}
81 changes: 68 additions & 13 deletions src/librustc_typeck/check/op.rs
Expand Up @@ -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 <op>= b`
Expand Down Expand Up @@ -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!("<Output = {}>", 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!(
Expand Down Expand Up @@ -317,59 +362,65 @@ 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);
}
}
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!(
Expand All @@ -378,6 +429,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
lhs_ty
),
Some("std::cmp::PartialEq"),
false,
),
hir::BinOpKind::Lt
| hir::BinOpKind::Le
Expand All @@ -389,6 +441,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
lhs_ty
),
Some("std::cmp::PartialOrd"),
false,
),
_ => (
format!(
Expand All @@ -397,6 +450,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
lhs_ty
),
None,
false,
),
};
let mut err = struct_span_err!(
Expand Down Expand Up @@ -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);
}
Expand Down
5 changes: 4 additions & 1 deletion src/test/ui/issues/issue-6738.stderr
Expand Up @@ -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<T: std::ops::AddAssign> Foo<T> {
| ^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

Expand Down
10 changes: 8 additions & 2 deletions src/test/ui/type/type-check/missing_trait_impl.stderr
Expand Up @@ -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<T: std::ops::Add<Output = T>>(x: T, y: T) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0368]: binary assignment operation `+=` cannot be applied to type `T`
--> $DIR/missing_trait_impl.rs:9:5
Expand All @@ -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<T: std::ops::AddAssign>(x: T) {
| ^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

Expand Down

0 comments on commit 75f066d

Please sign in to comment.