Address comments

8 files changed, 131 insertions, 85 deletions

diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs
index 1a3229a0bda..4244e67482c 100644
--- a/compiler/rustc_ast_lowering/src/item.rs
+++ b/compiler/rustc_ast_lowering/src/item.rs
@@ -1378,7 +1378,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
         let mut params: SmallVec<[hir::GenericParam<'hir>; 4]> =
             self.lower_generic_params_mut(&generics.params).collect();
         let has_where_clause_predicates = !generics.where_clause.predicates.is_empty();
-        let has_where_clause_token = generics.where_clause.has_where_token;
         let where_clause_span = self.lower_span(generics.where_clause.span);
         let span = self.lower_span(generics.span);
         let res = f(self);
@@ -1397,7 +1396,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
             params: self.arena.alloc_from_iter(params),
             predicates: self.arena.alloc_from_iter(predicates),
             has_where_clause_predicates,
-            has_where_clause_token,
             where_clause_span,
             span,
         });
diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs
index bb34d02f5bc..6fe95a21fa4 100644
--- a/compiler/rustc_ast_lowering/src/lib.rs
+++ b/compiler/rustc_ast_lowering/src/lib.rs
@@ -1316,7 +1316,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                     params: lifetime_defs,
                     predicates: &[],
                     has_where_clause_predicates: false,
-                    has_where_clause_token: false,
                     where_clause_span: lctx.lower_span(span),
                     span: lctx.lower_span(span),
                 }),
@@ -1639,7 +1638,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                     params: generic_params,
                     predicates: &[],
                     has_where_clause_predicates: false,
-                    has_where_clause_token: false,
                     where_clause_span: this.lower_span(span),
                     span: this.lower_span(span),
                 }),
diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs
index 9d724a17cfc..bd5973f31cf 100644
--- a/compiler/rustc_hir/src/hir.rs
+++ b/compiler/rustc_hir/src/hir.rs
@@ -536,7 +536,6 @@ pub struct Generics<'hir> {
     pub params: &'hir [GenericParam<'hir>],
     pub predicates: &'hir [WherePredicate<'hir>],
     pub has_where_clause_predicates: bool,
-    pub has_where_clause_token: bool,
     pub where_clause_span: Span,
     pub span: Span,
 }
@@ -547,7 +546,6 @@ impl<'hir> Generics<'hir> {
             params: &[],
             predicates: &[],
             has_where_clause_predicates: false,
-            has_where_clause_token: false,
             where_clause_span: DUMMY_SP,
             span: DUMMY_SP,
         };
@@ -583,10 +581,6 @@ impl<'hir> Generics<'hir> {
         }
     }
 
-    pub fn where_clause_span(&self) -> Option<Span> {
-        if self.predicates.is_empty() { None } else { Some(self.where_clause_span) }
-    }
-
     /// `Span` where further predicates would be suggested, accounting for trailing commas, like
     ///  in `fn foo<T>(t: T) where T: Foo,` so we don't suggest two trailing commas.
     pub fn tail_span_for_predicate_suggestion(&self) -> Span {
@@ -607,10 +601,11 @@ impl<'hir> Generics<'hir> {
     pub fn add_where_or_trailing_comma(&self) -> &'static str {
         if self.has_where_clause_predicates {
             ","
-        } else if self.has_where_clause_token {
-            ""
-        } else {
+        } else if self.where_clause_span.is_empty() {
             " where"
+        } else {
+            // No where clause predicates, but we have `where` token
+            ""
         }
     }
 
diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs
index cd6f8055a5c..205ca72aae8 100644
--- a/compiler/rustc_lint/src/builtin.rs
+++ b/compiler/rustc_lint/src/builtin.rs
@@ -2295,9 +2295,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements {
             // (including the `where`)
             if hir_generics.has_where_clause_predicates && dropped_predicate_count == num_predicates
             {
-                let where_span = hir_generics
-                    .where_clause_span()
-                    .expect("span of (nonempty) where clause should exist");
+                let where_span = hir_generics.where_clause_span;
                 // Extend the where clause back to the closing `>` of the
                 // generics, except for tuple struct, which have the `where`
                 // after the fields of the struct.
diff --git a/compiler/rustc_middle/src/ty/diagnostics.rs b/compiler/rustc_middle/src/ty/diagnostics.rs
index 020c5e0ce42..1acded8c6e4 100644
--- a/compiler/rustc_middle/src/ty/diagnostics.rs
+++ b/compiler/rustc_middle/src/ty/diagnostics.rs
@@ -76,9 +76,13 @@ impl<'tcx> Ty<'tcx> {
 }
 
 pub trait IsSuggestable<'tcx> {
+    /// Whether this makes sense to suggest in a diagnostic.
+    ///
+    /// We filter out certain types and constants since they don't provide
+    /// meaningful rendered suggestions when pretty-printed. We leave some
+    /// nonsense, such as region vars, since those render as `'_` and are
+    /// usually okay to reinterpret as elided lifetimes.
     fn is_suggestable(self, tcx: TyCtxt<'tcx>) -> bool;
-
-    fn is_suggestable_modulo_impl_trait(self, tcx: TyCtxt<'tcx>, bound_str: &str) -> bool;
 }
 
 impl<'tcx, T> IsSuggestable<'tcx> for T
@@ -86,11 +90,7 @@ where
     T: TypeFoldable<'tcx>,
 {
     fn is_suggestable(self, tcx: TyCtxt<'tcx>) -> bool {
-        self.visit_with(&mut IsSuggestableVisitor { tcx, bound_str: None }).is_continue()
-    }
-
-    fn is_suggestable_modulo_impl_trait(self, tcx: TyCtxt<'tcx>, bound_str: &str) -> bool {
-        self.visit_with(&mut IsSuggestableVisitor { tcx, bound_str: Some(bound_str) }).is_continue()
+        self.visit_with(&mut IsSuggestableVisitor { tcx }).is_continue()
     }
 }
 
@@ -119,7 +119,7 @@ pub fn suggest_arbitrary_trait_bound<'tcx>(
         &format!(
             "consider {} `where` clause, but there might be an alternative better way to express \
              this requirement",
-            if generics.has_where_clause_token { "extending the" } else { "introducing a" },
+            if generics.where_clause_span.is_empty() { "introducing a" } else { "extending the" },
         ),
         format!("{} {}: {}", generics.add_where_or_trailing_comma(), param_name, constraint),
         Applicability::MaybeIncorrect,
@@ -417,12 +417,11 @@ impl<'v> hir::intravisit::Visitor<'v> for StaticLifetimeVisitor<'v> {
     }
 }
 
-pub struct IsSuggestableVisitor<'tcx, 's> {
+pub struct IsSuggestableVisitor<'tcx> {
     tcx: TyCtxt<'tcx>,
-    bound_str: Option<&'s str>,
 }
 
-impl<'tcx> TypeVisitor<'tcx> for IsSuggestableVisitor<'tcx, '_> {
+impl<'tcx> TypeVisitor<'tcx> for IsSuggestableVisitor<'tcx> {
     type BreakTy = ();
 
     fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
@@ -462,12 +461,13 @@ impl<'tcx> TypeVisitor<'tcx> for IsSuggestableVisitor<'tcx, '_> {
             }
 
             Param(param) => {
-                if let Some(found_bound_str) =
-                    param.name.as_str().strip_prefix("impl ").map(|s| s.trim_start())
-                {
-                    if self.bound_str.map_or(true, |bound_str| bound_str != found_bound_str) {
-                        return ControlFlow::Break(());
-                    }
+                // FIXME: It would be nice to make this not use string manipulation,
+                // but it's pretty hard to do this, since `ty::ParamTy` is missing
+                // sufficient info to determine if it is synthetic, and we don't
+                // always have a convenient way of getting `ty::Generics` at the call
+                // sites we invoke `IsSuggestable::is_suggestable`.
+                if param.name.as_str().starts_with("impl ") {
+                    return ControlFlow::Break(());
                 }
             }
 
diff --git a/compiler/rustc_middle/src/ty/generics.rs b/compiler/rustc_middle/src/ty/generics.rs
index 5fff840c39e..84547dca453 100644
--- a/compiler/rustc_middle/src/ty/generics.rs
+++ b/compiler/rustc_middle/src/ty/generics.rs
@@ -39,6 +39,13 @@ impl GenericParamDefKind {
             GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => true,
         }
     }
+
+    pub fn is_synthetic(&self) -> bool {
+        match self {
+            GenericParamDefKind::Type { synthetic, .. } => *synthetic,
+            _ => false,
+        }
+    }
 }
 
 #[derive(Clone, Debug, TyEncodable, TyDecodable, HashStable)]
diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
index adfa6e98568..353547a2fb8 100644
--- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
+++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
@@ -7,6 +7,7 @@ use crate::autoderef::Autoderef;
 use crate::infer::InferCtxt;
 use crate::traits::normalize_to;
 
+use hir::HirId;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_errors::{
@@ -23,7 +24,7 @@ use rustc_middle::hir::map;
 use rustc_middle::ty::{
     self, suggest_arbitrary_trait_bound, suggest_constraining_type_param, AdtKind, DefIdTree,
     GeneratorDiagnosticData, GeneratorInteriorTypeCause, Infer, InferTy, IsSuggestable,
-    ToPredicate, Ty, TyCtxt, TypeFoldable,
+    ToPredicate, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable,
 };
 use rustc_middle::ty::{TypeAndMut, TypeckResults};
 use rustc_session::Limit;
@@ -331,7 +332,8 @@ fn predicate_constraint(generics: &hir::Generics<'_>, pred: String) -> (Span, St
 /// param for cleaner code.
 fn suggest_restriction<'tcx>(
     tcx: TyCtxt<'tcx>,
-    generics: &hir::Generics<'tcx>,
+    hir_id: HirId,
+    hir_generics: &hir::Generics<'tcx>,
     msg: &str,
     err: &mut Diagnostic,
     fn_sig: Option<&hir::FnSig<'_>>,
@@ -344,24 +346,37 @@ fn suggest_restriction<'tcx>(
     //              &Ident
     super_traits: Option<(&Ident, &hir::GenericBounds<'_>)>,
 ) {
-    if generics.where_clause_span.from_expansion()
-        || generics.where_clause_span.desugaring_kind().is_some()
+    if hir_generics.where_clause_span.from_expansion()
+        || hir_generics.where_clause_span.desugaring_kind().is_some()
     {
         return;
     }
+    let Some(item_id) = hir_id.as_owner() else { return; };
+    let generics = tcx.generics_of(item_id);
     // Given `fn foo(t: impl Trait)` where `Trait` requires assoc type `A`...
-    if let Some((bound_str, fn_sig)) =
+    if let Some((param, bound_str, fn_sig)) =
         fn_sig.zip(projection).and_then(|(sig, p)| match p.self_ty().kind() {
             // Shenanigans to get the `Trait` from the `impl Trait`.
             ty::Param(param) => {
-                // `fn foo(t: impl Trait)`
-                //                 ^^^^^ get this string
-                param.name.as_str().strip_prefix("impl ").map(|s| (s.trim_start().to_string(), sig))
+                let param_def = generics.type_param(param, tcx);
+                if param_def.kind.is_synthetic() {
+                    let bound_str =
+                        param_def.name.as_str().strip_prefix("impl ")?.trim_start().to_string();
+                    return Some((param_def, bound_str, sig));
+                }
+                None
             }
             _ => None,
         })
     {
-        if !trait_pred.is_suggestable_modulo_impl_trait(tcx, &bound_str) {
+        let type_param_name = hir_generics.params.next_type_param_name(Some(&bound_str));
+        let trait_pred = trait_pred.fold_with(&mut ReplaceImplTraitFolder {
+            tcx,
+            param,
+            replace_ty: ty::ParamTy::new(generics.count() as u32, Symbol::intern(&type_param_name))
+                .to_ty(tcx),
+        });
+        if !trait_pred.is_suggestable(tcx) {
             return;
         }
         // We know we have an `impl Trait` that doesn't satisfy a required projection.
@@ -373,52 +388,21 @@ fn suggest_restriction<'tcx>(
         // where `T: Trait`.
         let mut ty_spans = vec![];
         for input in fn_sig.decl.inputs {
-            struct ReplaceImplTraitVisitor<'a> {
-                ty_spans: &'a mut Vec<Span>,
-                bound_str: &'a str,
-            }
-            impl<'a, 'hir> hir::intravisit::Visitor<'hir> for ReplaceImplTraitVisitor<'a> {
-                fn visit_ty(&mut self, t: &'hir hir::Ty<'hir>) {
-                    if let hir::TyKind::Path(hir::QPath::Resolved(
-                        None,
-                        hir::Path { segments: [segment], .. },
-                    )) = t.kind
-                    {
-                        if segment.ident.as_str().strip_prefix("impl ").map(|s| s.trim_start())
-                            == Some(self.bound_str)
-                        {
-                            // `fn foo(t: impl Trait)`
-                            //            ^^^^^^^^^^ get this to suggest `T` instead
-
-                            // There might be more than one `impl Trait`.
-                            self.ty_spans.push(t.span);
-                            return;
-                        }
-                    }
-                    hir::intravisit::walk_ty(self, t);
-                }
-            }
-            ReplaceImplTraitVisitor { ty_spans: &mut ty_spans, bound_str: &bound_str }
+            ReplaceImplTraitVisitor { ty_spans: &mut ty_spans, param_did: param.def_id }
                 .visit_ty(input);
         }
-
-        let type_param_name = generics.params.next_type_param_name(Some(&bound_str));
         // The type param `T: Trait` we will suggest to introduce.
         let type_param = format!("{}: {}", type_param_name, bound_str);
 
-        // FIXME: modify the `trait_pred` instead of string shenanigans.
-        // Turn `<impl Trait as Foo>::Bar: Qux` into `<T as Foo>::Bar: Qux`.
-        let pred = trait_pred.to_predicate(tcx).to_string();
-        let pred = pred.replace(&format!("impl {}", bound_str), &type_param_name);
         let mut sugg = vec![
-            if let Some(span) = generics.span_for_param_suggestion() {
+            if let Some(span) = hir_generics.span_for_param_suggestion() {
                 (span, format!(", {}", type_param))
             } else {
-                (generics.span, format!("<{}>", type_param))
+                (hir_generics.span, format!("<{}>", type_param))
             },
             // `fn foo(t: impl Trait)`
             //                       ^ suggest `where <T as Trait>::A: Bound`
-            predicate_constraint(generics, pred),
+            predicate_constraint(hir_generics, trait_pred.to_predicate(tcx).to_string()),
         ];
         sugg.extend(ty_spans.into_iter().map(|s| (s, type_param_name.to_string())));
 
@@ -436,13 +420,15 @@ fn suggest_restriction<'tcx>(
         }
         // Trivial case: `T` needs an extra bound: `T: Bound`.
         let (sp, suggestion) = match (
-            generics
+            hir_generics
                 .params
                 .iter()
                 .find(|p| !matches!(p.kind, hir::GenericParamKind::Type { synthetic: true, .. })),
             super_traits,
         ) {
-            (_, None) => predicate_constraint(generics, trait_pred.to_predicate(tcx).to_string()),
+            (_, None) => {
+                predicate_constraint(hir_generics, trait_pred.to_predicate(tcx).to_string())
+            }
             (None, Some((ident, []))) => (
                 ident.span.shrink_to_hi(),
                 format!(": {}", trait_pred.print_modifiers_and_trait_path()),
@@ -452,7 +438,7 @@ fn suggest_restriction<'tcx>(
                 format!(" + {}", trait_pred.print_modifiers_and_trait_path()),
             ),
             (Some(_), Some((_, []))) => (
-                generics.span.shrink_to_hi(),
+                hir_generics.span.shrink_to_hi(),
                 format!(": {}", trait_pred.print_modifiers_and_trait_path()),
             ),
         };
@@ -496,6 +482,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                     // Restricting `Self` for a single method.
                     suggest_restriction(
                         self.tcx,
+                        hir_id,
                         &generics,
                         "`Self`",
                         err,
@@ -515,7 +502,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                     assert!(param_ty);
                     // Restricting `Self` for a single method.
                     suggest_restriction(
-                        self.tcx, &generics, "`Self`", err, None, projection, trait_pred, None,
+                        self.tcx, hir_id, &generics, "`Self`", err, None, projection, trait_pred,
+                        None,
                     );
                     return;
                 }
@@ -536,6 +524,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                     // Missing restriction on associated type of type parameter (unmet projection).
                     suggest_restriction(
                         self.tcx,
+                        hir_id,
                         &generics,
                         "the associated type",
                         err,
@@ -555,6 +544,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                     // Missing restriction on associated type of type parameter (unmet projection).
                     suggest_restriction(
                         self.tcx,
+                        hir_id,
                         &generics,
                         "the associated type",
                         err,
@@ -583,6 +573,14 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                 | hir::Node::ImplItem(hir::ImplItem { generics, .. })
                     if param_ty =>
                 {
+                    // We skip the 0'th subst (self) because we do not want
+                    // to consider the predicate as not suggestible if the
+                    // self type is an arg position `impl Trait` -- instead,
+                    // we handle that by adding ` + Bound` below.
+                    // FIXME(compiler-errors): It would be nice to do the same
+                    // this that we do in `suggest_restriction` and pull the
+                    // `impl Trait` into a new generic if it shows up somewhere
+                    // else in the predicate.
                     if !trait_pred.skip_binder().trait_ref.substs[1..]
                         .iter()
                         .all(|g| g.is_suggestable(self.tcx))
@@ -2994,3 +2992,52 @@ fn suggest_trait_object_return_type_alternatives(
         );
     }
 }
+
+/// Collect the spans that we see the generic param `param_did`
+struct ReplaceImplTraitVisitor<'a> {
+    ty_spans: &'a mut Vec<Span>,
+    param_did: DefId,
+}
+
+impl<'a, 'hir> hir::intravisit::Visitor<'hir> for ReplaceImplTraitVisitor<'a> {
+    fn visit_ty(&mut self, t: &'hir hir::Ty<'hir>) {
+        if let hir::TyKind::Path(hir::QPath::Resolved(
+            None,
+            hir::Path { res: hir::def::Res::Def(_, segment_did), .. },
+        )) = t.kind
+        {
+            if self.param_did == *segment_did {
+                // `fn foo(t: impl Trait)`
+                //            ^^^^^^^^^^ get this to suggest `T` instead
+
+                // There might be more than one `impl Trait`.
+                self.ty_spans.push(t.span);
+                return;
+            }
+        }
+
+        hir::intravisit::walk_ty(self, t);
+    }
+}
+
+// Replace `param` with `replace_ty`
+struct ReplaceImplTraitFolder<'tcx> {
+    tcx: TyCtxt<'tcx>,
+    param: &'tcx ty::GenericParamDef,
+    replace_ty: Ty<'tcx>,
+}
+
+impl<'tcx> TypeFolder<'tcx> for ReplaceImplTraitFolder<'tcx> {
+    fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> {
+        if let ty::Param(ty::ParamTy { index, .. }) = t.kind() {
+            if self.param.index == *index {
+                return self.replace_ty;
+            }
+        }
+        t.super_fold_with(self)
+    }
+
+    fn tcx(&self) -> TyCtxt<'tcx> {
+        self.tcx
+    }
+}
diff --git a/compiler/rustc_typeck/src/lib.rs b/compiler/rustc_typeck/src/lib.rs
index 2fc9705527b..d613edf0ab0 100644
--- a/compiler/rustc_typeck/src/lib.rs
+++ b/compiler/rustc_typeck/src/lib.rs
@@ -211,7 +211,7 @@ fn check_main_fn_ty(tcx: TyCtxt<'_>, main_def_id: DefId) {
         let hir_id = tcx.hir().local_def_id_to_hir_id(def_id.expect_local());
         match tcx.hir().find(hir_id) {
             Some(Node::Item(hir::Item { kind: hir::ItemKind::Fn(_, ref generics, _), .. })) => {
-                generics.where_clause_span()
+                Some(generics.where_clause_span)
             }
             _ => {
                 span_bug!(tcx.def_span(def_id), "main has a non-function type");
@@ -401,14 +401,17 @@ fn check_start_fn_ty(tcx: TyCtxt<'_>, start_def_id: DefId) {
                         .emit();
                         error = true;
                     }
-                    if let Some(sp) = generics.where_clause_span() {
+                    if generics.has_where_clause_predicates {
                         struct_span_err!(
                             tcx.sess,
-                            sp,
+                            generics.where_clause_span,
                             E0647,
                             "start function is not allowed to have a `where` clause"
                         )
-                        .span_label(sp, "start function cannot have a `where` clause")
+                        .span_label(
+                            generics.where_clause_span,
+                            "start function cannot have a `where` clause",
+                        )
                         .emit();
                         error = true;
                     }