Do not discard ?Sized type params and suggest their removal

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,
                     })
                 })

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))
             }
         }

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,
         }
     }
 }

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(_) => {}
     }
 }
 

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");
+                }
             }
         }
     }

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| {

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 {

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,

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))

src/librustdoc/clean/mod.rs

@@ -128,6 +128,7 @@ impl Clean<GenericBound> for hir::GenericBound<'_> {
     fn clean(&self, cx: &mut DocContext<'_>) -> GenericBound {
         match *self {
             hir::GenericBound::Outlives(lt) => GenericBound::Outlives(lt.clean(cx)),
+            hir::GenericBound::Unsized(_) => GenericBound::maybe_sized(cx),
             hir::GenericBound::LangItemTrait(lang_item, span, _, generic_args) => {
                 let def_id = cx.tcx.require_lang_item(lang_item, Some(span));
 
@@ -562,13 +563,19 @@ impl Clean<Generics> for hir::Generics<'_> {
                 WherePredicate::BoundPredicate {
                     ty: Generic(ref name), ref mut bounds, ..
                 } => {
-                    if bounds.is_empty() {
+                    if let [] | [GenericBound::TraitBound(_, hir::TraitBoundModifier::Maybe)] =
+                        &bounds[..]
+                    {
                         for param in &mut generics.params {
                             match param.kind {
                                 GenericParamDefKind::Lifetime => {}
                                 GenericParamDefKind::Type { bounds: ref mut ty_bounds, .. } => {
                                     if &param.name == name {
                                         mem::swap(bounds, ty_bounds);
+                                        // We now keep track of `?Sized` obligations in the HIR.
+                                        // If we don't clear `ty_bounds` we end up with
+                                        // `fn foo<X: ?Sized>(_: X) where X: ?Sized`.
+                                        ty_bounds.clear();
                                         break;
                                     }
                                 }

src/test/ui/const-generics/const-argument-if-length.full.stderr

@@ -10,6 +10,11 @@ LL |     if std::mem::size_of::<T>() == 0 {
    |
 LL | pub const fn size_of<T>() -> usize {
    |                      - required by this bound in `std::mem::size_of`
+   |
+help: consider removing the `?Sized` bound to make the type parameter `Sized`
+   |
+LL | pub const fn is_zst<T>() -> usize {
+   |                     --
 
 error[E0277]: the size for values of type `T` cannot be known at compilation time
   --> $DIR/const-argument-if-length.rs:16:12
@@ -21,6 +26,10 @@ LL |     value: T,
    |
    = note: only the last field of a struct may have a dynamically sized type
    = help: change the field's type to have a statically known size
+help: consider removing the `?Sized` bound to make the type parameter `Sized`
+   |
+LL | pub struct AtLeastByte<T> {
+   |                        --
 help: borrowed types always have a statically known size
    |
 LL |     value: &T,

src/test/ui/const-generics/const-argument-if-length.min.stderr

@@ -17,6 +17,10 @@ LL |     value: T,
    |
    = note: only the last field of a struct may have a dynamically sized type
    = help: change the field's type to have a statically known size
+help: consider removing the `?Sized` bound to make the type parameter `Sized`
+   |
+LL | pub struct AtLeastByte<T> {
+   |                        --
 help: borrowed types always have a statically known size
    |
 LL |     value: &T,

src/test/ui/dst/dst-object-from-unsized-type.stderr

@@ -7,6 +7,10 @@ LL |     let u: &dyn Foo = t;
    |                       ^ doesn't have a size known at compile-time
    |
    = note: required for the cast to the object type `dyn Foo`
+help: consider removing the `?Sized` bound to make the type parameter `Sized`
+   |
+LL | fn test1<T: Foo>(t: &T) {
+   |            --
 
 error[E0277]: the size for values of type `T` cannot be known at compilation time
   --> $DIR/dst-object-from-unsized-type.rs:13:23
@@ -17,6 +21,10 @@ LL |     let v: &dyn Foo = t as &dyn Foo;
    |                       ^ doesn't have a size known at compile-time
    |
    = note: required for the cast to the object type `dyn Foo`
+help: consider removing the `?Sized` bound to make the type parameter `Sized`
+   |
+LL | fn test2<T: Foo>(t: &T) {
+   |            --
 
 error[E0277]: the size for values of type `str` cannot be known at compilation time
   --> $DIR/dst-object-from-unsized-type.rs:18:28

src/test/ui/packed/issue-27060-2.stderr

@@ -8,6 +8,10 @@ LL |     data: T,
    |
    = note: the last field of a packed struct may only have a dynamically sized type if it does not need drop to be run
    = help: change the field's type to have a statically known size
+help: consider removing the `?Sized` bound to make the type parameter `Sized`
+   |
+LL | pub struct Bad<T> {
+   |                --
 help: borrowed types always have a statically known size
    |
 LL |     data: &T,

src/test/ui/suggestions/adt-param-with-implicit-sized-bound.stderr

@@ -91,6 +91,10 @@ LL | struct X<T>(T);
    |          ^  - ...if indirection were used here: `Box<T>`
    |          |
    |          this could be changed to `T: ?Sized`...
+help: consider removing the `?Sized` bound to make the type parameter `Sized`
+   |
+LL | struct Struct5<T>{
+   |                --
 
 error: aborting due to 5 previous errors