Rollup merge of rust-lang#114059 - fmease:rustdoc-fix-x-crate-impl-sized, r=GuillaumeGomez

rustdoc: fix cross-crate `impl Sized` & `impl ?Sized`

Previously, cross-crate impl-Trait (APIT, RPIT, etc.) that only consists of a single `Sized` bound (modulo outlives-bounds) and ones that are `?Sized` were incorrectly rendered. To give you a taste (before vs. after):

```diff
- fn sized(x: impl ) -> impl
+ fn sized(x: impl Sized) -> impl Sized

- fn sized_outlives<'a>(x: impl 'a) -> impl 'a
+ fn sized_outlives<'a>(x: impl Sized + 'a) -> impl Sized + 'a

- fn maybe_sized(x: &impl ) -> &impl
+ fn maybe_sized(x: &impl ?Sized) -> &impl ?Sized

- fn debug_maybe_sized(x: &impl Debug) -> &impl ?Sized + Debug
+ fn debug_maybe_sized(x: &(impl Debug + ?Sized)) -> &(impl Debug + ?Sized)
```

Moreover, we now surround impl-Trait that has multiple bounds with parentheses if they're the pointee of a reference or raw pointer type. This affects both local and cross-crate docs. The current output isn't correct (rustc would emit the error *ambiguous `+` in a type* if we fed the rendered code back to it).

---

Best reviewed commit by commit :)

`@rustbot` label A-cross-crate-reexports

Author: GuillaumeGomez

src/librustdoc/clean/mod.rs

@@ -804,10 +804,10 @@ fn clean_ty_generics<'tcx>(
     let where_predicates = preds
         .predicates
         .iter()
-        .flat_map(|(p, _)| {
+        .flat_map(|(pred, _)| {
             let mut projection = None;
             let param_idx = (|| {
-                let bound_p = p.kind();
+                let bound_p = pred.kind();
                 match bound_p.skip_binder() {
                     ty::ClauseKind::Trait(pred) => {
                         if let ty::Param(param) = pred.self_ty().kind() {
@@ -832,33 +832,26 @@ fn clean_ty_generics<'tcx>(
             })();
 
             if let Some(param_idx) = param_idx
-                && let Some(b) = impl_trait.get_mut(&param_idx.into())
+                && let Some(bounds) = impl_trait.get_mut(&param_idx.into())
             {
-                let p: WherePredicate = clean_predicate(*p, cx)?;
+                let pred = clean_predicate(*pred, cx)?;
 
-                b.extend(
-                    p.get_bounds()
+                bounds.extend(
+                    pred.get_bounds()
                         .into_iter()
                         .flatten()
                         .cloned()
-                        .filter(|b| !b.is_sized_bound(cx)),
                 );
 
-                let proj = projection.map(|p| {
-                    (
-                        clean_projection(p.map_bound(|p| p.projection_ty), cx, None),
-                        p.map_bound(|p| p.term),
-                    )
-                });
-                if let Some(((_, trait_did, name), rhs)) = proj
-                    .as_ref()
-                    .and_then(|(lhs, rhs): &(Type, _)| Some((lhs.projection()?, rhs)))
+                if let Some(proj) = projection
+                    && let lhs = clean_projection(proj.map_bound(|p| p.projection_ty), cx, None)
+                    && let Some((_, trait_did, name)) = lhs.projection()
                 {
                     impl_trait_proj.entry(param_idx).or_default().push((
                         trait_did,
                         name,
-                        *rhs,
-                        p.get_bound_params()
+                        proj.map_bound(|p| p.term),
+                        pred.get_bound_params()
                             .into_iter()
                             .flatten()
                             .cloned()
@@ -869,13 +862,32 @@ fn clean_ty_generics<'tcx>(
                 return None;
             }
 
-            Some(p)
+            Some(pred)
         })
         .collect::<Vec<_>>();
 
     for (param, mut bounds) in impl_trait {
+        let mut has_sized = false;
+        bounds.retain(|b| {
+            if b.is_sized_bound(cx) {
+                has_sized = true;
+                false
+            } else {
+                true
+            }
+        });
+        if !has_sized {
+            bounds.push(GenericBound::maybe_sized(cx));
+        }
+
         // Move trait bounds to the front.
-        bounds.sort_by_key(|b| !matches!(b, GenericBound::TraitBound(..)));
+        bounds.sort_by_key(|b| !b.is_trait_bound());
+
+        // Add back a `Sized` bound if there are no *trait* bounds remaining (incl. `?Sized`).
+        // Since all potential trait bounds are at the front we can just check the first bound.
+        if bounds.first().map_or(true, |b| !b.is_trait_bound()) {
+            bounds.insert(0, GenericBound::sized(cx));
+        }
 
         let crate::core::ImplTraitParam::ParamIndex(idx) = param else { unreachable!() };
         if let Some(proj) = impl_trait_proj.remove(&idx) {
@@ -897,7 +909,7 @@ fn clean_ty_generics<'tcx>(
     // implicit `Sized` bound unless removed with `?Sized`.
     // However, in the list of where-predicates below, `Sized` appears like a
     // normal bound: It's either present (the type is sized) or
-    // absent (the type is unsized) but never *maybe* (i.e. `?Sized`).
+    // absent (the type might be unsized) but never *maybe* (i.e. `?Sized`).
     //
     // This is unsuitable for rendering.
     // Thus, as a first step remove all `Sized` bounds that should be implicit.
@@ -908,8 +920,8 @@ fn clean_ty_generics<'tcx>(
     let mut sized_params = FxHashSet::default();
     where_predicates.retain(|pred| {
         if let WherePredicate::BoundPredicate { ty: Generic(g), bounds, .. } = pred
-        && *g != kw::SelfUpper
-        && bounds.iter().any(|b| b.is_sized_bound(cx))
+            && *g != kw::SelfUpper
+            && bounds.iter().any(|b| b.is_sized_bound(cx))
         {
             sized_params.insert(*g);
             false
@@ -2119,7 +2131,6 @@ fn clean_middle_opaque_bounds<'tcx>(
     cx: &mut DocContext<'tcx>,
     bounds: Vec<ty::Clause<'tcx>>,
 ) -> Type {
-    let mut regions = vec![];
     let mut has_sized = false;
     let mut bounds = bounds
         .iter()
@@ -2128,10 +2139,7 @@ fn clean_middle_opaque_bounds<'tcx>(
             let trait_ref = match bound_predicate.skip_binder() {
                 ty::ClauseKind::Trait(tr) => bound_predicate.rebind(tr.trait_ref),
                 ty::ClauseKind::TypeOutlives(ty::OutlivesPredicate(_ty, reg)) => {
-                    if let Some(r) = clean_middle_region(reg) {
-                        regions.push(GenericBound::Outlives(r));
-                    }
-                    return None;
+                    return clean_middle_region(reg).map(GenericBound::Outlives);
                 }
                 _ => return None,
             };
@@ -2167,10 +2175,20 @@ fn clean_middle_opaque_bounds<'tcx>(
             Some(clean_poly_trait_ref_with_bindings(cx, trait_ref, bindings))
         })
         .collect::<Vec<_>>();
-    bounds.extend(regions);
-    if !has_sized && !bounds.is_empty() {
-        bounds.insert(0, GenericBound::maybe_sized(cx));
+
+    if !has_sized {
+        bounds.push(GenericBound::maybe_sized(cx));
     }
+
+    // Move trait bounds to the front.
+    bounds.sort_by_key(|b| !b.is_trait_bound());
+
+    // Add back a `Sized` bound if there are no *trait* bounds remaining (incl. `?Sized`).
+    // Since all potential trait bounds are at the front we can just check the first bound.
+    if bounds.first().map_or(true, |b| !b.is_trait_bound()) {
+        bounds.insert(0, GenericBound::sized(cx));
+    }
+
     ImplTrait(bounds)
 }
 

src/librustdoc/clean/types.rs

@@ -1219,15 +1219,24 @@ pub(crate) enum GenericBound {
 }
 
 impl GenericBound {
+    pub(crate) fn sized(cx: &mut DocContext<'_>) -> GenericBound {
+        Self::sized_with(cx, hir::TraitBoundModifier::None)
+    }
+
     pub(crate) fn maybe_sized(cx: &mut DocContext<'_>) -> GenericBound {
+        Self::sized_with(cx, hir::TraitBoundModifier::Maybe)
+    }
+
+    fn sized_with(cx: &mut DocContext<'_>, modifier: hir::TraitBoundModifier) -> GenericBound {
         let did = cx.tcx.require_lang_item(LangItem::Sized, None);
         let empty = ty::Binder::dummy(ty::GenericArgs::empty());
         let path = external_path(cx, did, false, ThinVec::new(), empty);
         inline::record_extern_fqn(cx, did, ItemType::Trait);
-        GenericBound::TraitBound(
-            PolyTrait { trait_: path, generic_params: Vec::new() },
-            hir::TraitBoundModifier::Maybe,
-        )
+        GenericBound::TraitBound(PolyTrait { trait_: path, generic_params: Vec::new() }, modifier)
+    }
+
+    pub(crate) fn is_trait_bound(&self) -> bool {
+        matches!(self, Self::TraitBound(..))
     }
 
     pub(crate) fn is_sized_bound(&self, cx: &DocContext<'_>) -> bool {

src/librustdoc/html/format.rs

@@ -1102,22 +1102,35 @@ fn fmt_type<'cx>(
             };
             let m = mutability.print_with_space();
             let amp = if f.alternate() { "&" } else { "&amp;" };
-            match **ty {
+
+            if let clean::Generic(name) = **ty {
+                return primitive_link(
+                    f,
+                    PrimitiveType::Reference,
+                    &format!("{amp}{lt}{m}{name}"),
+                    cx,
+                );
+            }
+
+            write!(f, "{amp}{lt}{m}")?;
+
+            let needs_parens = match **ty {
                 clean::DynTrait(ref bounds, ref trait_lt)
                     if bounds.len() > 1 || trait_lt.is_some() =>
                 {
-                    write!(f, "{}{}{}(", amp, lt, m)?;
-                    fmt_type(ty, f, use_absolute, cx)?;
-                    write!(f, ")")
-                }
-                clean::Generic(name) => {
-                    primitive_link(f, PrimitiveType::Reference, &format!("{amp}{lt}{m}{name}"), cx)
-                }
-                _ => {
-                    write!(f, "{}{}{}", amp, lt, m)?;
-                    fmt_type(ty, f, use_absolute, cx)
+                    true
                 }
+                clean::ImplTrait(ref bounds) if bounds.len() > 1 => true,
+                _ => false,
+            };
+            if needs_parens {
+                f.write_str("(")?;
             }
+            fmt_type(ty, f, use_absolute, cx)?;
+            if needs_parens {
+                f.write_str(")")?;
+            }
+            Ok(())
         }
         clean::ImplTrait(ref bounds) => {
             if f.alternate() {

tests/rustdoc/extern-impl-trait.rs

@@ -7,5 +7,5 @@ extern crate extern_impl_trait;
 // @has 'foo/struct.X.html' '//h4[@class="code-header"]' "impl Foo<Associated = ()> + 'a"
 pub use extern_impl_trait::X;
 
-// @has 'foo/struct.Y.html' '//h4[@class="code-header"]' "impl ?Sized + Foo<Associated = ()> + 'a"
+// @has 'foo/struct.Y.html' '//h4[@class="code-header"]' "impl Foo<Associated = ()> + ?Sized + 'a"
 pub use extern_impl_trait::Y;

tests/rustdoc/impl-everywhere.rs

@@ -25,6 +25,6 @@ pub fn foo_foo() -> impl Foo + Foo2 {
     Bar
 }
 
-// @has foo/fn.foo_foo_foo.html '//section[@id="main-content"]//pre' "x: &'x impl Foo + Foo2"
+// @has foo/fn.foo_foo_foo.html '//section[@id="main-content"]//pre' "x: &'x (impl Foo + Foo2)"
 pub fn foo_foo_foo<'x>(_x: &'x (impl Foo + Foo2)) {
 }

tests/rustdoc/inline_cross/auxiliary/impl-sized.rs

@@ -0,0 +1,21 @@
+use std::fmt::Debug;
+
+pub fn sized(x: impl Sized) -> impl Sized {
+    x
+}
+
+pub fn sized_outlives<'a>(x: impl Sized + 'a) -> impl Sized + 'a {
+    x
+}
+
+pub fn maybe_sized(x: &impl ?Sized) -> &impl ?Sized {
+    x
+}
+
+pub fn debug_maybe_sized(x: &(impl Debug + ?Sized)) -> &(impl Debug + ?Sized) {
+    x
+}
+
+pub fn maybe_sized_outlives<'t>(x: &(impl ?Sized + 't)) -> &(impl ?Sized + 't) {
+    x
+}

tests/rustdoc/inline_cross/impl-sized.rs

@@ -0,0 +1,27 @@
+#![crate_name = "user"]
+
+// aux-crate:impl_sized=impl-sized.rs
+// edition:2021
+
+// @has user/fn.sized.html
+// @has - '//pre[@class="rust item-decl"]' "sized(x: impl Sized) -> impl Sized"
+pub use impl_sized::sized;
+
+// @has user/fn.sized_outlives.html
+// @has - '//pre[@class="rust item-decl"]' \
+//     "sized_outlives<'a>(x: impl Sized + 'a) -> impl Sized + 'a"
+pub use impl_sized::sized_outlives;
+
+// @has user/fn.maybe_sized.html
+// @has - '//pre[@class="rust item-decl"]' "maybe_sized(x: &impl ?Sized) -> &impl ?Sized"
+pub use impl_sized::maybe_sized;
+
+// @has user/fn.debug_maybe_sized.html
+// @has - '//pre[@class="rust item-decl"]' \
+//     "debug_maybe_sized(x: &(impl Debug + ?Sized)) -> &(impl Debug + ?Sized)"
+pub use impl_sized::debug_maybe_sized;
+
+// @has user/fn.maybe_sized_outlives.html
+// @has - '//pre[@class="rust item-decl"]' \
+//     "maybe_sized_outlives<'t>(x: &(impl ?Sized + 't)) -> &(impl ?Sized + 't)"
+pub use impl_sized::maybe_sized_outlives;