Auto merge of #143559 - obi1kenobi:pg/allow_unsized, r=<try>

Show whether `?Sized` parameters are actually `Sized`

A mostly-working, "some polish still required" attempt at fixing #143197

- Add a new `allow_unsized: bool` field to `GenericParamDefKind::Type`.
- Expose that field as-is to rustdoc JSON, without tampering with `?Sized` clauses there.
- Suppress `?Sized` from HTML where `Sized` is implied.

I haven't figured out a good way to suppress `?Sized` from `impl Trait` in function parameters in HTML yet. The synthetic generics data doesn't seem to be "nearby" so more refactoring might be needed. I included a failing test case to remind me of this.

r? fmease

Author: rust-bors[bot]

src/librustdoc/clean/mod.rs

@@ -538,12 +538,15 @@ fn clean_generic_param_def(
             } else {
                 None
             };
+            let param_ty = ty::ParamTy::for_def(def);
+            let allow_unsized = !param_ty.to_ty(cx.tcx).is_sized(cx.tcx, cx.typing_env());
             (
                 def.name,
                 GenericParamDefKind::Type {
                     bounds: ThinVec::new(), // These are filled in from the where-clauses.
                     default: default.map(Box::new),
                     synthetic,
+                    allow_unsized,
                 },
             )
         }
@@ -617,12 +620,35 @@ fn clean_generic_param<'tcx>(
             } else {
                 ThinVec::new()
             };
+
+            // If this ends up being slow, then optimize it by reading the local bounds
+            // (from all predicate origins) and check if a bound on `?Sized` is present.
+            // If there's no `?Sized` bound, then definitely `allow_unsized = false`.
+            let allow_unsized = {
+                let parent = cx.tcx.hir_ty_param_owner(param.def_id);
+                let index = cx
+                    .tcx
+                    .generics_of(parent)
+                    .param_def_id_to_index(cx.tcx, param.def_id.to_def_id());
+
+                if let Some(index) = index {
+                    let param_ty = ty::ParamTy::new(index, param.name.ident().name);
+                    !param_ty.to_ty(cx.tcx).is_sized(cx.tcx, cx.typing_env())
+                } else {
+                    // AFAIU this should never happen:
+                    // it would mean the generic parameter wasn't found.
+                    // Advice on how to handle this would be appreciated :)
+                    unreachable!()
+                }
+            };
+
             (
                 param.name.ident().name,
                 GenericParamDefKind::Type {
                     bounds,
                     default: default.map(|t| clean_ty(t, cx)).map(Box::new),
                     synthetic,
+                    allow_unsized,
                 },
             )
         }
@@ -3201,6 +3227,7 @@ fn clean_bound_vars<'tcx>(
                         bounds: ThinVec::new(),
                         default: None,
                         synthetic: false,
+                        allow_unsized: false, // If `for<T>` could support `T: ?Sized`, fix this.
                     },
                 })
             }

src/librustdoc/clean/types.rs

@@ -6,6 +6,7 @@ use std::{fmt, iter};
 use arrayvec::ArrayVec;
 use itertools::Either;
 use rustc_abi::{ExternAbi, VariantIdx};
+use rustc_ast::BoundPolarity;
 use rustc_attr_data_structures::{
     AttributeKind, ConstStability, Deprecation, Stability, StableSince, find_attr,
 };
@@ -1295,6 +1296,16 @@ impl GenericBound {
         matches!(self, Self::TraitBound(..))
     }
 
+    pub(crate) fn is_maybe_sized_bound(&self, tcx: TyCtxt<'_>) -> bool {
+        if let GenericBound::TraitBound(PolyTrait { ref trait_, .. }, modifiers) = *self
+            && matches!(modifiers.polarity, BoundPolarity::Maybe(_))
+            && tcx.is_lang_item(trait_.def_id(), LangItem::Sized)
+        {
+            return true;
+        }
+        false
+    }
+
     pub(crate) fn is_sized_bound(&self, cx: &DocContext<'_>) -> bool {
         self.is_bounded_by_lang_item(cx, LangItem::Sized)
     }
@@ -1371,10 +1382,21 @@ impl WherePredicate {
 
 #[derive(Clone, PartialEq, Eq, Debug, Hash)]
 pub(crate) enum GenericParamDefKind {
-    Lifetime { outlives: ThinVec<Lifetime> },
-    Type { bounds: ThinVec<GenericBound>, default: Option<Box<Type>>, synthetic: bool },
+    Lifetime {
+        outlives: ThinVec<Lifetime>,
+    },
+    Type {
+        bounds: ThinVec<GenericBound>,
+        default: Option<Box<Type>>,
+        synthetic: bool,
+        allow_unsized: bool,
+    },
     // Option<Box<String>> makes this type smaller than `Option<String>` would.
-    Const { ty: Box<Type>, default: Option<Box<String>>, synthetic: bool },
+    Const {
+        ty: Box<Type>,
+        default: Option<Box<String>>,
+        synthetic: bool,
+    },
 }
 
 impl GenericParamDefKind {
@@ -1406,6 +1428,13 @@ impl GenericParamDef {
         self.kind.is_type()
     }
 
+    pub(crate) fn allow_unsized(&self) -> bool {
+        match &self.kind {
+            GenericParamDefKind::Type { allow_unsized, .. } => *allow_unsized,
+            _ => false,
+        }
+    }
+
     pub(crate) fn get_bounds(&self) -> Option<&[GenericBound]> {
         match self.kind {
             GenericParamDefKind::Type { ref bounds, .. } => Some(bounds),

src/librustdoc/html/format.rs

@@ -68,9 +68,21 @@ impl clean::GenericParamDef {
 
                 Ok(())
             }
-            clean::GenericParamDefKind::Type { bounds, default, .. } => {
+            clean::GenericParamDefKind::Type { bounds, default, allow_unsized, .. } => {
                 f.write_str(self.name.as_str())?;
 
+                let filtered: Vec<_>;
+                let bounds = if !allow_unsized {
+                    filtered = bounds
+                        .iter()
+                        .filter(|b| !b.is_maybe_sized_bound(cx.tcx()))
+                        .cloned()
+                        .collect();
+                    filtered.as_slice()
+                } else {
+                    bounds.as_slice()
+                };
+
                 if !bounds.is_empty() {
                     f.write_str(": ")?;
                     print_generic_bounds(bounds, cx).fmt(f)?;
@@ -127,14 +139,45 @@ pub(crate) enum Ending {
     NoNewline,
 }
 
-fn print_where_predicate(predicate: &clean::WherePredicate, cx: &Context<'_>) -> impl Display {
+fn print_where_predicate(
+    predicate: &clean::WherePredicate,
+    disallow_unsized: &FxHashSet<Symbol>,
+    cx: &Context<'_>,
+) -> impl Display {
     fmt::from_fn(move |f| {
         match predicate {
             clean::WherePredicate::BoundPredicate { ty, bounds, bound_params } => {
-                print_higher_ranked_params_with_space(bound_params, cx, "for").fmt(f)?;
-                ty.print(cx).fmt(f)?;
-                f.write_str(":")?;
+                let filtered: Vec<_>;
+                let mut bounds_slice = bounds.as_slice();
+                // Check if the bound is on a generic type parameter that
+                // does not accept unsized types.
+                if let clean::Type::Generic(symbol) = ty
+                    && disallow_unsized.contains(symbol)
+                {
+                    // Check if the predicate contains a `?Sized` bound on that generic type.
+                    // Even though the bound is syntactically present, we know that
+                    // another bound requires this generic to be `Sized`.
+                    // We omit the `?Sized` bound from the representation.
+                    // See: https://github.com/rust-lang/rust/issues/143197
+                    //
+                    // We do two passes to avoid allocating a new `Vec` and copying it
+                    // in the most common case: when `?Sized` is *not* present.
+                    if bounds.iter().any(|b| b.is_maybe_sized_bound(cx.tcx())) {
+                        filtered = bounds
+                            .iter()
+                            .filter(|b| !b.is_maybe_sized_bound(cx.tcx()))
+                            .cloned()
+                            .collect();
+                        bounds_slice = filtered.as_slice();
+                    }
+                };
+                let bounds = bounds_slice;
+
                 if !bounds.is_empty() {
+                    print_higher_ranked_params_with_space(bound_params, cx, "for").fmt(f)?;
+                    ty.print(cx).fmt(f)?;
+                    f.write_str(":")?;
+
                     f.write_str(" ")?;
                     print_generic_bounds(bounds, cx).fmt(f)?;
                 }
@@ -172,6 +215,8 @@ pub(crate) fn print_where_clause(
     if gens.where_predicates.is_empty() {
         return None;
     }
+    let disallow_unsized: FxHashSet<_> =
+        gens.params.iter().filter(|p| p.is_type() && !p.allow_unsized()).map(|p| p.name).collect();
 
     Some(fmt::from_fn(move |f| {
         let where_preds = fmt::from_fn(|f| {
@@ -184,7 +229,7 @@ pub(crate) fn print_where_clause(
                         } else {
                             f.write_str("\n")?;
                         }
-                        print_where_predicate(predicate, cx).fmt(f)
+                        print_where_predicate(predicate, &disallow_unsized, cx).fmt(f)
                     })
                 })
                 .joined(",", f)
@@ -1017,6 +1062,8 @@ fn fmt_type(
         }
         clean::ImplTrait(bounds) => {
             f.write_str("impl ")?;
+            // TODO: Figure out if one of the bounds here is `?Sized`
+            //       and whether another bound implies `Sized`.
             print_generic_bounds(bounds, cx).fmt(f)
         }
         clean::QPath(qpath) => qpath.print(cx).fmt(f),

src/librustdoc/json/conversions.rs

@@ -440,10 +440,11 @@ impl FromClean<clean::GenericParamDefKind> for GenericParamDefKind {
             Lifetime { outlives } => {
                 GenericParamDefKind::Lifetime { outlives: outlives.into_json(renderer) }
             }
-            Type { bounds, default, synthetic } => GenericParamDefKind::Type {
+            Type { bounds, default, synthetic, allow_unsized } => GenericParamDefKind::Type {
                 bounds: bounds.into_json(renderer),
                 default: default.into_json(renderer),
                 is_synthetic: *synthetic,
+                allow_unsized: *allow_unsized,
             },
             Const { ty, default, synthetic: _ } => GenericParamDefKind::Const {
                 type_: ty.into_json(renderer),

src/rustdoc-json-types/lib.rs

@@ -37,8 +37,8 @@ pub type FxHashMap<K, V> = HashMap<K, V>; // re-export for use in src/librustdoc
 // will instead cause conflicts. See #94591 for more. (This paragraph and the "Latest feature" line
 // are deliberately not in a doc comment, because they need not be in public docs.)
 //
-// Latest feature: Pretty printing of no_mangle attributes changed
-pub const FORMAT_VERSION: u32 = 53;
+// Latest feature: add `allow_unsized` field to `GenericParamDefKind::Type`
+pub const FORMAT_VERSION: u32 = 54;
 
 /// The root of the emitted JSON blob.
 ///
@@ -893,6 +893,11 @@ pub enum GenericParamDefKind {
         /// is bound by `Trait`) is synthetic, because it was not originally in
         /// the Rust source text.
         is_synthetic: bool,
+        /// Whether this type parameter can be instantiated with an unsized type.
+        ///
+        /// This is `true` if the parameter has a `?Sized` bound without any
+        /// additional bounds that imply `Sized`.
+        allow_unsized: bool,
     },
 
     /// Denotes a constant parameter.

src/tools/jsondoclint/src/validator.rs

@@ -332,7 +332,12 @@ impl<'a> Validator<'a> {
     fn check_generic_param_def(&mut self, gpd: &'a GenericParamDef) {
         match &gpd.kind {
             rustdoc_json_types::GenericParamDefKind::Lifetime { outlives: _ } => {}
-            rustdoc_json_types::GenericParamDefKind::Type { bounds, default, is_synthetic: _ } => {
+            rustdoc_json_types::GenericParamDefKind::Type {
+                bounds,
+                default,
+                is_synthetic: _,
+                allow_unsized: _,
+            } => {
                 bounds.iter().for_each(|b| self.check_generic_bound(b));
                 if let Some(ty) = default {
                     self.check_type(ty);