Skip to content

Commit

Permalink
Replace suggest_constraining_param with suggest_restricting_param_bound
Browse files Browse the repository at this point in the history
to fix incorrect suggestion for trait bounds involving binary operators.
Fixes rust-lang#93927, rust-lang#92347, rust-lang#93744.
  • Loading branch information
willcrichton committed Apr 26, 2022
1 parent 18b53ce commit 4d0fe27
Show file tree
Hide file tree
Showing 9 changed files with 80 additions and 89 deletions.
106 changes: 29 additions & 77 deletions compiler/rustc_typeck/src/check/op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,12 @@ use rustc_middle::ty::adjustment::{
};
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,
};
use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable, TypeVisitor};
use rustc_span::source_map::Spanned;
use rustc_span::symbol::{sym, Ident};
use rustc_span::Span;
use rustc_trait_selection::infer::InferCtxtExt;
use rustc_trait_selection::traits::error_reporting::suggestions::InferCtxtExt as _;
use rustc_trait_selection::traits::{FulfillmentError, TraitEngine, TraitEngineExt};

use std::ops::ControlFlow;
Expand Down Expand Up @@ -266,7 +265,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Err(_) if lhs_ty.references_error() || rhs_ty.references_error() => self.tcx.ty_error(),
Err(errors) => {
let source_map = self.tcx.sess.source_map();
let (mut err, missing_trait, use_output) = match is_assign {
let (mut err, missing_trait, _use_output) = match is_assign {
IsAssign::Yes => {
let mut err = struct_span_err!(
self.tcx.sess,
Expand Down Expand Up @@ -449,40 +448,33 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// 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),
Some(eraser.fold_ty(rhs_ty)),
Some(rhs_expr),
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 `{missing_trait}` is not implemented for `{lhs_ty}`"
));
}
} else {
bug!("type param visitor stored a non type param: {:?}", ty.kind());
// Look for a TraitPredicate in the Fulfillment errors,
// and use it to generate a suggestion.
//
// Note that lookup_op_method must be called again but
// with a specific rhs_ty instead of a placeholder so
// the resulting predicate generates a more specific
// suggestion for the user.
let errors = self
.lookup_op_method(lhs_ty, &[rhs_ty], Op::Binary(op, is_assign))
.unwrap_err();
let predicates = errors
.into_iter()
.filter_map(|error| error.obligation.predicate.to_opt_poly_trait_pred())
.collect::<Vec<_>>();
if !predicates.is_empty() {
for pred in predicates {
self.infcx.suggest_restricting_param_bound(&mut err,
pred,
self.body_id,
);
}
} 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 `{missing_trait}` is not implemented for `{lhs_ty}`"
));
}
}
err.emit();
Expand Down Expand Up @@ -973,46 +965,6 @@ fn is_builtin_binop<'tcx>(lhs: Ty<'tcx>, rhs: Ty<'tcx>, op: hir::BinOp) -> bool
}
}

fn suggest_constraining_param(
tcx: TyCtxt<'_>,
body_id: hir::HirId,
mut err: &mut Diagnostic,
lhs_ty: Ty<'_>,
rhs_ty: Ty<'_>,
missing_trait: &str,
p: ty::ParamTy,
set_output: bool,
) {
let hir = tcx.hir();
let msg = &format!("`{lhs_ty}` might need a bound for `{missing_trait}`");
// Try to find the def-id and details for the parameter p. We have only the index,
// so we have to find the enclosing function's def-id, then look through its declared
// generic parameters to get the declaration.
let def_id = hir.body_owner_def_id(hir::BodyId { hir_id: body_id });
let generics = tcx.generics_of(def_id);
let param_def_id = generics.type_param(&p, tcx).def_id;
if let Some(generics) = param_def_id
.as_local()
.map(|id| hir.local_def_id_to_hir_id(id))
.and_then(|id| hir.find_by_def_id(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(
tcx,
generics,
&mut err,
&lhs_ty.to_string(),
&format!("{missing_trait}{output}"),
None,
);
} else {
let span = tcx.def_span(param_def_id);
err.span_label(span, msg);
}
}

struct TypeParamVisitor<'tcx>(Vec<Ty<'tcx>>);

impl<'tcx> TypeVisitor<'tcx> for TypeParamVisitor<'tcx> {
Expand Down
20 changes: 20 additions & 0 deletions src/test/ui/binop/issue-93927.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Regression test for #93927: suggested trait bound for T should be Eq, not PartialEq
struct MyType<T>(T);

impl<T> PartialEq for MyType<T>
where
T: Eq,
{
fn eq(&self, other: &Self) -> bool {
true
}
}

fn cond<T: PartialEq>(val: MyType<T>) -> bool {
val == val
//~^ ERROR binary operation `==` cannot be applied to type `MyType<T>`
}

fn main() {
cond(MyType(0));
}
16 changes: 16 additions & 0 deletions src/test/ui/binop/issue-93927.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error[E0369]: binary operation `==` cannot be applied to type `MyType<T>`
--> $DIR/issue-93927.rs:14:9
|
LL | val == val
| --- ^^ --- MyType<T>
| |
| MyType<T>
|
help: consider further restricting this bound
|
LL | fn cond<T: PartialEq + std::cmp::Eq>(val: MyType<T>) -> bool {
| ++++++++++++++

error: aborting due to previous error

For more information about this error, try `rustc --explain E0369`.
2 changes: 1 addition & 1 deletion src/test/ui/generic-associated-types/missing-bounds.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ impl<B: Add + Add<Output = B>> Add for C<B> {

struct D<B>(B);

impl<B: std::ops::Add<Output = B>> Add for D<B> {
impl<B: std::ops::Add> Add for D<B> {
type Output = Self;

fn add(self, rhs: Self) -> Self {
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/generic-associated-types/missing-bounds.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ LL | Self(self.0 + rhs.0)
|
help: consider restricting type parameter `B`
|
LL | impl<B: std::ops::Add<Output = B>> Add for D<B> {
| +++++++++++++++++++++++++++
LL | impl<B: std::ops::Add> Add for D<B> {
| +++++++++++++++

error[E0308]: mismatched types
--> $DIR/missing-bounds.rs:42:14
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/issues/issue-35668.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ LL | a.iter().map(|a| a*a)
| |
| &T
|
help: consider restricting type parameter `T`
help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement
|
LL | fn func<'a, T: std::ops::Mul<Output = &T>>(a: &'a [T]) -> impl Iterator<Item=&'a T> {
| ++++++++++++++++++++++++++++
LL | fn func<'a, T>(a: &'a [T]) -> impl Iterator<Item=&'a T> where &T: Mul<&T> {
| +++++++++++++++++

error: aborting due to previous error

Expand Down
5 changes: 4 additions & 1 deletion src/test/ui/suggestions/invalid-bin-op.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@ note: an implementation of `PartialEq<_>` might be missing for `S<T>`
|
LL | struct S<T>(T);
| ^^^^^^^^^^^^^^^ must implement `PartialEq<_>`
= note: the trait `std::cmp::PartialEq` is not implemented for `S<T>`
help: consider annotating `S<T>` with `#[derive(PartialEq)]`
|
LL | #[derive(PartialEq)]
|
help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement
|
LL | pub fn foo<T>(s: S<T>, t: S<T>) where S<T>: PartialEq {
| +++++++++++++++++++++

error: aborting due to previous error

Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/traits/resolution-in-overloaded-op.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ LL | a * b
| |
| &T
|
help: consider further restricting this bound
help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement
|
LL | fn foo<T: MyMul<f64, f64> + std::ops::Mul<Output = f64>>(a: &T, b: f64) -> f64 {
| +++++++++++++++++++++++++++++
LL | fn foo<T: MyMul<f64, f64>>(a: &T, b: f64) -> f64 where &T: Mul<f64> {
| ++++++++++++++++++

error: aborting due to previous error

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/type/type-check/missing_trait_impl.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ LL | let z = x + y;
|
help: consider restricting type parameter `T`
|
LL | fn foo<T: std::ops::Add<Output = T>>(x: T, y: T) {
| +++++++++++++++++++++++++++
LL | fn foo<T: std::ops::Add>(x: T, y: T) {
| +++++++++++++++

error[E0368]: binary assignment operation `+=` cannot be applied to type `T`
--> $DIR/missing_trait_impl.rs:9:5
Expand Down

0 comments on commit 4d0fe27

Please sign in to comment.