From 7f795a5221cac85fe2ab80527cee0c4ac7700943 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 27 Dec 2023 21:14:43 +0000 Subject: [PATCH 1/2] 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. --- .../rustc_hir_typeck/src/method/suggest.rs | 26 ++++++++++++++++--- compiler/rustc_hir_typeck/src/op.rs | 7 ++--- tests/ui/binop/binary-op-suggest-deref.stderr | 5 ---- tests/ui/issues/issue-62375.stderr | 5 ---- 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index 47fdd64796e9d..6937ddaafdd38 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -2252,6 +2252,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &self, err: &mut Diagnostic, errors: Vec>, + suggest_derive: bool, ) { let all_local_types_needing_impls = errors.iter().all(|e| match e.obligation.predicate.kind().skip_binder() { @@ -2322,10 +2323,15 @@ 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: &[( @@ -2333,7 +2339,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Option>, Option>, )], - ) { + ) -> Vec<(String, Span, String)> { let mut derives = Vec::<(String, Span, Symbol)>::new(); let mut traits = Vec::new(); for (pred, _, _) in unsatisfied_predicates { @@ -2419,6 +2425,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } + derives_grouped + } + + pub fn suggest_derive( + &self, + err: &mut Diagnostic, + unsatisfied_predicates: &[( + ty::Predicate<'tcx>, + Option>, + Option>, + )], + ) { + let derives_grouped = + self.note_predicate_source_and_get_derives(err, unsatisfied_predicates); for (self_name, self_span, traits) in &derives_grouped { err.span_suggestion_verbose( self_span.shrink_to_lo(), diff --git a/compiler/rustc_hir_typeck/src/op.rs b/compiler/rustc_hir_typeck/src/op.rs index de3d5f498d5b7..7b49a7cc009db 100644 --- a/compiler/rustc_hir_typeck/src/op.rs +++ b/compiler/rustc_hir_typeck/src/op.rs @@ -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 => { @@ -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) } }; @@ -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); } } } diff --git a/tests/ui/binop/binary-op-suggest-deref.stderr b/tests/ui/binop/binary-op-suggest-deref.stderr index f5de64e3ab1ae..68b5a24bf974d 100644 --- a/tests/ui/binop/binary-op-suggest-deref.stderr +++ b/tests/ui/binop/binary-op-suggest-deref.stderr @@ -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 diff --git a/tests/ui/issues/issue-62375.stderr b/tests/ui/issues/issue-62375.stderr index 8750fbcf4cf75..faca94a03f018 100644 --- a/tests/ui/issues/issue-62375.stderr +++ b/tests/ui/issues/issue-62375.stderr @@ -11,11 +11,6 @@ note: an implementation of `PartialEq A {A::Value}>` might be missing | LL | enum A { | ^^^^^^ must implement `PartialEq 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(/* () */); From 2474b3708ab78db73fc5e7c275069f0bd7aa4669 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 4 Jan 2024 00:12:37 +0000 Subject: [PATCH 2/2] review comments --- .../rustc_hir_typeck/src/method/suggest.rs | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index 6937ddaafdd38..8a179c5a440e8 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -2339,7 +2339,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Option>, Option>, )], - ) -> Vec<(String, Span, String)> { + ) -> Vec<(String, Span, Symbol)> { let mut derives = Vec::<(String, Span, Symbol)>::new(); let mut traits = Vec::new(); for (pred, _, _) in unsatisfied_predicates { @@ -2388,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 = @@ -2425,10 +2410,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } - derives_grouped + derives } - pub fn suggest_derive( + pub(crate) fn suggest_derive( &self, err: &mut Diagnostic, unsatisfied_predicates: &[( @@ -2437,8 +2422,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Option>, )], ) { - let derives_grouped = - self.note_predicate_source_and_get_derives(err, unsatisfied_predicates); + 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(),