From 01d784131f0a11d19d0ab5398b00ca6bbfd283b1 Mon Sep 17 00:00:00 2001
From: Michael Goulet <michael@errs.io>
Date: Wed, 28 Dec 2022 06:35:34 +0000
Subject: [PATCH 1/3] Make trait/impl where clause mismatch on region error a
 bit more actionable

---
 .../src/infer/error_reporting/note.rs         | 129 ++++++++++++------
 .../ui/compare-method/region-extra-2.stderr   |   5 +
 .../ui/compare-method/region-extra.stderr     |   6 +
 .../impl_bounds.stderr                        |   5 +
 .../issue-90014.stderr                        |   6 +-
 .../issue-91883.stderr                        |   6 +-
 .../issue-92033.stderr                        |   6 +-
 .../missing-where-clause-on-trait.stderr      |   6 +
 8 files changed, 127 insertions(+), 42 deletions(-)

diff --git a/compiler/rustc_infer/src/infer/error_reporting/note.rs b/compiler/rustc_infer/src/infer/error_reporting/note.rs
index d2dffa4a0b78e..891c77d18109c 100644
--- a/compiler/rustc_infer/src/infer/error_reporting/note.rs
+++ b/compiler/rustc_infer/src/infer/error_reporting/note.rs
@@ -2,11 +2,14 @@ use crate::errors::RegionOriginNote;
 use crate::infer::error_reporting::{note_and_explain_region, TypeErrCtxt};
 use crate::infer::{self, SubregionOrigin};
 use rustc_errors::{
-    fluent, struct_span_err, AddToDiagnostic, Diagnostic, DiagnosticBuilder, ErrorGuaranteed,
+    fluent, struct_span_err, AddToDiagnostic, Applicability, Diagnostic, DiagnosticBuilder,
+    ErrorGuaranteed,
 };
+use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_middle::traits::ObligationCauseCode;
 use rustc_middle::ty::error::TypeError;
-use rustc_middle::ty::{self, Region};
+use rustc_middle::ty::{self, IsSuggestable, Region};
+use rustc_span::symbol::kw;
 
 use super::ObligationCauseAsDiagArg;
 
@@ -313,55 +316,43 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                 );
                 err
             }
-            infer::CompareImplItemObligation { span, impl_item_def_id, trait_item_def_id } => self
-                .report_extra_impl_obligation(
+            infer::CompareImplItemObligation { span, impl_item_def_id, trait_item_def_id } => {
+                let mut err = self.report_extra_impl_obligation(
                     span,
                     impl_item_def_id,
                     trait_item_def_id,
                     &format!("`{}: {}`", sup, sub),
-                ),
+                );
+                // We should only suggest rewriting the `where` clause if the predicate is within that `where` clause
+                if self
+                    .tcx
+                    .hir()
+                    .get_generics(impl_item_def_id)
+                    .unwrap()
+                    .where_clause_span
+                    .contains(span)
+                {
+                    self.suggest_copy_trait_method_bounds(
+                        trait_item_def_id,
+                        impl_item_def_id,
+                        &mut err,
+                    );
+                }
+                err
+            }
             infer::CheckAssociatedTypeBounds { impl_item_def_id, trait_item_def_id, parent } => {
                 let mut err = self.report_concrete_failure(*parent, sub, sup);
-
                 let trait_item_span = self.tcx.def_span(trait_item_def_id);
                 let item_name = self.tcx.item_name(impl_item_def_id.to_def_id());
                 err.span_label(
                     trait_item_span,
                     format!("definition of `{}` from trait", item_name),
                 );
-
-                let trait_predicates = self.tcx.explicit_predicates_of(trait_item_def_id);
-                let impl_predicates = self.tcx.explicit_predicates_of(impl_item_def_id);
-
-                let impl_predicates: rustc_data_structures::fx::FxHashSet<_> =
-                    impl_predicates.predicates.into_iter().map(|(pred, _)| pred).collect();
-                let clauses: Vec<_> = trait_predicates
-                    .predicates
-                    .into_iter()
-                    .filter(|&(pred, _)| !impl_predicates.contains(pred))
-                    .map(|(pred, _)| format!("{}", pred))
-                    .collect();
-
-                if !clauses.is_empty() {
-                    let generics = self.tcx.hir().get_generics(impl_item_def_id).unwrap();
-                    let where_clause_span = generics.tail_span_for_predicate_suggestion();
-
-                    let suggestion = format!(
-                        "{} {}",
-                        generics.add_where_or_trailing_comma(),
-                        clauses.join(", "),
-                    );
-                    err.span_suggestion(
-                        where_clause_span,
-                        &format!(
-                            "try copying {} from the trait",
-                            if clauses.len() > 1 { "these clauses" } else { "this clause" }
-                        ),
-                        suggestion,
-                        rustc_errors::Applicability::MaybeIncorrect,
-                    );
-                }
-
+                self.suggest_copy_trait_method_bounds(
+                    trait_item_def_id,
+                    impl_item_def_id,
+                    &mut err,
+                );
                 err
             }
             infer::AscribeUserTypeProvePredicate(span) => {
@@ -388,6 +379,66 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
         }
     }
 
+    pub fn suggest_copy_trait_method_bounds(
+        &self,
+        trait_item_def_id: DefId,
+        impl_item_def_id: LocalDefId,
+        err: &mut Diagnostic,
+    ) {
+        // FIXME(compiler-errors): Right now this is only being used for region
+        // predicate mismatches. Ideally, we'd use it for *all* predicate mismatches,
+        // but right now it's not really very smart when it comes to implicit `Sized`
+        // predicates and bounds on the trait itself.
+
+        let impl_def_id =
+            self.tcx.associated_item(impl_item_def_id).impl_container(self.tcx).unwrap();
+        let trait_substs = self
+            .tcx
+            .impl_trait_ref(impl_def_id)
+            .unwrap()
+            // Replace the explicit self type with `Self` for better suggestion rendering
+            .with_self_ty(self.tcx, self.tcx.mk_ty_param(0, kw::SelfUpper))
+            .substs;
+        let trait_item_substs =
+            ty::InternalSubsts::identity_for_item(self.tcx, impl_item_def_id.to_def_id())
+                .rebase_onto(self.tcx, impl_def_id, trait_substs);
+
+        let mut is_suggestable = true;
+        let trait_predicates = self
+            .tcx
+            .bound_explicit_predicates_of(trait_item_def_id)
+            .map_bound(|p| p.predicates)
+            .subst_iter_copied(self.tcx, trait_item_substs)
+            .map(|(pred, _)| {
+                if !pred.is_suggestable(self.tcx, false) {
+                    is_suggestable = false;
+                }
+                pred.to_string()
+            })
+            .collect::<Vec<_>>();
+
+        let generics = self.tcx.hir().get_generics(impl_item_def_id).unwrap();
+
+        if is_suggestable {
+            if trait_predicates.is_empty() {
+                err.span_suggestion_verbose(
+                    generics.where_clause_span,
+                    "remove the `where` clause",
+                    String::new(),
+                    Applicability::MachineApplicable,
+                );
+            } else {
+                let space = if generics.where_clause_span.is_empty() { " " } else { "" };
+                err.span_suggestion_verbose(
+                    generics.where_clause_span,
+                    "copy the `where` clause predicates from the trait",
+                    format!("{space}where {}", trait_predicates.join(", ")),
+                    Applicability::MachineApplicable,
+                );
+            }
+        }
+    }
+
     pub(super) fn report_placeholder_failure(
         &self,
         placeholder_origin: SubregionOrigin<'tcx>,
diff --git a/src/test/ui/compare-method/region-extra-2.stderr b/src/test/ui/compare-method/region-extra-2.stderr
index f01d7f4710c38..eb19d57ab05ac 100644
--- a/src/test/ui/compare-method/region-extra-2.stderr
+++ b/src/test/ui/compare-method/region-extra-2.stderr
@@ -6,6 +6,11 @@ LL |     fn renew<'b: 'a>(self) -> &'b mut [T];
 ...
 LL |     fn renew<'b: 'a>(self) -> &'b mut [T] where 'a: 'b {
    |                                                     ^^ impl has extra requirement `'a: 'b`
+   |
+help: copy the `where` clause predicates from the trait
+   |
+LL |     fn renew<'b: 'a>(self) -> &'b mut [T] where 'b: 'a {
+   |                                           ~~~~~~~~~~~~
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/compare-method/region-extra.stderr b/src/test/ui/compare-method/region-extra.stderr
index 4a3af65e9042b..1a471e18d9dc1 100644
--- a/src/test/ui/compare-method/region-extra.stderr
+++ b/src/test/ui/compare-method/region-extra.stderr
@@ -6,6 +6,12 @@ LL |     fn foo();
 ...
 LL |     fn foo() where 'a: 'b { }
    |                        ^^ impl has extra requirement `'a: 'b`
+   |
+help: remove the `where` clause
+   |
+LL -     fn foo() where 'a: 'b { }
+LL +     fn foo()  { }
+   |
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/generic-associated-types/impl_bounds.stderr b/src/test/ui/generic-associated-types/impl_bounds.stderr
index 3acd85c8ac6ba..3456b345cc28c 100644
--- a/src/test/ui/generic-associated-types/impl_bounds.stderr
+++ b/src/test/ui/generic-associated-types/impl_bounds.stderr
@@ -15,6 +15,11 @@ LL |     type B<'a, 'b> where 'a: 'b;
 ...
 LL |     type B<'a, 'b> = (&'a(), &'b ()) where 'b: 'a;
    |                                                ^^ impl has extra requirement `'b: 'a`
+   |
+help: copy the `where` clause predicates from the trait
+   |
+LL |     type B<'a, 'b> = (&'a(), &'b ()) where 'a: 'b;
+   |                                      ~~~~~~~~~~~~
 
 error[E0277]: the trait bound `T: Copy` is not satisfied
   --> $DIR/impl_bounds.rs:18:33
diff --git a/src/test/ui/generic-associated-types/issue-90014.stderr b/src/test/ui/generic-associated-types/issue-90014.stderr
index 2d3f4a6af7e0b..b4b1bc7da7f45 100644
--- a/src/test/ui/generic-associated-types/issue-90014.stderr
+++ b/src/test/ui/generic-associated-types/issue-90014.stderr
@@ -5,13 +5,17 @@ LL |     type Fut<'a> where Self: 'a;
    |     ------------ definition of `Fut` from trait
 ...
 LL |     type Fut<'a> = impl Future<Output = ()>;
-   |                    ^^^^^^^^^^^^^^^^^^^^^^^^- help: try copying this clause from the trait: `where Self: 'a`
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^
    |
 note: type must outlive the lifetime `'a` as defined here
   --> $DIR/issue-90014.rs:13:14
    |
 LL |     type Fut<'a> = impl Future<Output = ()>;
    |              ^^
+help: copy the `where` clause predicates from the trait
+   |
+LL |     type Fut<'a> = impl Future<Output = ()> where Self: 'a;
+   |                                             ++++++++++++++
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/generic-associated-types/issue-91883.stderr b/src/test/ui/generic-associated-types/issue-91883.stderr
index 1cfc2aaf1613f..d5db962094ce9 100644
--- a/src/test/ui/generic-associated-types/issue-91883.stderr
+++ b/src/test/ui/generic-associated-types/issue-91883.stderr
@@ -5,7 +5,7 @@ LL |     type Cursor<'tx>: Cursor<'tx>
    |     ----------------------------- definition of `Cursor` from trait
 ...
 LL |     type Cursor<'tx> = CursorImpl<'tx>;
-   |                        ^^^^^^^^^^^^^^^- help: try copying these clauses from the trait: `where 'db: 'tx, Self: 'tx`
+   |                        ^^^^^^^^^^^^^^^
    |
 note: lifetime parameter instantiated with the lifetime `'db` as defined here
   --> $DIR/issue-91883.rs:29:6
@@ -17,6 +17,10 @@ note: but lifetime parameter must outlive the lifetime `'tx` as defined here
    |
 LL |     type Cursor<'tx> = CursorImpl<'tx>;
    |                 ^^^
+help: copy the `where` clause predicates from the trait
+   |
+LL |     type Cursor<'tx> = CursorImpl<'tx> where 'db: 'tx, Self: 'tx;
+   |                                        +++++++++++++++++++++++++
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/generic-associated-types/issue-92033.stderr b/src/test/ui/generic-associated-types/issue-92033.stderr
index cd7eed25421e8..ddc420a7b4e61 100644
--- a/src/test/ui/generic-associated-types/issue-92033.stderr
+++ b/src/test/ui/generic-associated-types/issue-92033.stderr
@@ -5,13 +5,17 @@ LL |     type TextureIter<'a>: Iterator<Item = &'a Texture>
    |     -------------------------------------------------- definition of `TextureIter` from trait
 ...
 LL |     type TextureIter<'a> = std::option::IntoIter<&'a Texture>;
-   |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- help: try copying this clause from the trait: `where Self: 'a`
+   |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 note: type must outlive the lifetime `'a` as defined here
   --> $DIR/issue-92033.rs:20:22
    |
 LL |     type TextureIter<'a> = std::option::IntoIter<&'a Texture>;
    |                      ^^
+help: copy the `where` clause predicates from the trait
+   |
+LL |     type TextureIter<'a> = std::option::IntoIter<&'a Texture> where Self: 'a;
+   |                                                               ++++++++++++++
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/generic-associated-types/missing-where-clause-on-trait.stderr b/src/test/ui/generic-associated-types/missing-where-clause-on-trait.stderr
index ffdba6676bf8d..8a71fc73a9d2d 100644
--- a/src/test/ui/generic-associated-types/missing-where-clause-on-trait.stderr
+++ b/src/test/ui/generic-associated-types/missing-where-clause-on-trait.stderr
@@ -6,6 +6,12 @@ LL |     type Assoc<'a, 'b>;
 ...
 LL |     type Assoc<'a, 'b> = () where 'a: 'b;
    |                                       ^^ impl has extra requirement `'a: 'b`
+   |
+help: remove the `where` clause
+   |
+LL -     type Assoc<'a, 'b> = () where 'a: 'b;
+LL +     type Assoc<'a, 'b> = () ;
+   |
 
 error: aborting due to previous error
 

From 992ba801c271aeb087b1619467c487e35bae4e65 Mon Sep 17 00:00:00 2001
From: Michael Goulet <michael@errs.io>
Date: Wed, 28 Dec 2022 06:44:29 +0000
Subject: [PATCH 2/3] Add test for bad suggestion

---
 .../mismatched-where-clause-regions.rs          | 12 ++++++++++++
 .../mismatched-where-clause-regions.stderr      | 17 +++++++++++++++++
 2 files changed, 29 insertions(+)
 create mode 100644 src/test/ui/generic-associated-types/mismatched-where-clause-regions.rs
 create mode 100644 src/test/ui/generic-associated-types/mismatched-where-clause-regions.stderr

diff --git a/src/test/ui/generic-associated-types/mismatched-where-clause-regions.rs b/src/test/ui/generic-associated-types/mismatched-where-clause-regions.rs
new file mode 100644
index 0000000000000..8caf5317693ee
--- /dev/null
+++ b/src/test/ui/generic-associated-types/mismatched-where-clause-regions.rs
@@ -0,0 +1,12 @@
+trait Foo {
+    type T<'a1, 'b1>
+    where
+        'a1: 'b1;
+}
+
+impl Foo for () {
+    type T<'a2, 'b2> = () where 'b2: 'a2;
+    //~^ ERROR impl has stricter requirements than trait
+}
+
+fn main() {}
diff --git a/src/test/ui/generic-associated-types/mismatched-where-clause-regions.stderr b/src/test/ui/generic-associated-types/mismatched-where-clause-regions.stderr
new file mode 100644
index 0000000000000..91a0300764084
--- /dev/null
+++ b/src/test/ui/generic-associated-types/mismatched-where-clause-regions.stderr
@@ -0,0 +1,17 @@
+error[E0276]: impl has stricter requirements than trait
+  --> $DIR/mismatched-where-clause-regions.rs:8:38
+   |
+LL |     type T<'a1, 'b1>
+   |     ---------------- definition of `T` from trait
+...
+LL |     type T<'a2, 'b2> = () where 'b2: 'a2;
+   |                                      ^^^ impl has extra requirement `'b2: 'a2`
+   |
+help: copy the `where` clause predicates from the trait
+   |
+LL |     type T<'a2, 'b2> = () where 'a2: 'b2;
+   |                           ~~~~~~~~~~~~~~
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0276`.

From 6e794dcc8bc9435bf979d8d5cd6af465ab3004b8 Mon Sep 17 00:00:00 2001
From: Michael Goulet <michael@errs.io>
Date: Wed, 28 Dec 2022 18:35:16 +0000
Subject: [PATCH 3/3] Address review comments

---
 .../src/infer/error_reporting/note.rs         | 64 +++++++++----------
 1 file changed, 29 insertions(+), 35 deletions(-)

diff --git a/compiler/rustc_infer/src/infer/error_reporting/note.rs b/compiler/rustc_infer/src/infer/error_reporting/note.rs
index 891c77d18109c..d91ef882bc4b8 100644
--- a/compiler/rustc_infer/src/infer/error_reporting/note.rs
+++ b/compiler/rustc_infer/src/infer/error_reporting/note.rs
@@ -324,13 +324,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                     &format!("`{}: {}`", sup, sub),
                 );
                 // We should only suggest rewriting the `where` clause if the predicate is within that `where` clause
-                if self
-                    .tcx
-                    .hir()
-                    .get_generics(impl_item_def_id)
-                    .unwrap()
-                    .where_clause_span
-                    .contains(span)
+                if let Some(generics) = self.tcx.hir().get_generics(impl_item_def_id)
+                    && generics.where_clause_span.contains(span)
                 {
                     self.suggest_copy_trait_method_bounds(
                         trait_item_def_id,
@@ -390,12 +385,13 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
         // but right now it's not really very smart when it comes to implicit `Sized`
         // predicates and bounds on the trait itself.
 
-        let impl_def_id =
-            self.tcx.associated_item(impl_item_def_id).impl_container(self.tcx).unwrap();
-        let trait_substs = self
+        let Some(impl_def_id) =
+            self.tcx.associated_item(impl_item_def_id).impl_container(self.tcx) else { return; };
+        let Some(trait_ref) = self
             .tcx
             .impl_trait_ref(impl_def_id)
-            .unwrap()
+            else { return; };
+        let trait_substs = trait_ref
             // Replace the explicit self type with `Self` for better suggestion rendering
             .with_self_ty(self.tcx, self.tcx.mk_ty_param(0, kw::SelfUpper))
             .substs;
@@ -403,39 +399,37 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
             ty::InternalSubsts::identity_for_item(self.tcx, impl_item_def_id.to_def_id())
                 .rebase_onto(self.tcx, impl_def_id, trait_substs);
 
-        let mut is_suggestable = true;
-        let trait_predicates = self
+        let Ok(trait_predicates) = self
             .tcx
             .bound_explicit_predicates_of(trait_item_def_id)
             .map_bound(|p| p.predicates)
             .subst_iter_copied(self.tcx, trait_item_substs)
             .map(|(pred, _)| {
-                if !pred.is_suggestable(self.tcx, false) {
-                    is_suggestable = false;
+                if pred.is_suggestable(self.tcx, false) {
+                    Ok(pred.to_string())
+                } else {
+                    Err(())
                 }
-                pred.to_string()
             })
-            .collect::<Vec<_>>();
+            .collect::<Result<Vec<_>, ()>>() else { return; };
 
-        let generics = self.tcx.hir().get_generics(impl_item_def_id).unwrap();
+        let Some(generics) = self.tcx.hir().get_generics(impl_item_def_id) else { return; };
 
-        if is_suggestable {
-            if trait_predicates.is_empty() {
-                err.span_suggestion_verbose(
-                    generics.where_clause_span,
-                    "remove the `where` clause",
-                    String::new(),
-                    Applicability::MachineApplicable,
-                );
-            } else {
-                let space = if generics.where_clause_span.is_empty() { " " } else { "" };
-                err.span_suggestion_verbose(
-                    generics.where_clause_span,
-                    "copy the `where` clause predicates from the trait",
-                    format!("{space}where {}", trait_predicates.join(", ")),
-                    Applicability::MachineApplicable,
-                );
-            }
+        if trait_predicates.is_empty() {
+            err.span_suggestion_verbose(
+                generics.where_clause_span,
+                "remove the `where` clause",
+                String::new(),
+                Applicability::MachineApplicable,
+            );
+        } else {
+            let space = if generics.where_clause_span.is_empty() { " " } else { "" };
+            err.span_suggestion_verbose(
+                generics.where_clause_span,
+                "copy the `where` clause predicates from the trait",
+                format!("{space}where {}", trait_predicates.join(", ")),
+                Applicability::MachineApplicable,
+            );
         }
     }