Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweak binop errors #73674

Merged
merged 3 commits into from
Jun 26, 2020
Merged

Tweak binop errors #73674

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5aab1a9
Tweak binop errors
estebank Jun 24, 2020
09af184
review comments: clean up code
estebank Jun 24, 2020
8f40dae
Suggest type param trait bound for binop only when appropriate
estebank Jun 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 98 additions & 37 deletions src/librustc_typeck/check/op.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -254,6 +256,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if !lhs_ty.references_error() && !rhs_ty.references_error() {
estebank marked this conversation as resolved.
Show resolved Hide resolved
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!(
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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
estebank marked this conversation as resolved.
Show resolved Hide resolved
// 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.
estebank marked this conversation as resolved.
Show resolved Hide resolved
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 {
estebank marked this conversation as resolved.
Show resolved Hide resolved
suggest_impl_missing(&mut err, lhs_ty, &missing_trait);
}
}
Expand Down Expand Up @@ -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(
estebank marked this conversation as resolved.
Show resolved Hide resolved
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,
Expand All @@ -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
estebank marked this conversation as resolved.
Show resolved Hide resolved
// 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 {
estebank marked this conversation as resolved.
Show resolved Hide resolved
suggest_impl_missing(&mut err, lhs_ty, &missing_trait);
}
}
Expand Down Expand Up @@ -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
));
}
Expand Down Expand Up @@ -975,3 +1025,14 @@ 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)
}
}
9 changes: 9 additions & 0 deletions src/test/ui/binary-op-on-double-ref.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 change: 1 addition & 0 deletions src/test/ui/binary-op-on-double-ref.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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| {
Expand Down
7 changes: 5 additions & 2 deletions src/test/ui/binary-op-on-double-ref.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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

Expand Down
6 changes: 6 additions & 0 deletions src/test/ui/issues/issue-35668.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Output = &T>>(a: &'a [T]) -> impl Iterator<Item=&'a T> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-5239-1.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
13 changes: 13 additions & 0 deletions src/test/ui/suggestions/missing-trait-bound-for-op.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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() {}
13 changes: 13 additions & 0 deletions src/test/ui/suggestions/missing-trait-bound-for-op.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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() {}
17 changes: 17 additions & 0 deletions src/test/ui/suggestions/missing-trait-bound-for-op.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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`.
6 changes: 6 additions & 0 deletions src/test/ui/traits/trait-resolution-in-overloaded-op.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<T: MyMul<f64, f64> + std::ops::Mul<Output = f64>>(a: &T, b: f64) -> f64 {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

Expand Down