user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Rollup merge of #73674 - estebank:op-trait-bound-suggestion, r=davidtwco

Browse source 
Tweak binop errors

* Suggest potentially missing binop trait bound (fix #73416)
* Use structured suggestion for dereference in binop
This commit is contained in:
Manish Goregaokar 2020-06-25 18:00:23 -07:00 • committed by GitHub
commit 7fb7765cda
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 333 additions and 259 deletions
Download patch file Download diff file Expand all files Collapse all files

3
src/librustc_typeck/check/method/mod.rs
View file

@ -295,8 +295,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
opt_input_types: Option<&[Ty<'tcx>]>,
) -> Option<InferOk<'tcx, MethodCallee<'tcx>>> {
debug!(
"lookup_in_trait_adjusted(self_ty={:?}, \
m_name={}, trait_def_id={:?})",
"lookup_in_trait_adjusted(self_ty={:?}, m_name={}, trait_def_id={:?})",
self_ty, m_name, trait_def_id
);

4
src/librustc_typeck/check/method/probe.rs
View file

@ -380,8 +380,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.tcx.sess,
span,
E0699,
"the type of this value must be known \
to call a method on a raw pointer on it"
"the type of this value must be known to call a method on a raw pointer on \
it"
)
.emit();
} else {

504
src/librustc_typeck/check/op.rs
View file

@ -8,8 +8,11 @@ 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};
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;
@ -249,254 +252,229 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
method.sig.output()
}
// error types are considered "builtin"
Err(()) if lhs_ty.references_error() || rhs_ty.references_error() => {
self.tcx.ty_error()
}
Err(()) => {
// error types are considered "builtin"
if !lhs_ty.references_error() && !rhs_ty.references_error() {
let source_map = self.tcx.sess.source_map();
match is_assign {
IsAssign::Yes => {
let mut err = struct_span_err!(
self.tcx.sess,
expr.span,
E0368,
"binary assignment operation `{}=` cannot be applied to type `{}`",
op.node.as_str(),
lhs_ty,
);
err.span_label(
let source_map = self.tcx.sess.source_map();
let (mut err, missing_trait, use_output, involves_fn) = match is_assign {
IsAssign::Yes => {
let mut err = struct_span_err!(
self.tcx.sess,
expr.span,
E0368,
"binary assignment operation `{}=` cannot be applied to type `{}`",
op.node.as_str(),
lhs_ty,
);
err.span_label(
lhs_expr.span,
format!("cannot use `{}=` on type `{}`", op.node.as_str(), lhs_ty),
);
let missing_trait = match op.node {
hir::BinOpKind::Add => Some("std::ops::AddAssign"),
hir::BinOpKind::Sub => Some("std::ops::SubAssign"),
hir::BinOpKind::Mul => Some("std::ops::MulAssign"),
hir::BinOpKind::Div => Some("std::ops::DivAssign"),
hir::BinOpKind::Rem => Some("std::ops::RemAssign"),
hir::BinOpKind::BitAnd => Some("std::ops::BitAndAssign"),
hir::BinOpKind::BitXor => Some("std::ops::BitXorAssign"),
hir::BinOpKind::BitOr => Some("std::ops::BitOrAssign"),
hir::BinOpKind::Shl => Some("std::ops::ShlAssign"),
hir::BinOpKind::Shr => Some("std::ops::ShrAssign"),
_ => None,
};
(err, missing_trait, false, false)
}
IsAssign::No => {
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!(
"binary operation `{}` cannot be applied to type `{}`",
op.node.as_str(),
lhs_ty
),
Some("std::cmp::PartialEq"),
false,
),
hir::BinOpKind::Lt
| hir::BinOpKind::Le
| hir::BinOpKind::Gt
| hir::BinOpKind::Ge => (
format!(
"binary operation `{}` cannot be applied to type `{}`",
op.node.as_str(),
lhs_ty
),
Some("std::cmp::PartialOrd"),
false,
),
_ => (
format!(
"binary operation `{}` cannot be applied to type `{}`",
op.node.as_str(),
lhs_ty
),
None,
false,
),
};
let mut err =
struct_span_err!(self.tcx.sess, op.span, E0369, "{}", message.as_str());
let mut involves_fn = false;
if !lhs_expr.span.eq(&rhs_expr.span) {
involves_fn |= self.add_type_neq_err_label(
&mut err,
lhs_expr.span,
format!("cannot use `{}=` on type `{}`", op.node.as_str(), lhs_ty),
lhs_ty,
rhs_ty,
op,
is_assign,
);
involves_fn |= self.add_type_neq_err_label(
&mut err,
rhs_expr.span,
rhs_ty,
lhs_ty,
op,
is_assign,
);
let mut suggested_deref = false;
if let Ref(_, rty, _) = lhs_ty.kind {
if {
self.infcx.type_is_copy_modulo_regions(
self.param_env,
rty,
lhs_expr.span,
) && self
.lookup_op_method(rty, &[rhs_ty], Op::Binary(op, is_assign))
.is_ok()
} {
if let Ok(lstring) = source_map.span_to_snippet(lhs_expr.span) {
let msg = &format!(
"`{}=` can be used on '{}', you can dereference `{}`",
op.node.as_str(),
rty.peel_refs(),
lstring,
);
err.span_suggestion(
lhs_expr.span,
msg,
format!("*{}", lstring),
rustc_errors::Applicability::MachineApplicable,
);
suggested_deref = true;
}
}
}
let missing_trait = match op.node {
hir::BinOpKind::Add => Some("std::ops::AddAssign"),
hir::BinOpKind::Sub => Some("std::ops::SubAssign"),
hir::BinOpKind::Mul => Some("std::ops::MulAssign"),
hir::BinOpKind::Div => Some("std::ops::DivAssign"),
hir::BinOpKind::Rem => Some("std::ops::RemAssign"),
hir::BinOpKind::BitAnd => Some("std::ops::BitAndAssign"),
hir::BinOpKind::BitXor => Some("std::ops::BitXorAssign"),
hir::BinOpKind::BitOr => Some("std::ops::BitOrAssign"),
hir::BinOpKind::Shl => Some("std::ops::ShlAssign"),
hir::BinOpKind::Shr => Some("std::ops::ShrAssign"),
_ => None,
};
if let Some(missing_trait) = missing_trait {
if op.node == hir::BinOpKind::Add
&& self.check_str_addition(
lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, true, op,
)
{
// 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 {
suggest_impl_missing(&mut err, lhs_ty, &missing_trait);
}
}
err.emit();
}
IsAssign::No => {
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!(
"binary operation `{}` cannot be applied to type `{}`",
op.node.as_str(),
lhs_ty
),
Some("std::cmp::PartialEq"),
false,
),
hir::BinOpKind::Lt
| hir::BinOpKind::Le
| hir::BinOpKind::Gt
| hir::BinOpKind::Ge => (
format!(
"binary operation `{}` cannot be applied to type `{}`",
op.node.as_str(),
lhs_ty
),
Some("std::cmp::PartialOrd"),
false,
),
_ => (
format!(
"binary operation `{}` cannot be applied to type `{}`",
op.node.as_str(),
lhs_ty
),
None,
false,
),
};
let mut err = struct_span_err!(
self.tcx.sess,
op.span,
E0369,
"{}",
message.as_str()
(err, missing_trait, use_output, involves_fn)
}
};
let mut suggested_deref = false;
if let Ref(_, rty, _) = lhs_ty.kind {
if {
self.infcx.type_is_copy_modulo_regions(self.param_env, rty, lhs_expr.span)
&& self
.lookup_op_method(rty, &[rhs_ty], Op::Binary(op, is_assign))
.is_ok()
} {
if let Ok(lstring) = source_map.span_to_snippet(lhs_expr.span) {
let msg = &format!(
"`{}{}` can be used on `{}`, you can dereference `{}`",
op.node.as_str(),
match is_assign {
IsAssign::Yes => "=",
IsAssign::No => "",
},
rty.peel_refs(),
lstring,
);
let mut involves_fn = false;
if !lhs_expr.span.eq(&rhs_expr.span) {
involves_fn |= self.add_type_neq_err_label(
&mut err,
lhs_expr.span,
lhs_ty,
rhs_ty,
op,
is_assign,
);
involves_fn |= self.add_type_neq_err_label(
&mut err,
rhs_expr.span,
rhs_ty,
lhs_ty,
op,
is_assign,
);
}
let mut suggested_deref = false;
if let Ref(_, rty, _) = lhs_ty.kind {
if {
self.infcx.type_is_copy_modulo_regions(
self.param_env,
rty,
lhs_expr.span,
) && self
.lookup_op_method(rty, &[rhs_ty], Op::Binary(op, is_assign))
.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
));
suggested_deref = true;
}
}
}
if let Some(missing_trait) = missing_trait {
if op.node == hir::BinOpKind::Add
&& self.check_str_addition(
lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, false, op,
)
{
// 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 {
suggest_impl_missing(&mut err, lhs_ty, &missing_trait);
}
}
err.emit();
err.span_suggestion_verbose(
lhs_expr.span.shrink_to_lo(),
msg,
"*".to_string(),
rustc_errors::Applicability::MachineApplicable,
);
suggested_deref = true;
}
}
}
if let Some(missing_trait) = missing_trait {
let mut visitor = TypeParamVisitor(vec![]);
visitor.visit_ty(lhs_ty);
if op.node == hir::BinOpKind::Add
&& self.check_str_addition(
lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, is_assign, op,
)
{
// 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] = &visitor.0[..] {
if let ty::Param(p) = ty.kind {
// 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
));
}
} else {
bug!("type param visitor stored a non type param: {:?}", ty.kind);
}
} else if !suggested_deref && !involves_fn {
suggest_impl_missing(&mut err, lhs_ty, &missing_trait);
}
}
err.emit();
self.tcx.ty_error()
}
};
@ -570,7 +548,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
lhs_ty: Ty<'tcx>,
rhs_ty: Ty<'tcx>,
err: &mut rustc_errors::DiagnosticBuilder<'_>,
is_assign: bool,
is_assign: IsAssign,
op: hir::BinOp,
) -> bool {
let source_map = self.tcx.sess.source_map();
@ -593,7 +571,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&format!("{:?}", rhs_ty) == "&&str"
) =>
{
if !is_assign { // Do not supply this message if `&str += &str`
if let IsAssign::No = is_assign { // Do not supply this message if `&str += &str`
err.span_label(
op.span,
"`+` cannot be used to concatenate two `&str` strings",
@ -634,7 +612,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
source_map.span_to_snippet(rhs_expr.span),
is_assign,
) {
(Ok(l), Ok(r), false) => {
(Ok(l), Ok(r), IsAssign::No) => {
let to_string = if l.starts_with('&') {
// let a = String::new(); let b = String::new();
// let _ = &a + b;
@ -686,11 +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 => {
@ -928,8 +902,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 +948,32 @@ fn suggest_constraining_param(
err.span_label(span, msg);
}
}
struct TypeParamVisitor<'tcx>(Vec<Ty<'tcx>>);
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)
}
}
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),
}
}
}

9
src/test/ui/binary-op-on-double-ref.fixed Normal file
View file

@ -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);
}

1
src/test/ui/binary-op-on-double-ref.rs
View file

@ -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| {

7
src/test/ui/binary-op-on-double-ref.stderr
View file

@ -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

5
src/test/ui/issues/issue-35668.stderr
View file

@ -5,6 +5,11 @@ LL | a.iter().map(|a| a*a)
| -^- &T
| |
| &T
|
help: consider restricting type parameter `T`
|
LL | fn func<'a, T: std::ops::Mul<Output = &T>>(a: &'a [T]) -> impl Iterator<Item=&'a T> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to previous error

4
src/test/ui/issues/issue-5239-1.stderr
View file

@ -6,10 +6,10 @@ LL | let x = |ref x: isize| { x += 1; };
| |
| cannot use `+=` on type `&isize`
|
help: `+=` can be used on 'isize', you can dereference `x`
help: `+=` can be used on `isize`, you can dereference `x`
|
LL | let x = |ref x: isize| { *x += 1; };
| ^^
| ^
error: aborting due to previous error

7
src/test/ui/suggestions/invalid-bin-op.rs Normal file
View file

@ -0,0 +1,7 @@
pub fn foo<T>(s: S<T>, t: S<T>) {
let _ = s == t; //~ ERROR binary operation `==` cannot be applied to type `S<T>`
}
struct S<T>(T);
fn main() {}

13
src/test/ui/suggestions/invalid-bin-op.stderr Normal file
View file

@ -0,0 +1,13 @@
error[E0369]: binary operation `==` cannot be applied to type `S<T>`
--> $DIR/invalid-bin-op.rs:2:15
|
LL | let _ = s == t;
| - ^^ - S<T>
| |
| S<T>
|
= note: the trait `std::cmp::PartialEq` is not implemented for `S<T>`
error: aborting due to previous error
For more information about this error, try `rustc --explain E0369`.

7
src/test/ui/suggestions/missing-trait-bound-for-op.fixed Normal file
View file

@ -0,0 +1,7 @@
// run-rustfix
pub fn foo<T: std::cmp::PartialEq>(s: &[T], t: &[T]) {
let _ = s == t; //~ ERROR binary operation `==` cannot be applied to type `&[T]`
}
fn main() {}

7
src/test/ui/suggestions/missing-trait-bound-for-op.rs Normal file
View file

@ -0,0 +1,7 @@
// run-rustfix
pub fn foo<T>(s: &[T], t: &[T]) {
let _ = s == t; //~ ERROR binary operation `==` cannot be applied to type `&[T]`
}
fn main() {}

16
src/test/ui/suggestions/missing-trait-bound-for-op.stderr Normal file
View file

@ -0,0 +1,16 @@
error[E0369]: binary operation `==` cannot be applied to type `&[T]`
--> $DIR/missing-trait-bound-for-op.rs:4:15
|
LL | let _ = s == t;
| - ^^ - &[T]
| |
| &[T]
|
help: consider restricting type parameter `T`
|
LL | pub fn foo<T: std::cmp::PartialEq>(s: &[T], t: &[T]) {
| ^^^^^^^^^^^^^^^^^^^^^
error: aborting due to previous error
For more information about this error, try `rustc --explain E0369`.

5
src/test/ui/traits/trait-resolution-in-overloaded-op.stderr
View file

@ -5,6 +5,11 @@ LL | a * b
| - ^ - f64
| |
| &T
|
help: consider further restricting this bound
|
LL | fn foo<T: MyMul<f64, f64> + std::ops::Mul<Output = f64>>(a: &T, b: f64) -> f64 {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to previous error