Auto merge of #87421 - estebank:perf-run, r=oli-obk

Do not discard `?Sized` type params and suggest their removal

9 files changed, 145 insertions, 17 deletions

diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs
index 0fddb0ee4cc..3acf69ec2b7 100644
--- a/compiler/rustc_ast_lowering/src/item.rs
+++ b/compiler/rustc_ast_lowering/src/item.rs
@@ -1443,16 +1443,19 @@ impl<'hir> LoweringContext<'_, 'hir> {
                             ImplTraitContext::disallowed(),
                         ),
                         bounded_ty: this.lower_ty(bounded_ty, ImplTraitContext::disallowed()),
-                        bounds: this.arena.alloc_from_iter(bounds.iter().filter_map(|bound| {
-                            match *bound {
-                                // Ignore `?Trait` bounds.
-                                // They were copied into type parameters already.
-                                GenericBound::Trait(_, TraitBoundModifier::Maybe) => None,
-                                _ => Some(
-                                    this.lower_param_bound(bound, ImplTraitContext::disallowed()),
-                                ),
-                            }
-                        })),
+                        bounds: this.arena.alloc_from_iter(bounds.iter().map(
+                            |bound| match bound {
+                                // We used to ignore `?Trait` bounds, as they were copied into type
+                                // parameters already, but we need to keep them around only for
+                                // diagnostics when we suggest removal of `?Sized` bounds. See
+                                // `suggest_constraining_type_param`. This will need to change if
+                                // we ever allow something *other* than `?Sized`.
+                                GenericBound::Trait(p, TraitBoundModifier::Maybe) => {
+                                    hir::GenericBound::Unsized(p.span)
+                                }
+                                _ => this.lower_param_bound(bound, ImplTraitContext::disallowed()),
+                            },
+                        )),
                         span,
                     })
                 })
diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs
index 3bd42ba6090..581f177ad14 100644
--- a/compiler/rustc_ast_lowering/src/lib.rs
+++ b/compiler/rustc_ast_lowering/src/lib.rs
@@ -2160,12 +2160,12 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         tpb: &GenericBound,
         itctx: ImplTraitContext<'_, 'hir>,
     ) -> hir::GenericBound<'hir> {
-        match *tpb {
-            GenericBound::Trait(ref ty, modifier) => hir::GenericBound::Trait(
-                self.lower_poly_trait_ref(ty, itctx),
-                self.lower_trait_bound_modifier(modifier),
+        match tpb {
+            GenericBound::Trait(p, modifier) => hir::GenericBound::Trait(
+                self.lower_poly_trait_ref(p, itctx),
+                self.lower_trait_bound_modifier(*modifier),
             ),
-            GenericBound::Outlives(ref lifetime) => {
+            GenericBound::Outlives(lifetime) => {
                 hir::GenericBound::Outlives(self.lower_lifetime(lifetime))
             }
         }
diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs
index db7fe6cb12f..aac5d296f17 100644
--- a/compiler/rustc_hir/src/hir.rs
+++ b/compiler/rustc_hir/src/hir.rs
@@ -442,6 +442,7 @@ pub enum GenericBound<'hir> {
     Trait(PolyTraitRef<'hir>, TraitBoundModifier),
     // FIXME(davidtwco): Introduce `PolyTraitRef::LangItem`
     LangItemTrait(LangItem, Span, HirId, &'hir GenericArgs<'hir>),
+    Unsized(Span),
     Outlives(Lifetime),
 }
 
@@ -458,6 +459,7 @@ impl GenericBound<'_> {
             GenericBound::Trait(t, ..) => t.span,
             GenericBound::LangItemTrait(_, span, ..) => *span,
             GenericBound::Outlives(l) => l.span,
+            GenericBound::Unsized(span) => *span,
         }
     }
 }
diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs
index 17835493cda..ae186d66004 100644
--- a/compiler/rustc_hir/src/intravisit.rs
+++ b/compiler/rustc_hir/src/intravisit.rs
@@ -889,6 +889,7 @@ pub fn walk_param_bound<'v, V: Visitor<'v>>(visitor: &mut V, bound: &'v GenericB
             visitor.visit_generic_args(span, args);
         }
         GenericBound::Outlives(ref lifetime) => visitor.visit_lifetime(lifetime),
+        GenericBound::Unsized(_) => {}
     }
 }
 
diff --git a/compiler/rustc_hir_pretty/src/lib.rs b/compiler/rustc_hir_pretty/src/lib.rs
index 90ceb1d5c91..2b372392575 100644
--- a/compiler/rustc_hir_pretty/src/lib.rs
+++ b/compiler/rustc_hir_pretty/src/lib.rs
@@ -2230,6 +2230,9 @@ impl<'a> State<'a> {
                 GenericBound::Outlives(lt) => {
                     self.print_lifetime(lt);
                 }
+                GenericBound::Unsized(_) => {
+                    self.s.word("?Sized");
+                }
             }
         }
     }
diff --git a/compiler/rustc_middle/src/ty/diagnostics.rs b/compiler/rustc_middle/src/ty/diagnostics.rs
index bfb4c0cb538..4cfb104bee3 100644
--- a/compiler/rustc_middle/src/ty/diagnostics.rs
+++ b/compiler/rustc_middle/src/ty/diagnostics.rs
@@ -2,6 +2,7 @@
 
 use crate::ty::TyKind::*;
 use crate::ty::{InferTy, TyCtxt, TyS};
+use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::{Applicability, DiagnosticBuilder};
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
@@ -105,6 +106,116 @@ pub fn suggest_arbitrary_trait_bound(
     true
 }
 
+fn suggest_removing_unsized_bound(
+    generics: &hir::Generics<'_>,
+    err: &mut DiagnosticBuilder<'_>,
+    param_name: &str,
+    param: &hir::GenericParam<'_>,
+    def_id: Option<DefId>,
+) {
+    // See if there's a `?Sized` bound that can be removed to suggest that.
+    // First look at the `where` clause because we can have `where T: ?Sized`, but that
+    // `?Sized` bound is *also* included in the `GenericParam` as a bound, which breaks
+    // the spans. Hence the somewhat involved logic that follows.
+    let mut where_unsized_bounds = FxHashSet::default();
+    for (where_pos, predicate) in generics.where_clause.predicates.iter().enumerate() {
+        match predicate {
+            WherePredicate::BoundPredicate(WhereBoundPredicate {
+                bounded_ty:
+                    hir::Ty {
+                        kind:
+                            hir::TyKind::Path(hir::QPath::Resolved(
+                                None,
+                                hir::Path {
+                                    segments: [segment],
+                                    res: hir::def::Res::Def(hir::def::DefKind::TyParam, _),
+                                    ..
+                                },
+                            )),
+                        ..
+                    },
+                bounds,
+                span,
+                ..
+            }) if segment.ident.as_str() == param_name => {
+                for (pos, bound) in bounds.iter().enumerate() {
+                    match bound {
+                        hir::GenericBound::Unsized(_) => {}
+                        hir::GenericBound::Trait(poly, hir::TraitBoundModifier::Maybe)
+                            if poly.trait_ref.trait_def_id() == def_id => {}
+                        _ => continue,
+                    }
+                    let sp = match (
+                        bounds.len(),
+                        pos,
+                        generics.where_clause.predicates.len(),
+                        where_pos,
+                    ) {
+                        // where T: ?Sized
+                        // ^^^^^^^^^^^^^^^
+                        (1, _, 1, _) => generics.where_clause.span,
+                        // where Foo: Bar, T: ?Sized,
+                        //               ^^^^^^^^^^^
+                        (1, _, len, pos) if pos == len - 1 => generics.where_clause.predicates
+                            [pos - 1]
+                            .span()
+                            .shrink_to_hi()
+                            .to(*span),
+                        // where T: ?Sized, Foo: Bar,
+                        //       ^^^^^^^^^^^
+                        (1, _, _, pos) => {
+                            span.until(generics.where_clause.predicates[pos + 1].span())
+                        }
+                        // where T: ?Sized + Bar, Foo: Bar,
+                        //          ^^^^^^^^^
+                        (_, 0, _, _) => bound.span().to(bounds[1].span().shrink_to_lo()),
+                        // where T: Bar + ?Sized, Foo: Bar,
+                        //             ^^^^^^^^^
+                        (_, pos, _, _) => bounds[pos - 1].span().shrink_to_hi().to(bound.span()),
+                    };
+                    where_unsized_bounds.insert(bound.span());
+                    err.span_suggestion_verbose(
+                        sp,
+                        "consider removing the `?Sized` bound to make the \
+                            type parameter `Sized`",
+                        String::new(),
+                        Applicability::MaybeIncorrect,
+                    );
+                }
+            }
+            _ => {}
+        }
+    }
+    for (pos, bound) in param.bounds.iter().enumerate() {
+        match bound {
+            hir::GenericBound::Trait(poly, hir::TraitBoundModifier::Maybe)
+                if poly.trait_ref.trait_def_id() == def_id
+                    && !where_unsized_bounds.contains(&bound.span()) =>
+            {
+                let sp = match (param.bounds.len(), pos) {
+                    // T: ?Sized,
+                    //  ^^^^^^^^
+                    (1, _) => param.span.shrink_to_hi().to(bound.span()),
+                    // T: ?Sized + Bar,
+                    //    ^^^^^^^^^
+                    (_, 0) => bound.span().to(param.bounds[1].span().shrink_to_lo()),
+                    // T: Bar + ?Sized,
+                    //       ^^^^^^^^^
+                    (_, pos) => param.bounds[pos - 1].span().shrink_to_hi().to(bound.span()),
+                };
+                err.span_suggestion_verbose(
+                    sp,
+                    "consider removing the `?Sized` bound to make the type parameter \
+                        `Sized`",
+                    String::new(),
+                    Applicability::MaybeIncorrect,
+                );
+            }
+            _ => {}
+        }
+    }
+}
+
 /// Suggest restricting a type param with a new bound.
 pub fn suggest_constraining_type_param(
     tcx: TyCtxt<'_>,
@@ -130,6 +241,7 @@ pub fn suggest_constraining_type_param(
     if def_id == tcx.lang_items().sized_trait() {
         // Type parameters are already `Sized` by default.
         err.span_label(param.span, &format!("this type parameter needs to be `{}`", constraint));
+        suggest_removing_unsized_bound(generics, err, param_name, param, def_id);
         return true;
     }
     let mut suggest_restrict = |span| {
diff --git a/compiler/rustc_save_analysis/src/dump_visitor.rs b/compiler/rustc_save_analysis/src/dump_visitor.rs
index 4f8dc7d16d4..34302c3fb42 100644
--- a/compiler/rustc_save_analysis/src/dump_visitor.rs
+++ b/compiler/rustc_save_analysis/src/dump_visitor.rs
@@ -689,6 +689,7 @@ impl<'tcx> DumpVisitor<'tcx> {
                     (Some(self.tcx.require_lang_item(lang_item, Some(span))), span)
                 }
                 hir::GenericBound::Outlives(..) => continue,
+                hir::GenericBound::Unsized(_) => continue,
             };
 
             if let Some(id) = def_id {
diff --git a/compiler/rustc_typeck/src/astconv/mod.rs b/compiler/rustc_typeck/src/astconv/mod.rs
index 9871b14754e..92583f2b0ea 100644
--- a/compiler/rustc_typeck/src/astconv/mod.rs
+++ b/compiler/rustc_typeck/src/astconv/mod.rs
@@ -943,7 +943,8 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
                         false,
                     );
                 }
-                hir::GenericBound::Trait(_, hir::TraitBoundModifier::Maybe) => {}
+                hir::GenericBound::Trait(_, hir::TraitBoundModifier::Maybe)
+                | hir::GenericBound::Unsized(_) => {}
                 hir::GenericBound::LangItemTrait(lang_item, span, hir_id, args) => self
                     .instantiate_lang_item_trait_ref(
                         lang_item, span, hir_id, args, param_ty, bounds,
diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs
index 997fdcefe03..ce74d6fec9e 100644
--- a/compiler/rustc_typeck/src/collect.rs
+++ b/compiler/rustc_typeck/src/collect.rs
@@ -2230,7 +2230,9 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::GenericP
                             let constness = match modifier {
                                 hir::TraitBoundModifier::MaybeConst => hir::Constness::NotConst,
                                 hir::TraitBoundModifier::None => constness,
-                                hir::TraitBoundModifier::Maybe => bug!("this wasn't handled"),
+                                // We ignore `where T: ?Sized`, it is already part of
+                                // type parameter `T`.
+                                hir::TraitBoundModifier::Maybe => continue,
                             };
 
                             let mut bounds = Bounds::default();
@@ -2260,6 +2262,8 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::GenericP
                             predicates.extend(bounds.predicates(tcx, ty));
                         }
 
+                        hir::GenericBound::Unsized(_) => {}
+
                         hir::GenericBound::Outlives(lifetime) => {
                             let region =
                                 <dyn AstConv<'_>>::ast_region_to_region(&icx, lifetime, None);
@@ -2521,6 +2525,7 @@ fn predicates_from_bound<'tcx>(
             );
             bounds.predicates(astconv.tcx(), param_ty)
         }
+        hir::GenericBound::Unsized(_) => vec![],
         hir::GenericBound::Outlives(ref lifetime) => {
             let region = astconv.ast_region_to_region(lifetime, None);
             let pred = ty::PredicateKind::TypeOutlives(ty::OutlivesPredicate(param_ty, region))