From 0053d7602a87baf5437c013ab61ddc02c411faa6 Mon Sep 17 00:00:00 2001 From: Yotam Ofek Date: Sat, 6 Jun 2026 20:40:09 +0300 Subject: [PATCH] Cleanup and optimize `render_impls` - take ownership of the `Vec<&Impl>` instead of copying into another alloc - reuse `ImplString` to do natural sort ordering - lazy formatting --- src/librustdoc/html/render/mod.rs | 113 ++++++++---------- src/librustdoc/html/render/print_item.rs | 23 ++-- .../macro/const-rendering-macros-33302.rs | 20 ++-- 3 files changed, 77 insertions(+), 79 deletions(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index fd6d389542b99..4de2a46007877 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -40,6 +40,7 @@ mod type_layout; mod write_shared; use std::borrow::Cow; +use std::cmp::Ordering; use std::collections::VecDeque; use std::fmt::{self, Display as _, Write}; use std::iter::Peekable; @@ -79,7 +80,7 @@ use crate::html::format::{ use crate::html::markdown::{ HeadingOffset, IdMap, Markdown, MarkdownItemInfo, MarkdownSummaryLine, short_markdown_summary, }; -use crate::html::render::print_item::compare_names; +use crate::html::render::print_item::ImplString; use crate::html::render::search_index::get_function_type_for_search; use crate::html::static_files::SCRAPE_EXAMPLES_HELP_MD; use crate::html::{highlight, sources}; @@ -954,46 +955,50 @@ fn impl_trait_key(cx: &Context<'_>, i: &Impl) -> Option { // Render the list of items inside one of the sections "Trait Implementations", // "Auto Trait Implementations," "Blanket Trait Implementations" (on struct/enum pages). -fn render_impls( - cx: &Context<'_>, - mut w: impl Write, - impls: &[&Impl], - containing_item: &clean::Item, +fn render_impls<'a, 'cx>( + cx: &'a Context<'cx>, + mut impls: Vec<&'a Impl>, + containing_item: &'a clean::Item, toggle_open_by_default: bool, -) -> fmt::Result { +) -> impl fmt::Display + use<'a, 'cx> { + impls.sort_by_cached_key(|imp| { + let prefix = match imp.inner_impl().polarity { + ty::ImplPolarity::Positive | ty::ImplPolarity::Reservation => Ordering::Greater, + ty::ImplPolarity::Negative => Ordering::Less, + }; + (prefix, ImplString::new_path(imp, cx)) + }); // Render each impl alongside its `impl_trait_key`, which is used as the primary sorting key // to match the impl order in the sidebar. - let mut keyed_rendered_impls = impls - .iter() - .map(|i| { - let did = i.trait_did().unwrap(); - let provided_trait_methods = i.inner_impl().provided_trait_methods(cx.tcx()); - let assoc_link = AssocItemLink::GotoSource(did.into(), &provided_trait_methods); - let imp = render_impl( - cx, - i, - containing_item, - assoc_link, - RenderMode::Normal, - None, - &[], - ImplRenderingParameters { - show_def_docs: true, - show_default_items: true, - show_non_assoc_items: true, - toggle_open_by_default, - }, - ); - (impl_trait_key(cx, i).unwrap(), imp.to_string()) - }) - .collect::>(); - // Sort and then remove the `impl_trait_key`s, which are no longer needed after sorting. - keyed_rendered_impls - .sort_by(|(k1, h1), (k2, h2)| compare_names(k1, k2).then_with(|| h1.cmp(h2))); - let joined: String = keyed_rendered_impls.into_iter().map(|a| a.1).collect(); - - w.write_str(&joined) + fmt::from_fn(move |f| { + impls + .iter() + .map(|i| { + fmt::from_fn(|f| { + let did = i.trait_did().unwrap(); + let provided_trait_methods = i.inner_impl().provided_trait_methods(cx.tcx()); + let assoc_link = AssocItemLink::GotoSource(did.into(), &provided_trait_methods); + render_impl( + cx, + i, + containing_item, + assoc_link, + RenderMode::Normal, + None, + &[], + ImplRenderingParameters { + show_def_docs: true, + show_default_items: true, + show_non_assoc_items: true, + toggle_open_by_default, + }, + ) + .fmt(f) + }) + }) + .joined("", f) + }) } /// Build a (possibly empty) `href` attribute (a key-value pair) for the given associated item. @@ -1415,16 +1420,12 @@ fn render_all_impls( mut w: impl Write, cx: &Context<'_>, containing_item: &clean::Item, - concrete_impls: &[&Impl], - auto_trait_impls: &[&Impl], - blanket_impls: &[&Impl], + concrete_impls: Vec<&Impl>, + auto_trait_impls: Vec<&Impl>, + blanket_impls: Vec<&Impl>, ) -> fmt::Result { - let impls = { - let mut buf = String::new(); - render_impls(cx, &mut buf, concrete_impls, containing_item, true)?; - buf - }; - if !impls.is_empty() { + if !concrete_impls.is_empty() { + let impls = render_impls(cx, concrete_impls, containing_item, true); write!( w, "{}
{impls}
", @@ -1433,25 +1434,24 @@ fn render_all_impls( } if !auto_trait_impls.is_empty() { + let impls = render_impls(cx, auto_trait_impls, containing_item, false); // FIXME: Change the ID to `auto-trait-implementations-list`! write!( w, - "{}
", + "{}
{impls}
", write_impl_section_heading("Auto Trait Implementations", "synthetic-implementations",) )?; - render_impls(cx, &mut w, auto_trait_impls, containing_item, false)?; - w.write_str("
")?; } if !blanket_impls.is_empty() { + let impls = render_impls(cx, blanket_impls, containing_item, false); write!( w, - "{}
", + "{}
{impls}
", write_impl_section_heading("Blanket Implementations", "blanket-implementations") )?; - render_impls(cx, &mut w, blanket_impls, containing_item, false)?; - w.write_str("
")?; } + Ok(()) } @@ -1588,14 +1588,7 @@ fn render_assoc_items_inner( let (blanket_impls, concrete_impls): (Vec<&Impl>, _) = trait_impls.into_iter().partition(|t| t.inner_impl().kind.is_blanket()); - render_all_impls( - w, - cx, - containing_item, - &concrete_impls, - &auto_trait_impls, - &blanket_impls, - )?; + render_all_impls(w, cx, containing_item, concrete_impls, auto_trait_impls, blanket_impls)?; } Ok(()) } diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 8385e691ed385..7469b08b4b424 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -32,8 +32,8 @@ use crate::formats::item_type::ItemType; use crate::html::escape::{Escape, EscapeBodyTextWithWbr}; use crate::html::format::{ Ending, PrintWithSpace, full_print_fn_decl, print_abi_with_space, print_constness_with_space, - print_generic_bound, print_generics, print_impl, print_import, print_type, print_where_clause, - visibility_print_with_space, + print_generic_bound, print_generics, print_impl, print_import, print_path, print_type, + print_where_clause, visibility_print_with_space, }; use crate::html::markdown::{HeadingOffset, MarkdownSummaryLine}; use crate::html::render::sidebar::filters; @@ -1016,9 +1016,9 @@ fn item_trait(cx: &Context<'_>, it: &clean::Item, t: &clean::Trait) -> impl fmt: let (mut synthetic, mut concrete): (Vec<&&Impl>, Vec<&&Impl>) = local.iter().partition(|i| i.inner_impl().kind.is_auto()); - synthetic.sort_by_cached_key(|i| ImplString::new(i, cx)); - concrete.sort_by_cached_key(|i| ImplString::new(i, cx)); - foreign.sort_by_cached_key(|i| ImplString::new(i, cx)); + synthetic.sort_by_cached_key(|i| ImplString::new_impl(i, cx)); + concrete.sort_by_cached_key(|i| ImplString::new_impl(i, cx)); + foreign.sort_by_cached_key(|i| ImplString::new_impl(i, cx)); if !foreign.is_empty() { write!( @@ -1969,7 +1969,7 @@ fn item_primitive(cx: &Context<'_>, it: &clean::Item) -> impl fmt::Display { let (concrete, synthetic, blanket_impl) = get_filtered_impls_for_reference(&cx.shared, it); - render_all_impls(w, cx, it, &concrete, &synthetic, &blanket_impl) + render_all_impls(w, cx, it, concrete, synthetic, blanket_impl) } }) } @@ -2346,16 +2346,21 @@ where } #[derive(PartialEq, Eq)] -struct ImplString { +pub(super) struct ImplString { // Plain text (not HTML text) because this is only used for sorting purposes, and the plain // text is much shorter and thus faster to compare. cmp_text: String, } impl ImplString { - fn new(i: &Impl, cx: &Context<'_>) -> ImplString { + fn new_impl(i: &Impl, cx: &Context<'_>) -> Self { let impl_ = i.inner_impl(); - ImplString { cmp_text: format!("{:#}", print_impl(impl_, false, cx)) } + Self { cmp_text: format!("{:#}", print_impl(impl_, false, cx)) } + } + + pub(super) fn new_path(i: &Impl, cx: &Context<'_>) -> Option { + let path = i.inner_impl().trait_.as_ref()?; + Some(Self { cmp_text: format!("{:#}", print_path(path, cx)) }) } } diff --git a/tests/rustdoc-html/macro/const-rendering-macros-33302.rs b/tests/rustdoc-html/macro/const-rendering-macros-33302.rs index 9fd45df08be34..26b7d3d008481 100644 --- a/tests/rustdoc-html/macro/const-rendering-macros-33302.rs +++ b/tests/rustdoc-html/macro/const-rendering-macros-33302.rs @@ -1,5 +1,5 @@ // https://github.com/rust-lang/rust/issues/33302 -#![crate_name="issue_33302"] +#![crate_name = "issue_33302"] // Ensure constant and array length values are not taken from source // code, which wreaks havoc with macros. @@ -25,11 +25,12 @@ macro_rules! make { } //@ has issue_33302/struct.S.html \ - // '//*[@class="impl"]' 'impl T<[i32; 16]> for S' - //@ has - '//*[@id="associatedconstant.C"]' 'const C: [i32; 16]' + // '//*[@class="impl"]' 'impl T<(i32, i32)> for S' + //@ has - '//*[@id="associatedconstant.C"]' 'const C: (i32, i32)' //@ has - '//*[@id="associatedconstant.D"]' 'const D: i32' - impl T<[i32; ($n * $n)]> for S { - const C: [i32; ($n * $n)] = [0; ($n * $n)]; + impl T<(i32, i32)> for S { + const C: (i32, i32) = ($n, $n); + const D: i32 = ($n / $n); } //@ has issue_33302/struct.S.html \ @@ -41,12 +42,11 @@ macro_rules! make { } //@ has issue_33302/struct.S.html \ - // '//*[@class="impl"]' 'impl T<(i32, i32)> for S' - //@ has - '//*[@id="associatedconstant.C-2"]' 'const C: (i32, i32)' + // '//*[@class="impl"]' 'impl T<[i32; 16]> for S' + //@ has - '//*[@id="associatedconstant.C-2"]' 'const C: [i32; 16]' //@ has - '//*[@id="associatedconstant.D-2"]' 'const D: i32' - impl T<(i32, i32)> for S { - const C: (i32, i32) = ($n, $n); - const D: i32 = ($n / $n); + impl T<[i32; ($n * $n)]> for S { + const C: [i32; ($n * $n)] = [0; ($n * $n)]; } }; }