Auto merge of #136244 - yotamofek:pr/rustdoc-join-iter, r=GuillaumeGomez

librustdoc: create a helper for separating elements of an iterator instead of implementing it multiple times

This implements something similar to [`Itertools::format`](https://docs.rs/itertools/latest/itertools/trait.Itertools.html#method.format), but on `Fn`s returning iterators instead of directly on iterators, to allow implementing `Display` without the use of a `Cell` (to handle the possibility of `fmt` being called multiple times while receiving `&self`).

~This is WIP, I just want to get a perf run first to see if the regression I saw in #135494 is fixed~

This was originally part of #135494 , but originally caused a perf regression that was since fixed:
7d5ae1863a/src/librustdoc/html/format.rs (L507)
This commit is contained in:
bors 2025-02-05 02:51:17 +00:00
commit 8df89d1cb0
5 changed files with 203 additions and 206 deletions

View File

@ -14,6 +14,7 @@ use rustc_span::Span;
use rustc_span::symbol::{Symbol, sym};
use crate::html::escape::Escape;
use crate::joined::Joined as _;
#[cfg(test)]
mod tests;
@ -396,6 +397,8 @@ impl Display<'_> {
sub_cfgs: &[Cfg],
separator: &str,
) -> fmt::Result {
use fmt::Display as _;
let short_longhand = self.1.is_long() && {
let all_crate_features =
sub_cfgs.iter().all(|sub_cfg| matches!(sub_cfg, Cfg::Cfg(sym::feature, Some(_))));
@ -414,20 +417,29 @@ impl Display<'_> {
}
};
for (i, sub_cfg) in sub_cfgs.iter().enumerate() {
if i != 0 {
fmt.write_str(separator)?;
}
if let (true, Cfg::Cfg(_, Some(feat))) = (short_longhand, sub_cfg) {
if self.1.is_html() {
write!(fmt, "<code>{feat}</code>")?;
} else {
write!(fmt, "`{feat}`")?;
}
} else {
write_with_opt_paren(fmt, !sub_cfg.is_all(), Display(sub_cfg, self.1))?;
}
}
fmt::from_fn(|f| {
sub_cfgs
.iter()
.map(|sub_cfg| {
fmt::from_fn(move |fmt| {
if let Cfg::Cfg(_, Some(feat)) = sub_cfg
&& short_longhand
{
if self.1.is_html() {
write!(fmt, "<code>{feat}</code>")?;
} else {
write!(fmt, "`{feat}`")?;
}
} else {
write_with_opt_paren(fmt, !sub_cfg.is_all(), Display(sub_cfg, self.1))?;
}
Ok(())
})
})
.joined(separator, f)
})
.fmt(fmt)?;
Ok(())
}
}
@ -439,11 +451,20 @@ impl fmt::Display for Display<'_> {
Cfg::Any(ref sub_cfgs) => {
let separator =
if sub_cfgs.iter().all(Cfg::is_simple) { " nor " } else { ", nor " };
for (i, sub_cfg) in sub_cfgs.iter().enumerate() {
fmt.write_str(if i == 0 { "neither " } else { separator })?;
write_with_opt_paren(fmt, !sub_cfg.is_all(), Display(sub_cfg, self.1))?;
}
Ok(())
fmt.write_str("neither ")?;
sub_cfgs
.iter()
.map(|sub_cfg| {
fmt::from_fn(|fmt| {
write_with_opt_paren(
fmt,
!sub_cfg.is_all(),
Display(sub_cfg, self.1),
)
})
})
.joined(separator, fmt)
}
ref simple @ Cfg::Cfg(..) => write!(fmt, "non-{}", Display(simple, self.1)),
ref c => write!(fmt, "not ({})", Display(c, self.1)),

View File

@ -8,12 +8,11 @@
//! not be used external to this module.
use std::borrow::Cow;
use std::cell::Cell;
use std::cmp::Ordering;
use std::fmt::{self, Display, Write};
use std::iter::{self, once};
use itertools::Itertools;
use itertools::Either;
use rustc_abi::ExternAbi;
use rustc_attr_parsing::{ConstStability, StabilityLevel, StableSince};
use rustc_data_structures::captures::Captures;
@ -35,6 +34,7 @@ use crate::formats::cache::Cache;
use crate::formats::item_type::ItemType;
use crate::html::escape::{Escape, EscapeBodyText};
use crate::html::render::Context;
use crate::joined::Joined as _;
use crate::passes::collect_intra_doc_links::UrlFragment;
pub(crate) trait Print {
@ -146,22 +146,6 @@ impl Buffer {
}
}
pub(crate) fn comma_sep<T: Display>(
items: impl Iterator<Item = T>,
space_after_comma: bool,
) -> impl Display {
let items = Cell::new(Some(items));
fmt::from_fn(move |f| {
for (i, item) in items.take().unwrap().enumerate() {
if i != 0 {
write!(f, ",{}", if space_after_comma { " " } else { "" })?;
}
item.fmt(f)?;
}
Ok(())
})
}
pub(crate) fn print_generic_bounds<'a, 'tcx: 'a>(
bounds: &'a [clean::GenericBound],
cx: &'a Context<'tcx>,
@ -169,13 +153,11 @@ pub(crate) fn print_generic_bounds<'a, 'tcx: 'a>(
fmt::from_fn(move |f| {
let mut bounds_dup = FxHashSet::default();
for (i, bound) in bounds.iter().filter(|b| bounds_dup.insert(*b)).enumerate() {
if i > 0 {
f.write_str(" + ")?;
}
bound.print(cx).fmt(f)?;
}
Ok(())
bounds
.iter()
.filter(move |b| bounds_dup.insert(*b))
.map(|bound| bound.print(cx))
.joined(" + ", f)
})
}
@ -190,12 +172,7 @@ impl clean::GenericParamDef {
if !outlives.is_empty() {
f.write_str(": ")?;
for (i, lt) in outlives.iter().enumerate() {
if i != 0 {
f.write_str(" + ")?;
}
write!(f, "{}", lt.print())?;
}
outlives.iter().map(|lt| lt.print()).joined(" + ", f)?;
}
Ok(())
@ -245,10 +222,12 @@ impl clean::Generics {
return Ok(());
}
let real_params =
fmt::from_fn(|f| real_params.clone().map(|g| g.print(cx)).joined(", ", f));
if f.alternate() {
write!(f, "<{:#}>", comma_sep(real_params.map(|g| g.print(cx)), true))
write!(f, "<{:#}>", real_params)
} else {
write!(f, "&lt;{}&gt;", comma_sep(real_params.map(|g| g.print(cx)), true))
write!(f, "&lt;{}&gt;", real_params)
}
})
}
@ -260,6 +239,42 @@ pub(crate) enum Ending {
NoNewline,
}
fn print_where_predicate<'a, 'tcx: 'a>(
predicate: &'a clean::WherePredicate,
cx: &'a Context<'tcx>,
) -> impl Display + 'a + Captures<'tcx> {
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(":")?;
if !bounds.is_empty() {
f.write_str(" ")?;
print_generic_bounds(bounds, cx).fmt(f)?;
}
Ok(())
}
clean::WherePredicate::RegionPredicate { lifetime, bounds } => {
// We don't need to check `alternate` since we can be certain that neither
// the lifetime nor the bounds contain any characters which need escaping.
write!(f, "{}:", lifetime.print())?;
if !bounds.is_empty() {
write!(f, " {}", print_generic_bounds(bounds, cx))?;
}
Ok(())
}
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))
}
}
}
})
}
/// * The Generics from which to emit a where-clause.
/// * The number of spaces to indent each line with.
/// * Whether the where-clause needs to add a comma and newline after the last bound.
@ -270,55 +285,26 @@ pub(crate) fn print_where_clause<'a, 'tcx: 'a>(
ending: Ending,
) -> impl Display + 'a + Captures<'tcx> {
fmt::from_fn(move |f| {
let mut where_predicates = gens
.where_predicates
.iter()
.map(|pred| {
fmt::from_fn(move |f| {
if f.alternate() {
f.write_str(" ")?;
} else {
f.write_str("\n")?;
}
match pred {
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(":")?;
if !bounds.is_empty() {
f.write_str(" ")?;
print_generic_bounds(bounds, cx).fmt(f)?;
}
Ok(())
}
clean::WherePredicate::RegionPredicate { lifetime, bounds } => {
// We don't need to check `alternate` since we can be certain that neither
// the lifetime nor the bounds contain any characters which need escaping.
write!(f, "{}:", lifetime.print())?;
if !bounds.is_empty() {
write!(f, " {}", print_generic_bounds(bounds, cx))?;
}
Ok(())
}
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() {
if gens.where_predicates.is_empty() {
return Ok(());
}
let where_preds = comma_sep(where_predicates, false);
let where_preds = fmt::from_fn(|f| {
gens.where_predicates
.iter()
.map(|predicate| {
fmt::from_fn(|f| {
if f.alternate() {
f.write_str(" ")?;
} else {
f.write_str("\n")?;
}
print_where_predicate(predicate, cx).fmt(f)
})
})
.joined(",", f)
});
let clause = if f.alternate() {
if ending == Ending::Newline {
format!(" where{where_preds},")
@ -415,12 +401,7 @@ impl clean::GenericBound {
} else {
f.write_str("use&lt;")?;
}
for (i, arg) in args.iter().enumerate() {
if i > 0 {
write!(f, ", ")?;
}
arg.fmt(f)?;
}
args.iter().joined(", ", f)?;
if f.alternate() { f.write_str(">") } else { f.write_str("&gt;") }
}
})
@ -438,29 +419,18 @@ impl clean::GenericArgs {
} else {
f.write_str("&lt;")?;
}
let mut comma = false;
for arg in args.iter() {
if comma {
f.write_str(", ")?;
}
comma = true;
if f.alternate() {
write!(f, "{:#}", arg.print(cx))?;
} else {
write!(f, "{}", arg.print(cx))?;
}
}
for constraint in constraints.iter() {
if comma {
f.write_str(", ")?;
}
comma = true;
if f.alternate() {
write!(f, "{:#}", constraint.print(cx))?;
} else {
write!(f, "{}", constraint.print(cx))?;
}
}
[Either::Left(args), Either::Right(constraints)]
.into_iter()
.flat_map(Either::factor_into_iter)
.map(|either| {
either.map_either(
|arg| arg.print(cx),
|constraint| constraint.print(cx),
)
})
.joined(", ", f)?;
if f.alternate() {
f.write_str(">")?;
} else {
@ -470,14 +440,7 @@ impl clean::GenericArgs {
}
clean::GenericArgs::Parenthesized { inputs, output } => {
f.write_str("(")?;
let mut comma = false;
for ty in inputs.iter() {
if comma {
f.write_str(", ")?;
}
comma = true;
ty.print(cx).fmt(f)?;
}
inputs.iter().map(|ty| ty.print(cx)).joined(", ", f)?;
f.write_str(")")?;
if let Some(ref ty) = *output {
if f.alternate() {
@ -524,6 +487,7 @@ pub(crate) enum HrefError {
// Panics if `syms` is empty.
pub(crate) fn join_with_double_colon(syms: &[Symbol]) -> String {
let mut s = String::with_capacity(estimate_item_path_byte_length(syms.len()));
// NOTE: using `Joined::joined` here causes a noticeable perf regression
s.push_str(syms[0].as_str());
for sym in &syms[1..] {
s.push_str("::");
@ -572,20 +536,20 @@ fn generate_macro_def_id_path(
}
if let Some(last) = path.last_mut() {
*last = Symbol::intern(&format!("macro.{}.html", last.as_str()));
*last = Symbol::intern(&format!("macro.{last}.html"));
}
let url = match cache.extern_locations[&def_id.krate] {
ExternalLocation::Remote(ref s) => {
// `ExternalLocation::Remote` always end with a `/`.
format!("{s}{path}", path = path.iter().map(|p| p.as_str()).join("/"))
format!("{s}{path}", path = fmt::from_fn(|f| path.iter().joined("/", f)))
}
ExternalLocation::Local => {
// `root_path` always end with a `/`.
format!(
"{root_path}{path}",
root_path = root_path.unwrap_or(""),
path = path.iter().map(|p| p.as_str()).join("/")
path = fmt::from_fn(|f| path.iter().joined("/", f))
)
}
ExternalLocation::Unknown => {
@ -682,9 +646,8 @@ fn make_href(
url_parts.push("index.html");
}
_ => {
let prefix = shortty.as_str();
let last = fqp.last().unwrap();
url_parts.push_fmt(format_args!("{prefix}.{last}.html"));
url_parts.push_fmt(format_args!("{shortty}.{last}.html"));
}
}
Ok((url_parts.finish(), shortty, fqp.to_vec()))
@ -950,12 +913,7 @@ fn tybounds<'a, 'tcx: 'a>(
cx: &'a Context<'tcx>,
) -> impl Display + 'a + Captures<'tcx> {
fmt::from_fn(move |f| {
for (i, bound) in bounds.iter().enumerate() {
if i > 0 {
write!(f, " + ")?;
}
bound.print(cx).fmt(f)?;
}
bounds.iter().map(|bound| bound.print(cx)).joined(" + ", f)?;
if let Some(lt) = lt {
// We don't need to check `alternate` since we can be certain that
// the lifetime doesn't contain any characters which need escaping.
@ -974,7 +932,7 @@ fn print_higher_ranked_params_with_space<'a, 'tcx: 'a>(
if !params.is_empty() {
f.write_str(keyword)?;
f.write_str(if f.alternate() { "<" } else { "&lt;" })?;
comma_sep(params.iter().map(|lt| lt.print(cx)), true).fmt(f)?;
params.iter().map(|lt| lt.print(cx)).joined(", ", f)?;
f.write_str(if f.alternate() { "> " } else { "&gt; " })?;
}
Ok(())
@ -1025,9 +983,7 @@ fn fmt_type(
clean::Primitive(clean::PrimitiveType::Never) => {
primitive_link(f, PrimitiveType::Never, format_args!("!"), cx)
}
clean::Primitive(prim) => {
primitive_link(f, prim, format_args!("{}", prim.as_sym().as_str()), cx)
}
clean::Primitive(prim) => primitive_link(f, prim, format_args!("{}", prim.as_sym()), cx),
clean::BareFunction(ref decl) => {
print_higher_ranked_params_with_space(&decl.generic_params, cx, "for").fmt(f)?;
decl.safety.print_with_space().fmt(f)?;
@ -1067,18 +1023,16 @@ fn fmt_type(
primitive_link(
f,
PrimitiveType::Tuple,
format_args!("({})", generic_names.iter().map(|s| s.as_str()).join(", ")),
format_args!(
"({})",
fmt::from_fn(|f| generic_names.iter().joined(", ", f))
),
cx,
)
} else {
write!(f, "(")?;
for (i, item) in many.iter().enumerate() {
if i != 0 {
write!(f, ", ")?;
}
item.print(cx).fmt(f)?;
}
write!(f, ")")
f.write_str("(")?;
many.iter().map(|item| item.print(cx)).joined(", ", f)?;
f.write_str(")")
}
}
},
@ -1407,16 +1361,17 @@ impl clean::Arguments {
cx: &'a Context<'tcx>,
) -> impl Display + 'a + Captures<'tcx> {
fmt::from_fn(move |f| {
for (i, input) in self.values.iter().enumerate() {
if !input.name.is_empty() {
write!(f, "{}: ", input.name)?;
}
input.type_.print(cx).fmt(f)?;
if i + 1 < self.values.len() {
write!(f, ", ")?;
}
}
Ok(())
self.values
.iter()
.map(|input| {
fmt::from_fn(|f| {
if !input.name.is_empty() {
write!(f, "{}: ", input.name)?;
}
input.type_.print(cx).fmt(f)
})
})
.joined(", ", f)
})
}
}
@ -1725,12 +1680,7 @@ impl clean::ImportSource {
}
let name = self.path.last();
if let hir::def::Res::PrimTy(p) = self.path.res {
primitive_link(
f,
PrimitiveType::from(p),
format_args!("{}", name.as_str()),
cx,
)?;
primitive_link(f, PrimitiveType::from(p), format_args!("{name}"), cx)?;
} else {
f.write_str(name.as_str())?;
}

View File

@ -1,5 +1,6 @@
use std::cmp::Ordering;
use std::fmt;
use std::fmt::{Display, Write};
use itertools::Itertools;
use rinja::Template;
@ -36,6 +37,7 @@ use crate::html::format::{
use crate::html::markdown::{HeadingOffset, MarkdownSummaryLine};
use crate::html::render::{document_full, document_item_info};
use crate::html::url_parts_builder::UrlPartsBuilder;
use crate::joined::Joined as _;
/// Generates a Rinja template struct for rendering items with common methods.
///
@ -290,7 +292,7 @@ fn should_hide_fields(n_fields: usize) -> bool {
n_fields > 12
}
fn toggle_open(mut w: impl fmt::Write, text: impl fmt::Display) {
fn toggle_open(mut w: impl fmt::Write, text: impl Display) {
write!(
w,
"<details class=\"toggle type-contents-toggle\">\
@ -305,7 +307,7 @@ fn toggle_close(mut w: impl fmt::Write) {
w.write_str("</details>").unwrap();
}
trait ItemTemplate<'a, 'cx: 'a>: rinja::Template + fmt::Display {
trait ItemTemplate<'a, 'cx: 'a>: rinja::Template + Display {
fn item_and_cx(&self) -> (&'a clean::Item, &'a Context<'cx>);
}
@ -519,13 +521,9 @@ fn extra_info_tags<'a, 'tcx: 'a>(
item: &'a clean::Item,
parent: &'a clean::Item,
import_def_id: Option<DefId>,
) -> impl fmt::Display + 'a + Captures<'tcx> {
) -> impl Display + 'a + Captures<'tcx> {
fmt::from_fn(move |f| {
fn tag_html<'a>(
class: &'a str,
title: &'a str,
contents: &'a str,
) -> impl fmt::Display + 'a {
fn tag_html<'a>(class: &'a str, title: &'a str, contents: &'a str) -> impl Display + 'a {
fmt::from_fn(move |f| {
write!(
f,
@ -1374,7 +1372,7 @@ fn item_union(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::Uni
);
impl<'a, 'cx: 'a> ItemUnion<'a, 'cx> {
fn render_union<'b>(&'b self) -> impl fmt::Display + Captures<'a> + 'b + Captures<'cx> {
fn render_union<'b>(&'b self) -> impl Display + Captures<'a> + 'b + Captures<'cx> {
fmt::from_fn(move |f| {
let v = render_union(self.it, Some(&self.s.generics), &self.s.fields, self.cx);
write!(f, "{v}")
@ -1384,7 +1382,7 @@ fn item_union(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::Uni
fn document_field<'b>(
&'b self,
field: &'a clean::Item,
) -> impl fmt::Display + Captures<'a> + 'b + Captures<'cx> {
) -> impl Display + Captures<'a> + 'b + Captures<'cx> {
fmt::from_fn(move |f| {
let v = document(self.cx, field, Some(self.it), HeadingOffset::H3);
write!(f, "{v}")
@ -1398,7 +1396,7 @@ fn item_union(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::Uni
fn print_ty<'b>(
&'b self,
ty: &'a clean::Type,
) -> impl fmt::Display + Captures<'a> + 'b + Captures<'cx> {
) -> impl Display + Captures<'a> + 'b + Captures<'cx> {
fmt::from_fn(move |f| {
let v = ty.print(self.cx);
write!(f, "{v}")
@ -1425,7 +1423,7 @@ fn item_union(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::Uni
fn print_tuple_struct_fields<'a, 'cx: 'a>(
cx: &'a Context<'cx>,
s: &'a [clean::Item],
) -> impl fmt::Display + 'a + Captures<'cx> {
) -> impl Display + 'a + Captures<'cx> {
fmt::from_fn(|f| {
if !s.is_empty()
&& s.iter().all(|field| {
@ -1435,17 +1433,15 @@ fn print_tuple_struct_fields<'a, 'cx: 'a>(
return f.write_str("<span class=\"comment\">/* private fields */</span>");
}
for (i, ty) in s.iter().enumerate() {
if i > 0 {
f.write_str(", ")?;
}
match ty.kind {
clean::StrippedItem(box clean::StructFieldItem(_)) => f.write_str("_")?,
clean::StructFieldItem(ref ty) => write!(f, "{}", ty.print(cx))?,
_ => unreachable!(),
}
}
Ok(())
s.iter()
.map(|ty| {
fmt::from_fn(|f| match ty.kind {
clean::StrippedItem(box clean::StructFieldItem(_)) => f.write_str("_"),
clean::StructFieldItem(ref ty) => write!(f, "{}", ty.print(cx)),
_ => unreachable!(),
})
})
.joined(", ", f)
})
}
@ -2068,12 +2064,12 @@ fn bounds(t_bounds: &[clean::GenericBound], trait_alias: bool, cx: &Context<'_>)
bounds.push_str(": ");
}
}
for (i, p) in t_bounds.iter().enumerate() {
if i > 0 {
bounds.push_str(inter_str);
}
bounds.push_str(&p.print(cx).to_string());
}
write!(
bounds,
"{}",
fmt::from_fn(|f| t_bounds.iter().map(|p| p.print(cx)).joined(inter_str, f))
)
.unwrap();
bounds
}
@ -2150,7 +2146,7 @@ fn render_union<'a, 'cx: 'a>(
g: Option<&'a clean::Generics>,
fields: &'a [clean::Item],
cx: &'a Context<'cx>,
) -> impl fmt::Display + 'a + Captures<'cx> {
) -> impl Display + 'a + Captures<'cx> {
fmt::from_fn(move |mut f| {
write!(f, "{}union {}", visibility_print_with_space(it, cx), it.name.unwrap(),)?;
@ -2330,7 +2326,7 @@ fn document_non_exhaustive_header(item: &clean::Item) -> &str {
if item.is_non_exhaustive() { " (Non-exhaustive)" } else { "" }
}
fn document_non_exhaustive(item: &clean::Item) -> impl fmt::Display + '_ {
fn document_non_exhaustive(item: &clean::Item) -> impl Display + '_ {
fmt::from_fn(|f| {
if item.is_non_exhaustive() {
write!(

29
src/librustdoc/joined.rs Normal file
View File

@ -0,0 +1,29 @@
use std::fmt::{self, Display, Formatter};
pub(crate) trait Joined: IntoIterator {
/// Takes an iterator over elements that implement [`Display`], and format them into `f`, separated by `sep`.
///
/// This is similar to [`Itertools::format`](itertools::Itertools::format), but instead of returning an implementation of `Display`,
/// it formats directly into a [`Formatter`].
///
/// The performance of `joined` is slightly better than `format`, since it doesn't need to use a `Cell` to keep track of whether [`fmt`](Display::fmt)
/// was already called (`joined`'s API doesn't allow it be called more than once).
fn joined(self, sep: &str, f: &mut Formatter<'_>) -> fmt::Result;
}
impl<I, T> Joined for I
where
I: IntoIterator<Item = T>,
T: Display,
{
fn joined(self, sep: &str, f: &mut Formatter<'_>) -> fmt::Result {
let mut iter = self.into_iter();
let Some(first) = iter.next() else { return Ok(()) };
first.fmt(f)?;
while let Some(item) = iter.next() {
f.write_str(sep)?;
item.fmt(f)?;
}
Ok(())
}
}

View File

@ -113,6 +113,7 @@ mod fold;
mod formats;
// used by the error-index generator, so it needs to be public
pub mod html;
mod joined;
mod json;
pub(crate) mod lint;
mod markdown;