Rollup merge of #93412 - fee1-dead:improve-rustdoc-const-bounds, r=GuillaumeGomez

matthiaskrgr · web-flow · commit faf1a7ffc879 · 2022-03-06T15:41:26.000+01:00
Improve rustdoc const bounds - Rustdoc no longer displays `~const` in trait bounds, because it currently means nothing for stable users, and because we still haven't decided on the final syntax yet. - Rustdoc will hide trait bounds where the trait is `Drop` AND it is `~const`, i.e. `~const Drop` bounds because it has no effect on stable users as well. - Because of additional logic that hides the whole `where` statement where it consists of `~const Drop` bounds (so it doesn't display `struct Foo<T>() where ;` like that), bounds that have no trait e.g. `where [T; N+1]: ;` are also hidden. Cherry-picked from #92433.
diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs
@@ -143,11 +143,14 @@ impl Buffer {
     }
 }
 
-fn comma_sep<T: fmt::Display>(items: impl Iterator<Item = T>) -> impl fmt::Display {
+fn comma_sep<T: fmt::Display>(
+    items: impl Iterator<Item = T>,
+    space_after_comma: bool,
+) -> impl fmt::Display {
     display_fn(move |f| {
         for (i, item) in items.enumerate() {
             if i != 0 {
-                write!(f, ", ")?;
+                write!(f, ",{}", if space_after_comma { " " } else { "" })?;
             }
             fmt::Display::fmt(&item, f)?;
         }
@@ -248,9 +251,9 @@ impl clean::Generics {
             }
 
             if f.alternate() {
-                write!(f, "<{:#}>", comma_sep(real_params.map(|g| g.print(cx))))
+                write!(f, "<{:#}>", comma_sep(real_params.map(|g| g.print(cx)), true))
             } else {
-                write!(f, "&lt;{}&gt;", comma_sep(real_params.map(|g| g.print(cx))))
+                write!(f, "&lt;{}&gt;", comma_sep(real_params.map(|g| g.print(cx)), true))
             }
         })
     }
@@ -266,10 +269,80 @@ crate fn print_where_clause<'a, 'tcx: 'a>(
     end_newline: bool,
 ) -> impl fmt::Display + 'a + Captures<'tcx> {
     display_fn(move |f| {
-        if gens.where_predicates.is_empty() {
+        let mut where_predicates = gens.where_predicates.iter().filter(|pred| {
+            !matches!(pred, clean::WherePredicate::BoundPredicate { bounds, .. } if bounds.is_empty())
+        }).map(|pred| {
+            display_fn(move |f| {
+                if f.alternate() {
+                    f.write_str(" ")?;
+                } else {
+                    f.write_str("<br>")?;
+                }
+
+                match pred {
+                    clean::WherePredicate::BoundPredicate { ty, bounds, bound_params } => {
+                        let bounds = bounds;
+                        let for_prefix = if bound_params.is_empty() {
+                            String::new()
+                        } else if f.alternate() {
+                            format!(
+                                "for&lt;{:#}&gt; ",
+                                comma_sep(bound_params.iter().map(|lt| lt.print()), true)
+                            )
+                        } else {
+                            format!(
+                                "for&lt;{}&gt; ",
+                                comma_sep(bound_params.iter().map(|lt| lt.print()), true)
+                            )
+                        };
+
+                        if f.alternate() {
+                            write!(
+                                f,
+                                "{}{:#}: {:#}",
+                                for_prefix,
+                                ty.print(cx),
+                                print_generic_bounds(bounds, cx)
+                            )
+                        } else {
+                            write!(
+                                f,
+                                "{}{}: {}",
+                                for_prefix,
+                                ty.print(cx),
+                                print_generic_bounds(bounds, cx)
+                            )
+                        }
+                    }
+                    clean::WherePredicate::RegionPredicate { lifetime, bounds } => {
+                        write!(
+                            f,
+                            "{}: {}",
+                            lifetime.print(),
+                            bounds
+                                .iter()
+                                .map(|b| b.print(cx).to_string())
+                                .collect::<Vec<_>>()
+                                .join(" + ")
+                        )
+                    }
+                    clean::WherePredicate::EqPredicate { lhs, rhs } => {
+                        if f.alternate() {
+                            write!(f, "{:#} == {:#}", lhs.print(cx), rhs.print(cx),)
+                        } else {
+                            write!(f, "{} == {}", lhs.print(cx), rhs.print(cx),)
+                        }
+                    }
+                }
+            })
+        }).peekable();
+
+        if where_predicates.peek().is_none() {
             return Ok(());
         }
+
         let mut clause = String::new();
+
         if f.alternate() {
             clause.push_str(" where");
         } else {
@@ -279,72 +352,11 @@ crate fn print_where_clause<'a, 'tcx: 'a>(
                 clause.push_str(" <span class=\"where\">where");
             }
         }
-        for (i, pred) in gens.where_predicates.iter().enumerate() {
-            if f.alternate() {
-                clause.push(' ');
-            } else {
-                clause.push_str("<br>");
-            }
-
-            match pred {
-                clean::WherePredicate::BoundPredicate { ty, bounds, bound_params } => {
-                    let bounds = bounds;
-                    let for_prefix = match bound_params.len() {
-                        0 => String::new(),
-                        _ if f.alternate() => {
-                            format!(
-                                "for&lt;{:#}&gt; ",
-                                comma_sep(bound_params.iter().map(|lt| lt.print()))
-                            )
-                        }
-                        _ => format!(
-                            "for&lt;{}&gt; ",
-                            comma_sep(bound_params.iter().map(|lt| lt.print()))
-                        ),
-                    };
-
-                    if f.alternate() {
-                        clause.push_str(&format!(
-                            "{}{:#}: {:#}",
-                            for_prefix,
-                            ty.print(cx),
-                            print_generic_bounds(bounds, cx)
-                        ));
-                    } else {
-                        clause.push_str(&format!(
-                            "{}{}: {}",
-                            for_prefix,
-                            ty.print(cx),
-                            print_generic_bounds(bounds, cx)
-                        ));
-                    }
-                }
-                clean::WherePredicate::RegionPredicate { lifetime, bounds } => {
-                    clause.push_str(&format!(
-                        "{}: {}",
-                        lifetime.print(),
-                        bounds
-                            .iter()
-                            .map(|b| b.print(cx).to_string())
-                            .collect::<Vec<_>>()
-                            .join(" + ")
-                    ));
-                }
-                clean::WherePredicate::EqPredicate { lhs, rhs } => {
-                    if f.alternate() {
-                        clause.push_str(&format!("{:#} == {:#}", lhs.print(cx), rhs.print(cx),));
-                    } else {
-                        clause.push_str(&format!("{} == {}", lhs.print(cx), rhs.print(cx),));
-                    }
-                }
-            }
 
-            if i < gens.where_predicates.len() - 1 || end_newline {
-                clause.push(',');
-            }
-        }
+        clause.push_str(&comma_sep(where_predicates, false).to_string());
 
         if end_newline {
+            clause.push(',');
             // add a space so stripping <br> tags and breaking spaces still renders properly
             if f.alternate() {
                 clause.push(' ');
@@ -394,13 +406,13 @@ impl clean::PolyTrait {
                     write!(
                         f,
                         "for<{:#}> ",
-                        comma_sep(self.generic_params.iter().map(|g| g.print(cx)))
+                        comma_sep(self.generic_params.iter().map(|g| g.print(cx)), true)
                     )?;
                 } else {
                     write!(
                         f,
                         "for&lt;{}&gt; ",
-                        comma_sep(self.generic_params.iter().map(|g| g.print(cx)))
+                        comma_sep(self.generic_params.iter().map(|g| g.print(cx)), true)
                     )?;
                 }
             }
@@ -424,7 +436,8 @@ impl clean::GenericBound {
                 let modifier_str = match modifier {
                     hir::TraitBoundModifier::None => "",
                     hir::TraitBoundModifier::Maybe => "?",
-                    hir::TraitBoundModifier::MaybeConst => "~const",
+                    // ~const is experimental; do not display those bounds in rustdoc
+                    hir::TraitBoundModifier::MaybeConst => "",
                 };
                 if f.alternate() {
                     write!(f, "{}{:#}", modifier_str, ty.print(cx))
@@ -1111,7 +1124,7 @@ impl clean::BareFunctionDecl {
                 write!(
                     f,
                     "for&lt;{}&gt; ",
-                    comma_sep(self.generic_params.iter().map(|g| g.print(cx)))
+                    comma_sep(self.generic_params.iter().map(|g| g.print(cx)), true)
                 )
             } else {
                 Ok(())
diff --git a/src/test/rustdoc/const-generics/generic_const_exprs.rs b/src/test/rustdoc/const-generics/generic_const_exprs.rs
@@ -3,5 +3,5 @@
 #![allow(incomplete_features)]
 // make sure that `ConstEvaluatable` predicates dont cause rustdoc to ICE #77647
 // @has foo/struct.Ice.html '//pre[@class="rust struct"]' \
-//      'pub struct Ice<const N: usize> where [(); N + 1]: ;'
+//      'pub struct Ice<const N: usize>;'
 pub struct Ice<const N: usize> where [(); N + 1]:;
diff --git a/src/test/rustdoc/rfc-2632-const-trait-impl.rs b/src/test/rustdoc/rfc-2632-const-trait-impl.rs
@@ -0,0 +1,60 @@
+// Test that we do not currently display `~const` in rustdoc
+// as that syntax is currently provisional; `~const Drop` has
+// no effect on stable code so it should be hidden as well.
+//
+// To future blessers: make sure that `const_trait_impl` is
+// stabilized when changing `@!has` to `@has`, and please do
+// not remove this test.
+#![feature(const_trait_impl)]
+#![crate_name = "foo"]
+
+pub struct S<T>(T);
+
+// @!has foo/trait.Tr.html '//pre[@class="rust trait"]/code/a[@class="trait"]' '~const'
+// @!has - '//pre[@class="rust trait"]/code/a[@class="trait"]' 'Drop'
+// @has - '//pre[@class="rust trait"]/code/a[@class="trait"]' 'Clone'
+// @!has - '//pre[@class="rust trait"]/code/span[@class="where"]' '~const'
+// @!has - '//pre[@class="rust trait"]/code/span[@class="where"]' 'Drop'
+// @has - '//pre[@class="rust trait"]/code/span[@class="where"]' ': Clone'
+pub trait Tr<T> {
+    // @!has - '//div[@id="method.a"]/h4[@class="code-header"]' '~const'
+    // @!has - '//div[@id="method.a"]/h4[@class="code-header"]/a[@class="trait"]' 'Drop'
+    // @has - '//div[@id="method.a"]/h4[@class="code-header"]/a[@class="trait"]' 'Clone'
+    // @!has - '//div[@id="method.a"]/h4[@class="code-header"]/span[@class="where"]' '~const'
+    // @!has - '//div[@id="method.a"]/h4[@class="code-header"]/span[@class="where fmt-newline"]' 'Drop'
+    // @has - '//div[@id="method.a"]/h4[@class="code-header"]/span[@class="where fmt-newline"]' ': Clone'
+    #[default_method_body_is_const]
+    fn a<A: ~const Drop + ~const Clone>() where Option<A>: ~const Drop + ~const Clone {}
+}
+
+// @!has - '//section[@id="impl-Tr%3CT%3E"]/h3[@class="code-header in-band"]' '~const'
+// @!has - '//section[@id="impl-Tr%3CT%3E"]/h3[@class="code-header in-band"]/a[@class="trait"]' 'Drop'
+// @has - '//section[@id="impl-Tr%3CT%3E"]/h3[@class="code-header in-band"]/a[@class="trait"]' 'Clone'
+// @!has - '//section[@id="impl-Tr%3CT%3E"]/h3[@class="code-header in-band"]/span[@class="where"]' '~const'
+// @!has - '//section[@id="impl-Tr%3CT%3E"]/h3[@class="code-header in-band"]/span[@class="where fmt-newline"]' 'Drop'
+// @has - '//section[@id="impl-Tr%3CT%3E"]/h3[@class="code-header in-band"]/span[@class="where fmt-newline"]' ': Clone'
+impl<T: ~const Drop + ~const Clone> const Tr<T> for T where Option<T>: ~const Drop + ~const Clone {
+    fn a<A: ~const Drop + ~const Clone>() where Option<A>: ~const Drop + ~const Clone {}
+}
+
+// @!has foo/fn.foo.html '//pre[@class="rust fn"]/code/a[@class="trait"]' '~const'
+// @!has - '//pre[@class="rust fn"]/code/a[@class="trait"]' 'Drop'
+// @has - '//pre[@class="rust fn"]/code/a[@class="trait"]' 'Clone'
+// @!has - '//pre[@class="rust fn"]/code/span[@class="where fmt-newline"]' '~const'
+// @!has - '//pre[@class="rust fn"]/code/span[@class="where fmt-newline"]' 'Drop'
+// @has - '//pre[@class="rust fn"]/code/span[@class="where fmt-newline"]' ': Clone'
+pub const fn foo<F: ~const Drop + ~const Clone>() where Option<F>: ~const Drop + ~const Clone {
+    F::a()
+}
+
+impl<T> S<T> {
+    // @!has foo/struct.S.html '//section[@id="method.foo"]/h4[@class="code-header"]' '~const'
+    // @!has - '//section[@id="method.foo"]/h4[@class="code-header"]/a[@class="trait"]' 'Drop'
+    // @has - '//section[@id="method.foo"]/h4[@class="code-header"]/a[@class="trait"]' 'Clone'
+    // @!has - '//section[@id="method.foo"]/h4[@class="code-header"]/span[@class="where"]' '~const'
+    // @!has - '//section[@id="method.foo"]/h4[@class="code-header"]/span[@class="where fmt-newline"]' 'Drop'
+    // @has - '//section[@id="method.foo"]/h4[@class="code-header"]/span[@class="where fmt-newline"]' ': Clone'
+    pub const fn foo<B: ~const Drop + ~const Clone>() where B: ~const Drop + ~const Clone {
+        B::a()
+    }
+}
