Skip to content

Commit

Permalink
Rollup merge of #119362 - estebank:restrict-derive-suggestion, r=fmease
Browse files Browse the repository at this point in the history
Make `derive(Trait)` suggestion more accurate

Only suggest `derive(PartialEq)` when both LHS and RHS types are the same, otherwise the suggestion is not useful.
  • Loading branch information
matthiaskrgr authored Jan 4, 2024
2 parents 3fb61ce + 2474b37 commit 6526b16
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 31 deletions.
55 changes: 37 additions & 18 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2252,6 +2252,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
err: &mut Diagnostic,
errors: Vec<FulfillmentError<'tcx>>,
suggest_derive: bool,
) {
let all_local_types_needing_impls =
errors.iter().all(|e| match e.obligation.predicate.kind().skip_binder() {
Expand Down Expand Up @@ -2322,18 +2323,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.iter()
.map(|e| (e.obligation.predicate, None, Some(e.obligation.cause.clone())))
.collect();
self.suggest_derive(err, &preds);
if suggest_derive {
self.suggest_derive(err, &preds);
} else {
// The predicate comes from a binop where the lhs and rhs have different types.
let _ = self.note_predicate_source_and_get_derives(err, &preds);
}
}

pub fn suggest_derive(
fn note_predicate_source_and_get_derives(
&self,
err: &mut Diagnostic,
unsatisfied_predicates: &[(
ty::Predicate<'tcx>,
Option<ty::Predicate<'tcx>>,
Option<ObligationCause<'tcx>>,
)],
) {
) -> Vec<(String, Span, Symbol)> {
let mut derives = Vec::<(String, Span, Symbol)>::new();
let mut traits = Vec::new();
for (pred, _, _) in unsatisfied_predicates {
Expand Down Expand Up @@ -2382,21 +2388,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
traits.sort();
traits.dedup();

derives.sort();
derives.dedup();

let mut derives_grouped = Vec::<(String, Span, String)>::new();
for (self_name, self_span, trait_name) in derives.into_iter() {
if let Some((last_self_name, _, ref mut last_trait_names)) = derives_grouped.last_mut()
{
if last_self_name == &self_name {
last_trait_names.push_str(format!(", {trait_name}").as_str());
continue;
}
}
derives_grouped.push((self_name, self_span, trait_name.to_string()));
}

let len = traits.len();
if len > 0 {
let span =
Expand All @@ -2419,6 +2410,34 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);
}

derives
}

pub(crate) fn suggest_derive(
&self,
err: &mut Diagnostic,
unsatisfied_predicates: &[(
ty::Predicate<'tcx>,
Option<ty::Predicate<'tcx>>,
Option<ObligationCause<'tcx>>,
)],
) {
let mut derives = self.note_predicate_source_and_get_derives(err, unsatisfied_predicates);
derives.sort();
derives.dedup();

let mut derives_grouped = Vec::<(String, Span, String)>::new();
for (self_name, self_span, trait_name) in derives.into_iter() {
if let Some((last_self_name, _, ref mut last_trait_names)) = derives_grouped.last_mut()
{
if last_self_name == &self_name {
last_trait_names.push_str(format!(", {trait_name}").as_str());
continue;
}
}
derives_grouped.push((self_name, self_span, trait_name.to_string()));
}

for (self_name, self_span, traits) in &derives_grouped {
err.span_suggestion_verbose(
self_span.shrink_to_lo(),
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_hir_typeck/src/op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
lhs_expr.span,
format!("cannot use `{}=` on type `{}`", op.node.as_str(), lhs_ty),
);
self.note_unmet_impls_on_type(&mut err, errors);
self.note_unmet_impls_on_type(&mut err, errors, false);
(err, None)
}
IsAssign::No => {
Expand Down Expand Up @@ -375,7 +375,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
err.span_label(lhs_expr.span, lhs_ty.to_string());
err.span_label(rhs_expr.span, rhs_ty.to_string());
}
self.note_unmet_impls_on_type(&mut err, errors);
let suggest_derive = self.can_eq(self.param_env, lhs_ty, rhs_ty);
self.note_unmet_impls_on_type(&mut err, errors, suggest_derive);
(err, output_def_id)
}
};
Expand Down Expand Up @@ -852,7 +853,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Str | Never | Char | Tuple(_) | Array(_, _) => {}
Ref(_, lty, _) if *lty.kind() == Str => {}
_ => {
self.note_unmet_impls_on_type(&mut err, errors);
self.note_unmet_impls_on_type(&mut err, errors, true);
}
}
}
Expand Down
5 changes: 0 additions & 5 deletions tests/ui/binop/binary-op-suggest-deref.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -270,11 +270,6 @@ note: an implementation of `PartialEq<&&{integer}>` might be missing for `Foo`
|
LL | struct Foo;
| ^^^^^^^^^^ must implement `PartialEq<&&{integer}>`
help: consider annotating `Foo` with `#[derive(PartialEq)]`
|
LL + #[derive(PartialEq)]
LL | struct Foo;
|

error[E0277]: can't compare `&String` with `str`
--> $DIR/binary-op-suggest-deref.rs:69:20
Expand Down
5 changes: 0 additions & 5 deletions tests/ui/issues/issue-62375.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,6 @@ note: an implementation of `PartialEq<fn(()) -> A {A::Value}>` might be missing
|
LL | enum A {
| ^^^^^^ must implement `PartialEq<fn(()) -> A {A::Value}>`
help: consider annotating `A` with `#[derive(PartialEq)]`
|
LL + #[derive(PartialEq)]
LL | enum A {
|
help: use parentheses to construct this tuple variant
|
LL | a == A::Value(/* () */);
Expand Down

0 comments on commit 6526b16

Please sign in to comment.