From a7baf4bce491b50ec30c38895d5c1ec296c23ab5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 23 May 2025 15:35:32 +1000 Subject: Simplify the "is some" test in `TypeAliasPart::get`. The comparison against `text` seems to be unnecessary. --- src/librustdoc/html/render/write_shared.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/librustdoc/html/render/write_shared.rs') diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs index b2bbf4614bf..fdeb4bc1f3e 100644 --- a/src/librustdoc/html/render/write_shared.rs +++ b/src/librustdoc/html/render/write_shared.rs @@ -653,7 +653,7 @@ impl TypeAliasPart { ) .to_string(); let type_alias_fqp = (*type_alias_fqp).iter().join("::"); - if Some(&text) == ret.last().map(|s: &AliasSerializableImpl| &s.text) { + if ret.last().map(|s: &AliasSerializableImpl| &s.text).is_some() { ret.last_mut() .expect("already established that ret.last() is Some()") .aliases -- cgit 1.4.1-3-g733a5 From e01f40738fdf1bc7b89c39db347ee6cb8cff9048 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 23 May 2025 15:44:49 +1000 Subject: Move code inside the `else` in `TypeAliasPart::get`. This is a huge perf win for rustdoc on the `typenum` and `nalgebra` benchmarks, because the `else` branch doesn't get hit much. --- src/librustdoc/html/render/write_shared.rs | 62 ++++++++++++++++-------------- 1 file changed, 33 insertions(+), 29 deletions(-) (limited to 'src/librustdoc/html/render/write_shared.rs') diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs index fdeb4bc1f3e..adb220d3547 100644 --- a/src/librustdoc/html/render/write_shared.rs +++ b/src/librustdoc/html/render/write_shared.rs @@ -623,35 +623,6 @@ impl TypeAliasPart { for &(type_alias_fqp, type_alias_item) in type_aliases { cx.id_map.borrow_mut().clear(); cx.deref_id_map.borrow_mut().clear(); - let target_did = impl_ - .inner_impl() - .trait_ - .as_ref() - .map(|trait_| trait_.def_id()) - .or_else(|| impl_.inner_impl().for_.def_id(&cx.shared.cache)); - let provided_methods; - let assoc_link = if let Some(target_did) = target_did { - provided_methods = impl_.inner_impl().provided_trait_methods(cx.tcx()); - AssocItemLink::GotoSource(ItemId::DefId(target_did), &provided_methods) - } else { - AssocItemLink::Anchor(None) - }; - let text = super::render_impl( - cx, - impl_, - type_alias_item, - assoc_link, - RenderMode::Normal, - None, - &[], - ImplRenderingParameters { - show_def_docs: true, - show_default_items: true, - show_non_assoc_items: true, - toggle_open_by_default: true, - }, - ) - .to_string(); let type_alias_fqp = (*type_alias_fqp).iter().join("::"); if ret.last().map(|s: &AliasSerializableImpl| &s.text).is_some() { ret.last_mut() @@ -659,6 +630,39 @@ impl TypeAliasPart { .aliases .push(type_alias_fqp); } else { + let target_did = impl_ + .inner_impl() + .trait_ + .as_ref() + .map(|trait_| trait_.def_id()) + .or_else(|| impl_.inner_impl().for_.def_id(&cx.shared.cache)); + let provided_methods; + let assoc_link = if let Some(target_did) = target_did { + provided_methods = + impl_.inner_impl().provided_trait_methods(cx.tcx()); + AssocItemLink::GotoSource( + ItemId::DefId(target_did), + &provided_methods, + ) + } else { + AssocItemLink::Anchor(None) + }; + let text = super::render_impl( + cx, + impl_, + type_alias_item, + assoc_link, + RenderMode::Normal, + None, + &[], + ImplRenderingParameters { + show_def_docs: true, + show_default_items: true, + show_non_assoc_items: true, + toggle_open_by_default: true, + }, + ) + .to_string(); ret.push(AliasSerializableImpl { text, trait_: trait_.clone(), -- cgit 1.4.1-3-g733a5 From dfe8fe88f03e1d72b170b7cd056026e0a932c5be Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Sat, 24 May 2025 07:48:22 +1000 Subject: Simplify things a little more. --- src/librustdoc/html/render/write_shared.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'src/librustdoc/html/render/write_shared.rs') diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs index adb220d3547..4f6e9abdbca 100644 --- a/src/librustdoc/html/render/write_shared.rs +++ b/src/librustdoc/html/render/write_shared.rs @@ -611,7 +611,7 @@ impl TypeAliasPart { .impl_ .values() .flat_map(|AliasedTypeImpl { impl_, type_aliases }| { - let mut ret = Vec::new(); + let mut ret: Vec = Vec::new(); let trait_ = impl_ .inner_impl() .trait_ @@ -624,11 +624,8 @@ impl TypeAliasPart { cx.id_map.borrow_mut().clear(); cx.deref_id_map.borrow_mut().clear(); let type_alias_fqp = (*type_alias_fqp).iter().join("::"); - if ret.last().map(|s: &AliasSerializableImpl| &s.text).is_some() { - ret.last_mut() - .expect("already established that ret.last() is Some()") - .aliases - .push(type_alias_fqp); + if let Some(last) = ret.last_mut() { + last.aliases.push(type_alias_fqp); } else { let target_did = impl_ .inner_impl() -- cgit 1.4.1-3-g733a5 From 4f1f1a2b57ae9dccca4af131def547bf1106f582 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 23 May 2025 16:22:07 +1000 Subject: Avoid some unnecessary cloning. --- src/librustdoc/clean/types.rs | 2 +- src/librustdoc/html/layout.rs | 1 - src/librustdoc/html/markdown.rs | 2 +- src/librustdoc/html/render/mod.rs | 12 ++++++------ src/librustdoc/html/render/print_item.rs | 4 ++-- src/librustdoc/html/render/write_shared.rs | 2 +- 6 files changed, 11 insertions(+), 12 deletions(-) (limited to 'src/librustdoc/html/render/write_shared.rs') diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index bfc02b6b1d4..d27f0b9d006 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -106,7 +106,7 @@ impl From for ItemId { } /// The crate currently being documented. -#[derive(Clone, Debug)] +#[derive(Debug)] pub(crate) struct Crate { pub(crate) module: Item, /// Only here so that they can be filtered through the rustdoc passes. diff --git a/src/librustdoc/html/layout.rs b/src/librustdoc/html/layout.rs index 3b5f9b5a458..50320cb231d 100644 --- a/src/librustdoc/html/layout.rs +++ b/src/librustdoc/html/layout.rs @@ -8,7 +8,6 @@ use super::static_files::{STATIC_FILES, StaticFiles}; use crate::externalfiles::ExternalHtml; use crate::html::render::{StylePath, ensure_trailing_slash}; -#[derive(Clone)] pub(crate) struct Layout { pub(crate) logo: String, pub(crate) favicon: String, diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index ad7dfafd90c..d6ad3545b69 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -195,7 +195,7 @@ fn slugify(c: char) -> Option { } } -#[derive(Clone, Debug)] +#[derive(Debug)] pub struct Playground { pub crate_name: Option, pub url: String, diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 06cb9269cc8..dd6b58adf52 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -2530,7 +2530,7 @@ fn item_ty_to_section(ty: ItemType) -> ItemSection { /// types are re-exported, we don't use the corresponding /// entry from the js file, as inlining will have already /// picked up the impl -fn collect_paths_for_type(first_ty: clean::Type, cache: &Cache) -> Vec { +fn collect_paths_for_type(first_ty: &clean::Type, cache: &Cache) -> Vec { let mut out = Vec::new(); let mut visited = FxHashSet::default(); let mut work = VecDeque::new(); @@ -2547,7 +2547,7 @@ fn collect_paths_for_type(first_ty: clean::Type, cache: &Cache) -> Vec { work.push_back(first_ty); while let Some(ty) = work.pop_front() { - if !visited.insert(ty.clone()) { + if !visited.insert(ty) { continue; } @@ -2557,16 +2557,16 @@ fn collect_paths_for_type(first_ty: clean::Type, cache: &Cache) -> Vec { work.extend(tys.into_iter()); } clean::Type::Slice(ty) => { - work.push_back(*ty); + work.push_back(ty); } clean::Type::Array(ty, _) => { - work.push_back(*ty); + work.push_back(ty); } clean::Type::RawPointer(_, ty) => { - work.push_back(*ty); + work.push_back(ty); } clean::Type::BorrowedRef { type_, .. } => { - work.push_back(*type_); + work.push_back(type_); } clean::Type::QPath(box clean::QPathData { self_type, trait_, .. }) => { work.push_back(self_type); diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 32f535e8e8c..fa3347c65d3 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -659,7 +659,7 @@ fn item_trait(cx: &Context<'_>, it: &clean::Item, t: &clean::Trait) -> impl fmt: let count_types = required_types.len() + provided_types.len(); let count_consts = required_consts.len() + provided_consts.len(); let count_methods = required_methods.len() + provided_methods.len(); - let must_implement_one_of_functions = tcx.trait_def(t.def_id).must_implement_one_of.clone(); + let must_implement_one_of_functions = &tcx.trait_def(t.def_id).must_implement_one_of; // Output the trait definition wrap_item(w, |mut w| { @@ -1095,7 +1095,7 @@ fn item_trait(cx: &Context<'_>, it: &clean::Item, t: &clean::Trait) -> impl fmt: it, &implementor_dups, &collect_paths_for_type( - implementor.inner_impl().for_.clone(), + &implementor.inner_impl().for_, &cx.shared.cache, ), ) diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs index 4f6e9abdbca..76f52206b99 100644 --- a/src/librustdoc/html/render/write_shared.rs +++ b/src/librustdoc/html/render/write_shared.rs @@ -760,7 +760,7 @@ impl TraitAliasPart { Some(Implementor { text: imp.inner_impl().print(false, cx).to_string(), synthetic: imp.inner_impl().kind.is_auto(), - types: collect_paths_for_type(imp.inner_impl().for_.clone(), cache), + types: collect_paths_for_type(&imp.inner_impl().for_, cache), }) } }) -- cgit 1.4.1-3-g733a5 From 8dde6d53495c5a2dd745553898327b840172444f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 26 May 2025 10:35:24 +1000 Subject: Streamline `TypeAliasPart::get`. - `ret` only ever gets at most one entry, so it can be an `Option` instead of a `Vec`. - Which means we can use `filter_map` instead of `flat_map`. - Move `trait_` next to the `ret` assignment, which can only happen once. - No need for `impls` to be a `Vec`, it can remain an iterator. - Avoid `Result` when collecting `impls`. --- src/librustdoc/html/render/write_shared.rs | 33 +++++++++++++++--------------- 1 file changed, 16 insertions(+), 17 deletions(-) (limited to 'src/librustdoc/html/render/write_shared.rs') diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs index 76f52206b99..c1355750067 100644 --- a/src/librustdoc/html/render/write_shared.rs +++ b/src/librustdoc/html/render/write_shared.rs @@ -607,16 +607,9 @@ impl TypeAliasPart { let cx = type_impl_collector.cx; let aliased_types = type_impl_collector.aliased_types; for aliased_type in aliased_types.values() { - let impls = aliased_type - .impl_ - .values() - .flat_map(|AliasedTypeImpl { impl_, type_aliases }| { - let mut ret: Vec = Vec::new(); - let trait_ = impl_ - .inner_impl() - .trait_ - .as_ref() - .map(|trait_| format!("{:#}", trait_.print(cx))); + let impls = aliased_type.impl_.values().filter_map( + |AliasedTypeImpl { impl_, type_aliases }| { + let mut ret: Option = None; // render_impl will filter out "impossible-to-call" methods // to make that functionality work here, it needs to be called with // each type alias, and if it gives a different result, split the impl @@ -624,8 +617,8 @@ impl TypeAliasPart { cx.id_map.borrow_mut().clear(); cx.deref_id_map.borrow_mut().clear(); let type_alias_fqp = (*type_alias_fqp).iter().join("::"); - if let Some(last) = ret.last_mut() { - last.aliases.push(type_alias_fqp); + if let Some(ret) = &mut ret { + ret.aliases.push(type_alias_fqp); } else { let target_did = impl_ .inner_impl() @@ -660,16 +653,22 @@ impl TypeAliasPart { }, ) .to_string(); - ret.push(AliasSerializableImpl { + // The alternate display disables html escaping of '<' and '>'. + let trait_ = impl_ + .inner_impl() + .trait_ + .as_ref() + .map(|trait_| format!("{:#}", trait_.print(cx))); + ret = Some(AliasSerializableImpl { text, - trait_: trait_.clone(), + trait_, aliases: vec![type_alias_fqp], }) } } ret - }) - .collect::>(); + }, + ); let mut path = PathBuf::from("type.impl"); for component in &aliased_type.target_fqp[..aliased_type.target_fqp.len() - 1] { @@ -682,7 +681,7 @@ impl TypeAliasPart { )); let part = OrderedJson::array_sorted( - impls.iter().map(OrderedJson::serialize).collect::, _>>().unwrap(), + impls.map(|impl_| OrderedJson::serialize(impl_).unwrap()), ); path_parts.push(path, OrderedJson::array_unsorted([crate_name_json, &part])); } -- cgit 1.4.1-3-g733a5 From 68f32169e59dd8bde8fd66cc4ffe98cd24ac09ca Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 30 May 2025 13:26:36 +1000 Subject: Address review comments. --- src/librustdoc/html/format.rs | 10 +++++----- src/librustdoc/html/render/write_shared.rs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'src/librustdoc/html/render/write_shared.rs') diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index c3107e24969..e9a7f4367a3 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -12,7 +12,7 @@ use std::fmt::{self, Display, Write}; use std::iter::{self, once}; use std::slice; -use itertools::Either; +use itertools::{Either, Itertools}; use rustc_abi::ExternAbi; use rustc_attr_data_structures::{ConstStability, StabilityLevel, StableSince}; use rustc_data_structures::fx::FxHashSet; @@ -1118,8 +1118,8 @@ impl clean::Impl { write!(f, "!")?; } if self.kind.is_fake_variadic() - && let Some(mut generics) = ty.generics() - && let (Some(inner_type), None) = (generics.next(), generics.next()) + && let Some(generics) = ty.generics() + && let Ok(inner_type) = generics.exactly_one() { let last = ty.last(); if f.alternate() { @@ -1197,8 +1197,8 @@ impl clean::Impl { fmt_type(&bare_fn.decl.output, f, use_absolute, cx)?; } } else if let clean::Type::Path { path } = type_ - && let Some(mut generics) = path.generics() - && let (Some(ty), None) = (generics.next(), generics.next()) + && let Some(generics) = path.generics() + && let Ok(ty) = generics.exactly_one() && self.kind.is_fake_variadic() { let wrapper = print_anchor(path.def_id(), path.last(), cx); diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs index c1355750067..33738f7a242 100644 --- a/src/librustdoc/html/render/write_shared.rs +++ b/src/librustdoc/html/render/write_shared.rs @@ -653,7 +653,7 @@ impl TypeAliasPart { }, ) .to_string(); - // The alternate display disables html escaping of '<' and '>'. + // The alternate display prints it as plaintext instead of HTML. let trait_ = impl_ .inner_impl() .trait_ -- cgit 1.4.1-3-g733a5