From cc671754a308177c2772b5f7e49939180b97d3a4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 10 Jun 2025 14:41:23 +1000 Subject: [PATCH 1/4] Simplify `JsonRenderer`. - It doesn't need to be cloneable. - Some of the `Rc`s and `RefCell`s aren't doing anything. - `after_krate` can consume `self`. --- src/librustdoc/formats/renderer.rs | 2 +- src/librustdoc/html/render/context.rs | 2 +- src/librustdoc/json/mod.rs | 15 +++++++-------- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/librustdoc/formats/renderer.rs b/src/librustdoc/formats/renderer.rs index 5e4e6f27a1541..072b04086f622 100644 --- a/src/librustdoc/formats/renderer.rs +++ b/src/librustdoc/formats/renderer.rs @@ -67,7 +67,7 @@ pub(crate) trait FormatRenderer<'tcx>: Sized { } /// Post processing hook for cleanup and dumping output to files. - fn after_krate(&mut self) -> Result<(), Error>; + fn after_krate(self) -> Result<(), Error>; fn cache(&self) -> &Cache; } diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 5984dcd74caf9..8af4a884ebe71 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -609,7 +609,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { self.info = info; } - fn after_krate(&mut self) -> Result<(), Error> { + fn after_krate(mut self) -> Result<(), Error> { let crate_name = self.tcx().crate_name(LOCAL_CRATE); let final_file = self.dst.join(crate_name.as_str()).join("all.html"); let settings_file = self.dst.join("settings.html"); diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs index 131a12ce228f7..180452fa85396 100644 --- a/src/librustdoc/json/mod.rs +++ b/src/librustdoc/json/mod.rs @@ -36,19 +36,18 @@ use crate::formats::cache::Cache; use crate::json::conversions::IntoJson; use crate::{clean, try_err}; -#[derive(Clone)] pub(crate) struct JsonRenderer<'tcx> { tcx: TyCtxt<'tcx>, /// A mapping of IDs that contains all local items for this crate which gets output as a top /// level field of the JSON blob. - index: Rc>>, + index: FxHashMap, /// The directory where the JSON blob should be written to. /// /// If this is `None`, the blob will be printed to `stdout` instead. out_dir: Option, cache: Rc, imported_items: DefIdSet, - id_interner: Rc>, + id_interner: RefCell, } impl<'tcx> JsonRenderer<'tcx> { @@ -197,7 +196,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { Ok(( JsonRenderer { tcx, - index: Rc::new(RefCell::new(FxHashMap::default())), + index: FxHashMap::default(), out_dir: if options.output_to_stdout { None } else { Some(options.output) }, cache: Rc::new(cache), imported_items, @@ -272,7 +271,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { | types::ItemEnum::Macro(_) | types::ItemEnum::ProcMacro(_) => false, }; - let removed = self.index.borrow_mut().insert(new_item.id, new_item.clone()); + let removed = self.index.insert(new_item.id, new_item.clone()); // FIXME(adotinthevoid): Currently, the index is duplicated. This is a sanity check // to make sure the items are unique. The main place this happens is when an item, is @@ -295,11 +294,11 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { unreachable!("RUN_ON_MODULE = false, should never call mod_item_in") } - fn after_krate(&mut self) -> Result<(), Error> { + fn after_krate(self) -> Result<(), Error> { debug!("Done with crate"); let e = ExternalCrate { crate_num: LOCAL_CRATE }; - let index = (*self.index).clone().into_inner(); + let index = self.index.clone(); // Note that tcx.rust_target_features is inappropriate here because rustdoc tries to run for // multiple targets: https://github.com/rust-lang/rust/pull/137632 @@ -324,7 +323,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { types::ItemSummary { crate_id: k.krate.as_u32(), path: path.iter().map(|s| s.to_string()).collect(), - kind: kind.into_json(self), + kind: kind.into_json(&self), }, ) }) From 195c985398a282aa56befff72b19069a0bf9306d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 10 Jun 2025 15:23:11 +1000 Subject: [PATCH 2/4] Avoid cloning `self.index` in `after_krate`. It can be very big. This reduces peak memory usage for some `--output-format=json` runs by up to 8%. --- src/librustdoc/json/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs index 180452fa85396..db068e0fe39c5 100644 --- a/src/librustdoc/json/mod.rs +++ b/src/librustdoc/json/mod.rs @@ -294,11 +294,13 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { unreachable!("RUN_ON_MODULE = false, should never call mod_item_in") } - fn after_krate(self) -> Result<(), Error> { + fn after_krate(mut self) -> Result<(), Error> { debug!("Done with crate"); let e = ExternalCrate { crate_num: LOCAL_CRATE }; - let index = self.index.clone(); + + // We've finished using the index, and don't want to clone it, because it is big. + let index = std::mem::take(&mut self.index); // Note that tcx.rust_target_features is inappropriate here because rustdoc tries to run for // multiple targets: https://github.com/rust-lang/rust/pull/137632 From 79b3c08bdb729e77c38957500eade13e5dd16923 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 10 Jun 2025 22:29:36 +1000 Subject: [PATCH 3/4] Avoid more clones in rustdoc JSON output. By making `JsonRenderer::item` take `&clean::Item` instead of a `clean::Item`. This required also changing `FromClean` and `IntoJson` methods to take references, which required a lot of follow-on sigil wrangling that is mostly tedious. --- src/librustdoc/formats/renderer.rs | 14 +- src/librustdoc/html/render/context.rs | 2 +- src/librustdoc/json/conversions.rs | 232 +++++++++++++------------- src/librustdoc/json/mod.rs | 10 +- 4 files changed, 133 insertions(+), 125 deletions(-) diff --git a/src/librustdoc/formats/renderer.rs b/src/librustdoc/formats/renderer.rs index 072b04086f622..48626171404f0 100644 --- a/src/librustdoc/formats/renderer.rs +++ b/src/librustdoc/formats/renderer.rs @@ -56,7 +56,7 @@ pub(crate) trait FormatRenderer<'tcx>: Sized { fn restore_module_data(&mut self, info: Self::ModuleData); /// Renders a single non-module item. This means no recursive sub-item rendering is required. - fn item(&mut self, item: clean::Item) -> Result<(), Error>; + fn item(&mut self, item: &clean::Item) -> Result<(), Error>; /// Renders a module (should not handle recursing into children). fn mod_item_in(&mut self, item: &clean::Item) -> Result<(), Error>; @@ -74,7 +74,7 @@ pub(crate) trait FormatRenderer<'tcx>: Sized { fn run_format_inner<'tcx, T: FormatRenderer<'tcx>>( cx: &mut T, - item: clean::Item, + item: &clean::Item, prof: &SelfProfilerRef, ) -> Result<(), Error> { if item.is_mod() && T::RUN_ON_MODULE { @@ -84,12 +84,12 @@ fn run_format_inner<'tcx, T: FormatRenderer<'tcx>>( prof.generic_activity_with_arg("render_mod_item", item.name.unwrap().to_string()); cx.mod_item_in(&item)?; - let (clean::StrippedItem(box clean::ModuleItem(module)) | clean::ModuleItem(module)) = - item.inner.kind + let (clean::StrippedItem(box clean::ModuleItem(ref module)) + | clean::ModuleItem(ref module)) = item.inner.kind else { unreachable!() }; - for it in module.items { + for it in module.items.iter() { let info = cx.save_module_data(); run_format_inner(cx, it, prof)?; cx.restore_module_data(info); @@ -101,7 +101,7 @@ fn run_format_inner<'tcx, T: FormatRenderer<'tcx>>( } else if let Some(item_name) = item.name && !item.is_extern_crate() { - prof.generic_activity_with_arg("render_item", item_name.as_str()).run(|| cx.item(item))?; + prof.generic_activity_with_arg("render_item", item_name.as_str()).run(|| cx.item(&item))?; } Ok(()) } @@ -125,7 +125,7 @@ pub(crate) fn run_format<'tcx, T: FormatRenderer<'tcx>>( } // Render the crate documentation - run_format_inner(&mut format_renderer, krate.module, prof)?; + run_format_inner(&mut format_renderer, &krate.module, prof)?; prof.verbose_generic_activity_with_arg("renderer_after_krate", T::descr()) .run(|| format_renderer.after_krate()) diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 8af4a884ebe71..3821445165754 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -830,7 +830,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { Ok(()) } - fn item(&mut self, item: clean::Item) -> Result<(), Error> { + fn item(&mut self, item: &clean::Item) -> Result<(), Error> { // Stripped modules survive the rustdoc passes (i.e., `strip-private`) // if they contain impls for public types. These modules can also // contain items such as publicly re-exported structures. diff --git a/src/librustdoc/json/conversions.rs b/src/librustdoc/json/conversions.rs index 3ade40940bc97..6bdf3b5fe3876 100644 --- a/src/librustdoc/json/conversions.rs +++ b/src/librustdoc/json/conversions.rs @@ -13,6 +13,7 @@ use rustc_metadata::rendered_const; use rustc_middle::{bug, ty}; use rustc_span::{Pos, Symbol, kw}; use rustdoc_json_types::*; +use thin_vec::ThinVec; use crate::clean::{self, ItemId}; use crate::formats::FormatRenderer; @@ -21,7 +22,7 @@ use crate::json::JsonRenderer; use crate::passes::collect_intra_doc_links::UrlFragment; impl JsonRenderer<'_> { - pub(super) fn convert_item(&self, item: clean::Item) -> Option { + pub(super) fn convert_item(&self, item: &clean::Item) -> Option { let deprecation = item.deprecation(self.tcx); let links = self .cache @@ -107,49 +108,54 @@ impl JsonRenderer<'_> { } } - fn ids(&self, items: impl IntoIterator) -> Vec { + fn ids(&self, items: &[clean::Item]) -> Vec { items - .into_iter() - .filter(|x| !x.is_stripped() && !x.is_keyword()) + .iter() + .filter(|i| !i.is_stripped() && !i.is_keyword()) .map(|i| self.id_from_item(&i)) .collect() } - fn ids_keeping_stripped( - &self, - items: impl IntoIterator, - ) -> Vec> { + fn ids_keeping_stripped(&self, items: &[clean::Item]) -> Vec> { items - .into_iter() + .iter() .map(|i| (!i.is_stripped() && !i.is_keyword()).then(|| self.id_from_item(&i))) .collect() } } pub(crate) trait FromClean { - fn from_clean(f: T, renderer: &JsonRenderer<'_>) -> Self; + fn from_clean(f: &T, renderer: &JsonRenderer<'_>) -> Self; } pub(crate) trait IntoJson { - fn into_json(self, renderer: &JsonRenderer<'_>) -> T; + fn into_json(&self, renderer: &JsonRenderer<'_>) -> T; } impl IntoJson for T where U: FromClean, { - fn into_json(self, renderer: &JsonRenderer<'_>) -> U { + fn into_json(&self, renderer: &JsonRenderer<'_>) -> U { U::from_clean(self, renderer) } } -impl FromClean for Vec +impl FromClean> for Vec where - I: IntoIterator, U: FromClean, { - fn from_clean(f: I, renderer: &JsonRenderer<'_>) -> Vec { - f.into_iter().map(|x| x.into_json(renderer)).collect() + fn from_clean(items: &Vec, renderer: &JsonRenderer<'_>) -> Vec { + items.iter().map(|i| i.into_json(renderer)).collect() + } +} + +impl FromClean> for Vec +where + U: FromClean, +{ + fn from_clean(items: &ThinVec, renderer: &JsonRenderer<'_>) -> Vec { + items.iter().map(|i| i.into_json(renderer)).collect() } } @@ -165,7 +171,7 @@ pub(crate) fn from_deprecation(deprecation: attrs::Deprecation) -> Deprecation { } impl FromClean for GenericArgs { - fn from_clean(args: clean::GenericArgs, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(args: &clean::GenericArgs, renderer: &JsonRenderer<'_>) -> Self { use clean::GenericArgs::*; match args { AngleBracketed { args, constraints } => GenericArgs::AngleBracketed { @@ -174,7 +180,7 @@ impl FromClean for GenericArgs { }, Parenthesized { inputs, output } => GenericArgs::Parenthesized { inputs: inputs.into_json(renderer), - output: output.map(|a| (*a).into_json(renderer)), + output: output.as_ref().map(|a| a.as_ref().into_json(renderer)), }, ReturnTypeNotation => GenericArgs::ReturnTypeNotation, } @@ -182,7 +188,7 @@ impl FromClean for GenericArgs { } impl FromClean for GenericArg { - fn from_clean(arg: clean::GenericArg, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(arg: &clean::GenericArg, renderer: &JsonRenderer<'_>) -> Self { use clean::GenericArg::*; match arg { Lifetime(l) => GenericArg::Lifetime(convert_lifetime(l)), @@ -195,7 +201,7 @@ impl FromClean for GenericArg { impl FromClean for Constant { // FIXME(generic_const_items): Add support for generic const items. - fn from_clean(constant: clean::Constant, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(constant: &clean::Constant, renderer: &JsonRenderer<'_>) -> Self { let tcx = renderer.tcx; let expr = constant.expr(tcx); let value = constant.value(tcx); @@ -206,7 +212,7 @@ impl FromClean for Constant { impl FromClean for Constant { // FIXME(generic_const_items): Add support for generic const items. - fn from_clean(constant: clean::ConstantKind, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(constant: &clean::ConstantKind, renderer: &JsonRenderer<'_>) -> Self { let tcx = renderer.tcx; let expr = constant.expr(tcx); let value = constant.value(tcx); @@ -216,7 +222,7 @@ impl FromClean for Constant { } impl FromClean for AssocItemConstraint { - fn from_clean(constraint: clean::AssocItemConstraint, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(constraint: &clean::AssocItemConstraint, renderer: &JsonRenderer<'_>) -> Self { AssocItemConstraint { name: constraint.assoc.name.to_string(), args: constraint.assoc.args.into_json(renderer), @@ -226,7 +232,7 @@ impl FromClean for AssocItemConstraint { } impl FromClean for AssocItemConstraintKind { - fn from_clean(kind: clean::AssocItemConstraintKind, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(kind: &clean::AssocItemConstraintKind, renderer: &JsonRenderer<'_>) -> Self { use clean::AssocItemConstraintKind::*; match kind { Equality { term } => AssocItemConstraintKind::Equality(term.into_json(renderer)), @@ -235,15 +241,15 @@ impl FromClean for AssocItemConstraintKind { } } -fn from_clean_item(item: clean::Item, renderer: &JsonRenderer<'_>) -> ItemEnum { +fn from_clean_item(item: &clean::Item, renderer: &JsonRenderer<'_>) -> ItemEnum { use clean::ItemKind::*; let name = item.name; let is_crate = item.is_crate(); let header = item.fn_header(renderer.tcx); - match item.inner.kind { + match &item.inner.kind { ModuleItem(m) => { - ItemEnum::Module(Module { is_crate, items: renderer.ids(m.items), is_stripped: false }) + ItemEnum::Module(Module { is_crate, items: renderer.ids(&m.items), is_stripped: false }) } ImportItem(i) => ItemEnum::Use(i.into_json(renderer)), StructItem(s) => ItemEnum::Struct(s.into_json(renderer)), @@ -251,27 +257,27 @@ fn from_clean_item(item: clean::Item, renderer: &JsonRenderer<'_>) -> ItemEnum { StructFieldItem(f) => ItemEnum::StructField(f.into_json(renderer)), EnumItem(e) => ItemEnum::Enum(e.into_json(renderer)), VariantItem(v) => ItemEnum::Variant(v.into_json(renderer)), - FunctionItem(f) => ItemEnum::Function(from_function(*f, true, header.unwrap(), renderer)), + FunctionItem(f) => ItemEnum::Function(from_function(f, true, header.unwrap(), renderer)), ForeignFunctionItem(f, _) => { - ItemEnum::Function(from_function(*f, false, header.unwrap(), renderer)) + ItemEnum::Function(from_function(f, false, header.unwrap(), renderer)) } - TraitItem(t) => ItemEnum::Trait((*t).into_json(renderer)), + TraitItem(t) => ItemEnum::Trait(t.as_ref().into_json(renderer)), TraitAliasItem(t) => ItemEnum::TraitAlias(t.into_json(renderer)), - MethodItem(m, _) => ItemEnum::Function(from_function(*m, true, header.unwrap(), renderer)), + MethodItem(m, _) => ItemEnum::Function(from_function(m, true, header.unwrap(), renderer)), RequiredMethodItem(m) => { - ItemEnum::Function(from_function(*m, false, header.unwrap(), renderer)) + ItemEnum::Function(from_function(m, false, header.unwrap(), renderer)) } - ImplItem(i) => ItemEnum::Impl((*i).into_json(renderer)), - StaticItem(s) => ItemEnum::Static(convert_static(s, rustc_hir::Safety::Safe, renderer)), + ImplItem(i) => ItemEnum::Impl(i.as_ref().into_json(renderer)), + StaticItem(s) => ItemEnum::Static(convert_static(s, &rustc_hir::Safety::Safe, renderer)), ForeignStaticItem(s, safety) => ItemEnum::Static(convert_static(s, safety, renderer)), ForeignTypeItem => ItemEnum::ExternType, - TypeAliasItem(t) => ItemEnum::TypeAlias(t.into_json(renderer)), + TypeAliasItem(t) => ItemEnum::TypeAlias(t.as_ref().into_json(renderer)), // FIXME(generic_const_items): Add support for generic free consts ConstantItem(ci) => ItemEnum::Constant { type_: ci.type_.into_json(renderer), const_: ci.kind.into_json(renderer), }, - MacroItem(m) => ItemEnum::Macro(m.source), + MacroItem(m) => ItemEnum::Macro(m.source.clone()), ProcMacroItem(m) => ItemEnum::ProcMacro(m.into_json(renderer)), PrimitiveItem(p) => { ItemEnum::Primitive(Primitive { @@ -281,7 +287,7 @@ fn from_clean_item(item: clean::Item, renderer: &JsonRenderer<'_>) -> ItemEnum { } // FIXME(generic_const_items): Add support for generic associated consts. RequiredAssocConstItem(_generics, ty) => { - ItemEnum::AssocConst { type_: (*ty).into_json(renderer), value: None } + ItemEnum::AssocConst { type_: ty.as_ref().into_json(renderer), value: None } } // FIXME(generic_const_items): Add support for generic associated consts. ProvidedAssocConstItem(ci) | ImplAssocConstItem(ci) => ItemEnum::AssocConst { @@ -296,22 +302,22 @@ fn from_clean_item(item: clean::Item, renderer: &JsonRenderer<'_>) -> ItemEnum { AssocTypeItem(t, b) => ItemEnum::AssocType { generics: t.generics.into_json(renderer), bounds: b.into_json(renderer), - type_: Some(t.item_type.unwrap_or(t.type_).into_json(renderer)), + type_: Some(t.item_type.as_ref().unwrap_or(&t.type_).into_json(renderer)), }, // `convert_item` early returns `None` for stripped items and keywords. KeywordItem => unreachable!(), StrippedItem(inner) => { - match *inner { + match inner.as_ref() { ModuleItem(m) => ItemEnum::Module(Module { is_crate, - items: renderer.ids(m.items), + items: renderer.ids(&m.items), is_stripped: true, }), // `convert_item` early returns `None` for stripped items we're not including _ => unreachable!(), } } - ExternCrateItem { ref src } => ItemEnum::ExternCrate { + ExternCrateItem { src } => ItemEnum::ExternCrate { name: name.as_ref().unwrap().to_string(), rename: src.map(|x| x.to_string()), }, @@ -319,17 +325,17 @@ fn from_clean_item(item: clean::Item, renderer: &JsonRenderer<'_>) -> ItemEnum { } impl FromClean for Struct { - fn from_clean(struct_: clean::Struct, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(struct_: &clean::Struct, renderer: &JsonRenderer<'_>) -> Self { let has_stripped_fields = struct_.has_stripped_entries(); let clean::Struct { ctor_kind, generics, fields } = struct_; let kind = match ctor_kind { - Some(CtorKind::Fn) => StructKind::Tuple(renderer.ids_keeping_stripped(fields)), + Some(CtorKind::Fn) => StructKind::Tuple(renderer.ids_keeping_stripped(&fields)), Some(CtorKind::Const) => { assert!(fields.is_empty()); StructKind::Unit } - None => StructKind::Plain { fields: renderer.ids(fields), has_stripped_fields }, + None => StructKind::Plain { fields: renderer.ids(&fields), has_stripped_fields }, }; Struct { @@ -341,13 +347,13 @@ impl FromClean for Struct { } impl FromClean for Union { - fn from_clean(union_: clean::Union, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(union_: &clean::Union, renderer: &JsonRenderer<'_>) -> Self { let has_stripped_fields = union_.has_stripped_entries(); let clean::Union { generics, fields } = union_; Union { generics: generics.into_json(renderer), has_stripped_fields, - fields: renderer.ids(fields), + fields: renderer.ids(&fields), impls: Vec::new(), // Added in JsonRenderer::item } } @@ -377,12 +383,12 @@ fn convert_abi(a: ExternAbi) -> Abi { } } -fn convert_lifetime(l: clean::Lifetime) -> String { +fn convert_lifetime(l: &clean::Lifetime) -> String { l.0.to_string() } impl FromClean for Generics { - fn from_clean(generics: clean::Generics, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(generics: &clean::Generics, renderer: &JsonRenderer<'_>) -> Self { Generics { params: generics.params.into_json(renderer), where_predicates: generics.where_predicates.into_json(renderer), @@ -391,7 +397,7 @@ impl FromClean for Generics { } impl FromClean for GenericParamDef { - fn from_clean(generic_param: clean::GenericParamDef, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(generic_param: &clean::GenericParamDef, renderer: &JsonRenderer<'_>) -> Self { GenericParamDef { name: generic_param.name.to_string(), kind: generic_param.kind.into_json(renderer), @@ -400,7 +406,7 @@ impl FromClean for GenericParamDef { } impl FromClean for GenericParamDefKind { - fn from_clean(kind: clean::GenericParamDefKind, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(kind: &clean::GenericParamDefKind, renderer: &JsonRenderer<'_>) -> Self { use clean::GenericParamDefKind::*; match kind { Lifetime { outlives } => GenericParamDefKind::Lifetime { @@ -408,29 +414,29 @@ impl FromClean for GenericParamDefKind { }, Type { bounds, default, synthetic } => GenericParamDefKind::Type { bounds: bounds.into_json(renderer), - default: default.map(|x| (*x).into_json(renderer)), - is_synthetic: synthetic, + default: default.as_ref().map(|x| x.as_ref().into_json(renderer)), + is_synthetic: *synthetic, }, Const { ty, default, synthetic: _ } => GenericParamDefKind::Const { - type_: (*ty).into_json(renderer), - default: default.map(|x| *x), + type_: ty.as_ref().into_json(renderer), + default: default.as_ref().map(|x| x.as_ref().clone()), }, } } } impl FromClean for WherePredicate { - fn from_clean(predicate: clean::WherePredicate, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(predicate: &clean::WherePredicate, renderer: &JsonRenderer<'_>) -> Self { use clean::WherePredicate::*; match predicate { BoundPredicate { ty, bounds, bound_params } => WherePredicate::BoundPredicate { type_: ty.into_json(renderer), bounds: bounds.into_json(renderer), generic_params: bound_params - .into_iter() + .iter() .map(|x| { let name = x.name.to_string(); - let kind = match x.kind { + let kind = match &x.kind { clean::GenericParamDefKind::Lifetime { outlives } => { GenericParamDefKind::Lifetime { outlives: outlives.iter().map(|lt| lt.0.to_string()).collect(), @@ -442,14 +448,16 @@ impl FromClean for WherePredicate { .into_iter() .map(|bound| bound.into_json(renderer)) .collect(), - default: default.map(|ty| (*ty).into_json(renderer)), - is_synthetic: synthetic, + default: default + .as_ref() + .map(|ty| ty.as_ref().into_json(renderer)), + is_synthetic: *synthetic, } } clean::GenericParamDefKind::Const { ty, default, synthetic: _ } => { GenericParamDefKind::Const { - type_: (*ty).into_json(renderer), - default: default.map(|d| *d), + type_: ty.as_ref().into_json(renderer), + default: default.as_ref().map(|d| d.as_ref().clone()), } } }; @@ -462,7 +470,7 @@ impl FromClean for WherePredicate { outlives: bounds .iter() .map(|bound| match bound { - clean::GenericBound::Outlives(lt) => convert_lifetime(*lt), + clean::GenericBound::Outlives(lt) => convert_lifetime(lt), _ => bug!("found non-outlives-bound on lifetime predicate"), }) .collect(), @@ -479,7 +487,7 @@ impl FromClean for WherePredicate { } impl FromClean for GenericBound { - fn from_clean(bound: clean::GenericBound, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(bound: &clean::GenericBound, renderer: &JsonRenderer<'_>) -> Self { use clean::GenericBound::*; match bound { TraitBound(clean::PolyTrait { trait_, generic_params }, modifier) => { @@ -494,7 +502,7 @@ impl FromClean for GenericBound { args.iter() .map(|arg| match arg { clean::PreciseCapturingArg::Lifetime(lt) => { - PreciseCapturingArg::Lifetime(convert_lifetime(*lt)) + PreciseCapturingArg::Lifetime(convert_lifetime(lt)) } clean::PreciseCapturingArg::Param(param) => { PreciseCapturingArg::Param(param.to_string()) @@ -507,7 +515,7 @@ impl FromClean for GenericBound { } pub(crate) fn from_trait_bound_modifier( - modifiers: rustc_hir::TraitBoundModifiers, + modifiers: &rustc_hir::TraitBoundModifiers, ) -> TraitBoundModifier { use rustc_hir as hir; let hir::TraitBoundModifiers { constness, polarity } = modifiers; @@ -523,7 +531,7 @@ pub(crate) fn from_trait_bound_modifier( } impl FromClean for Type { - fn from_clean(ty: clean::Type, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(ty: &clean::Type, renderer: &JsonRenderer<'_>) -> Self { use clean::Type::{ Array, BareFunction, BorrowedRef, Generic, ImplTrait, Infer, Primitive, QPath, RawPointer, SelfTy, Slice, Tuple, UnsafeBinder, @@ -532,35 +540,35 @@ impl FromClean for Type { match ty { clean::Type::Path { path } => Type::ResolvedPath(path.into_json(renderer)), clean::Type::DynTrait(bounds, lt) => Type::DynTrait(DynTrait { - lifetime: lt.map(convert_lifetime), + lifetime: lt.as_ref().map(convert_lifetime), traits: bounds.into_json(renderer), }), Generic(s) => Type::Generic(s.to_string()), // FIXME: add dedicated variant to json Type? SelfTy => Type::Generic("Self".to_owned()), Primitive(p) => Type::Primitive(p.as_sym().to_string()), - BareFunction(f) => Type::FunctionPointer(Box::new((*f).into_json(renderer))), + BareFunction(f) => Type::FunctionPointer(Box::new(f.as_ref().into_json(renderer))), Tuple(t) => Type::Tuple(t.into_json(renderer)), - Slice(t) => Type::Slice(Box::new((*t).into_json(renderer))), + Slice(t) => Type::Slice(Box::new(t.as_ref().into_json(renderer))), Array(t, s) => { - Type::Array { type_: Box::new((*t).into_json(renderer)), len: s.to_string() } + Type::Array { type_: Box::new(t.as_ref().into_json(renderer)), len: s.to_string() } } clean::Type::Pat(t, p) => Type::Pat { - type_: Box::new((*t).into_json(renderer)), + type_: Box::new(t.as_ref().into_json(renderer)), __pat_unstable_do_not_use: p.to_string(), }, ImplTrait(g) => Type::ImplTrait(g.into_json(renderer)), Infer => Type::Infer, RawPointer(mutability, type_) => Type::RawPointer { - is_mutable: mutability == ast::Mutability::Mut, - type_: Box::new((*type_).into_json(renderer)), + is_mutable: *mutability == ast::Mutability::Mut, + type_: Box::new(type_.as_ref().into_json(renderer)), }, BorrowedRef { lifetime, mutability, type_ } => Type::BorrowedRef { - lifetime: lifetime.map(convert_lifetime), - is_mutable: mutability == ast::Mutability::Mut, - type_: Box::new((*type_).into_json(renderer)), + lifetime: lifetime.as_ref().map(convert_lifetime), + is_mutable: *mutability == ast::Mutability::Mut, + type_: Box::new(type_.as_ref().into_json(renderer)), }, - QPath(qpath) => (*qpath).into_json(renderer), + QPath(qpath) => qpath.as_ref().into_json(renderer), // FIXME(unsafe_binder): Implement rustdoc-json. UnsafeBinder(_) => todo!(), } @@ -568,30 +576,30 @@ impl FromClean for Type { } impl FromClean for Path { - fn from_clean(path: clean::Path, renderer: &JsonRenderer<'_>) -> Path { + fn from_clean(path: &clean::Path, renderer: &JsonRenderer<'_>) -> Path { Path { path: path.whole_name(), id: renderer.id_from_item_default(path.def_id().into()), - args: path.segments.last().map(|args| Box::new(args.clone().args.into_json(renderer))), + args: path.segments.last().map(|args| Box::new(args.args.into_json(renderer))), } } } impl FromClean for Type { - fn from_clean(qpath: clean::QPathData, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(qpath: &clean::QPathData, renderer: &JsonRenderer<'_>) -> Self { let clean::QPathData { assoc, self_type, should_fully_qualify: _, trait_ } = qpath; Self::QualifiedPath { name: assoc.name.to_string(), args: Box::new(assoc.args.into_json(renderer)), self_type: Box::new(self_type.into_json(renderer)), - trait_: trait_.map(|trait_| trait_.into_json(renderer)), + trait_: trait_.as_ref().map(|trait_| trait_.into_json(renderer)), } } } impl FromClean for Term { - fn from_clean(term: clean::Term, renderer: &JsonRenderer<'_>) -> Term { + fn from_clean(term: &clean::Term, renderer: &JsonRenderer<'_>) -> Term { match term { clean::Term::Type(ty) => Term::Type(ty.into_json(renderer)), clean::Term::Constant(c) => Term::Constant(c.into_json(renderer)), @@ -600,14 +608,14 @@ impl FromClean for Term { } impl FromClean for FunctionPointer { - fn from_clean(bare_decl: clean::BareFunctionDecl, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(bare_decl: &clean::BareFunctionDecl, renderer: &JsonRenderer<'_>) -> Self { let clean::BareFunctionDecl { safety, generic_params, decl, abi } = bare_decl; FunctionPointer { header: FunctionHeader { is_unsafe: safety.is_unsafe(), is_const: false, is_async: false, - abi: convert_abi(abi), + abi: convert_abi(*abi), }, generic_params: generic_params.into_json(renderer), sig: decl.into_json(renderer), @@ -616,7 +624,7 @@ impl FromClean for FunctionPointer { } impl FromClean for FunctionSignature { - fn from_clean(decl: clean::FnDecl, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(decl: &clean::FnDecl, renderer: &JsonRenderer<'_>) -> Self { let clean::FnDecl { inputs, output, c_variadic } = decl; FunctionSignature { inputs: inputs @@ -629,13 +637,13 @@ impl FromClean for FunctionSignature { }) .collect(), output: if output.is_unit() { None } else { Some(output.into_json(renderer)) }, - is_c_variadic: c_variadic, + is_c_variadic: *c_variadic, } } } impl FromClean for Trait { - fn from_clean(trait_: clean::Trait, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(trait_: &clean::Trait, renderer: &JsonRenderer<'_>) -> Self { let tcx = renderer.tcx; let is_auto = trait_.is_auto(tcx); let is_unsafe = trait_.safety(tcx).is_unsafe(); @@ -645,7 +653,7 @@ impl FromClean for Trait { is_auto, is_unsafe, is_dyn_compatible, - items: renderer.ids(items), + items: renderer.ids(&items), generics: generics.into_json(renderer), bounds: bounds.into_json(renderer), implementations: Vec::new(), // Added in JsonRenderer::item @@ -655,7 +663,7 @@ impl FromClean for Trait { impl FromClean for PolyTrait { fn from_clean( - clean::PolyTrait { trait_, generic_params }: clean::PolyTrait, + clean::PolyTrait { trait_, generic_params }: &clean::PolyTrait, renderer: &JsonRenderer<'_>, ) -> Self { PolyTrait { @@ -666,14 +674,14 @@ impl FromClean for PolyTrait { } impl FromClean for Impl { - fn from_clean(impl_: clean::Impl, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(impl_: &clean::Impl, renderer: &JsonRenderer<'_>) -> Self { let provided_trait_methods = impl_.provided_trait_methods(renderer.tcx); let clean::Impl { safety, generics, trait_, for_, items, polarity, kind } = impl_; // FIXME: use something like ImplKind in JSON? let (is_synthetic, blanket_impl) = match kind { clean::ImplKind::Normal | clean::ImplKind::FakeVariadic => (false, None), clean::ImplKind::Auto => (true, None), - clean::ImplKind::Blanket(ty) => (false, Some(*ty)), + clean::ImplKind::Blanket(ty) => (false, Some(ty)), }; let is_negative = match polarity { ty::ImplPolarity::Positive | ty::ImplPolarity::Reservation => false, @@ -686,18 +694,18 @@ impl FromClean for Impl { .into_iter() .map(|x| x.to_string()) .collect(), - trait_: trait_.map(|path| path.into_json(renderer)), + trait_: trait_.as_ref().map(|path| path.into_json(renderer)), for_: for_.into_json(renderer), - items: renderer.ids(items), + items: renderer.ids(&items), is_negative, is_synthetic, - blanket_impl: blanket_impl.map(|x| x.into_json(renderer)), + blanket_impl: blanket_impl.map(|x| x.as_ref().into_json(renderer)), } } } pub(crate) fn from_function( - clean::Function { decl, generics }: clean::Function, + clean::Function { decl, generics }: &clean::Function, has_body: bool, header: rustc_hir::FnHeader, renderer: &JsonRenderer<'_>, @@ -711,30 +719,30 @@ pub(crate) fn from_function( } impl FromClean for Enum { - fn from_clean(enum_: clean::Enum, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(enum_: &clean::Enum, renderer: &JsonRenderer<'_>) -> Self { let has_stripped_variants = enum_.has_stripped_entries(); let clean::Enum { variants, generics } = enum_; Enum { generics: generics.into_json(renderer), has_stripped_variants, - variants: renderer.ids(variants), + variants: renderer.ids(&variants.as_slice().raw), impls: Vec::new(), // Added in JsonRenderer::item } } } impl FromClean for Variant { - fn from_clean(variant: clean::Variant, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(variant: &clean::Variant, renderer: &JsonRenderer<'_>) -> Self { use clean::VariantKind::*; - let discriminant = variant.discriminant.map(|d| d.into_json(renderer)); + let discriminant = variant.discriminant.as_ref().map(|d| d.into_json(renderer)); - let kind = match variant.kind { + let kind = match &variant.kind { CLike => VariantKind::Plain, - Tuple(fields) => VariantKind::Tuple(renderer.ids_keeping_stripped(fields)), + Tuple(fields) => VariantKind::Tuple(renderer.ids_keeping_stripped(&fields)), Struct(s) => VariantKind::Struct { has_stripped_fields: s.has_stripped_entries(), - fields: renderer.ids(s.fields), + fields: renderer.ids(&s.fields), }, }; @@ -743,7 +751,7 @@ impl FromClean for Variant { } impl FromClean for Discriminant { - fn from_clean(disr: clean::Discriminant, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(disr: &clean::Discriminant, renderer: &JsonRenderer<'_>) -> Self { let tcx = renderer.tcx; Discriminant { // expr is only none if going through the inlining path, which gets @@ -756,7 +764,7 @@ impl FromClean for Discriminant { } impl FromClean for Use { - fn from_clean(import: clean::Import, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(import: &clean::Import, renderer: &JsonRenderer<'_>) -> Self { use clean::ImportKind::*; let (name, is_glob) = match import.kind { Simple(s) => (s.to_string(), false), @@ -775,7 +783,7 @@ impl FromClean for Use { } impl FromClean for ProcMacro { - fn from_clean(mac: clean::ProcMacro, _renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(mac: &clean::ProcMacro, _renderer: &JsonRenderer<'_>) -> Self { ProcMacro { kind: from_macro_kind(mac.kind), helpers: mac.helpers.iter().map(|x| x.to_string()).collect(), @@ -792,21 +800,21 @@ pub(crate) fn from_macro_kind(kind: rustc_span::hygiene::MacroKind) -> MacroKind } } -impl FromClean> for TypeAlias { - fn from_clean(type_alias: Box, renderer: &JsonRenderer<'_>) -> Self { - let clean::TypeAlias { type_, generics, item_type: _, inner_type: _ } = *type_alias; +impl FromClean for TypeAlias { + fn from_clean(type_alias: &clean::TypeAlias, renderer: &JsonRenderer<'_>) -> Self { + let clean::TypeAlias { type_, generics, item_type: _, inner_type: _ } = type_alias; TypeAlias { type_: type_.into_json(renderer), generics: generics.into_json(renderer) } } } fn convert_static( - stat: clean::Static, - safety: rustc_hir::Safety, + stat: &clean::Static, + safety: &rustc_hir::Safety, renderer: &JsonRenderer<'_>, ) -> Static { let tcx = renderer.tcx; Static { - type_: (*stat.type_).into_json(renderer), + type_: stat.type_.as_ref().into_json(renderer), is_mutable: stat.mutability == ast::Mutability::Mut, is_unsafe: safety.is_unsafe(), expr: stat @@ -817,7 +825,7 @@ fn convert_static( } impl FromClean for TraitAlias { - fn from_clean(alias: clean::TraitAlias, renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(alias: &clean::TraitAlias, renderer: &JsonRenderer<'_>) -> Self { TraitAlias { generics: alias.generics.into_json(renderer), params: alias.bounds.into_json(renderer), @@ -826,7 +834,7 @@ impl FromClean for TraitAlias { } impl FromClean for ItemKind { - fn from_clean(kind: ItemType, _renderer: &JsonRenderer<'_>) -> Self { + fn from_clean(kind: &ItemType, _renderer: &JsonRenderer<'_>) -> Self { use ItemType::*; match kind { Module => ItemKind::Module, diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs index db068e0fe39c5..965212f019ff2 100644 --- a/src/librustdoc/json/mod.rs +++ b/src/librustdoc/json/mod.rs @@ -64,7 +64,7 @@ impl<'tcx> JsonRenderer<'tcx> { .iter() .map(|i| { let item = &i.impl_item; - self.item(item.clone()).unwrap(); + self.item(item).unwrap(); self.id_from_item(item) }) .collect() @@ -95,7 +95,7 @@ impl<'tcx> JsonRenderer<'tcx> { } if item.item_id.is_local() || is_primitive_impl { - self.item(item.clone()).unwrap(); + self.item(item).unwrap(); Some(self.id_from_item(item)) } else { None @@ -216,7 +216,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { /// Inserts an item into the index. This should be used rather than directly calling insert on /// the hashmap because certain items (traits and types) need to have their mappings for trait /// implementations filled out before they're inserted. - fn item(&mut self, item: clean::Item) -> Result<(), Error> { + fn item(&mut self, item: &clean::Item) -> Result<(), Error> { let item_type = item.type_(); let item_name = item.name; trace!("rendering {item_type} {item_name:?}"); @@ -224,11 +224,11 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { // Flatten items that recursively store other items. We include orphaned items from // stripped modules and etc that are otherwise reachable. if let ItemKind::StrippedItem(inner) = &item.kind { - inner.inner_items().for_each(|i| self.item(i.clone()).unwrap()); + inner.inner_items().for_each(|i| self.item(i).unwrap()); } // Flatten items that recursively store other items - item.kind.inner_items().for_each(|i| self.item(i.clone()).unwrap()); + item.kind.inner_items().for_each(|i| self.item(i).unwrap()); let item_id = item.item_id; if let Some(mut new_item) = self.convert_item(item) { From 278f4b2d9c2cdc741d38c78f14a60e78e2f668f4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 11 Jun 2025 12:54:19 +1000 Subject: [PATCH 4/4] Don't clone `new_item` in `after_krate`. We can avoid it by using the `entry` API, which lets us do the `assert_eq` comparison before `new_item` is consumed. --- src/librustdoc/json/mod.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs index 965212f019ff2..29c63a391e215 100644 --- a/src/librustdoc/json/mod.rs +++ b/src/librustdoc/json/mod.rs @@ -217,6 +217,8 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { /// the hashmap because certain items (traits and types) need to have their mappings for trait /// implementations filled out before they're inserted. fn item(&mut self, item: &clean::Item) -> Result<(), Error> { + use std::collections::hash_map::Entry; + let item_type = item.type_(); let item_name = item.name; trace!("rendering {item_type} {item_name:?}"); @@ -271,18 +273,25 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { | types::ItemEnum::Macro(_) | types::ItemEnum::ProcMacro(_) => false, }; - let removed = self.index.insert(new_item.id, new_item.clone()); // FIXME(adotinthevoid): Currently, the index is duplicated. This is a sanity check // to make sure the items are unique. The main place this happens is when an item, is // reexported in more than one place. See `rustdoc-json/reexport/in_root_and_mod` - if let Some(old_item) = removed { - // In case of generic implementations (like `impl Trait for T {}`), all the - // inner items will be duplicated so we can ignore if they are slightly different. - if !can_be_ignored { - assert_eq!(old_item, new_item); + match self.index.entry(new_item.id) { + Entry::Vacant(entry) => { + entry.insert(new_item); + } + Entry::Occupied(mut entry) => { + // In case of generic implementations (like `impl Trait for T {}`), all the + // inner items will be duplicated so we can ignore if they are slightly + // different. + let old_item = entry.get_mut(); + if !can_be_ignored { + assert_eq!(*old_item, new_item); + } + trace!("replaced {old_item:?}\nwith {new_item:?}"); + *old_item = new_item; } - trace!("replaced {old_item:?}\nwith {new_item:?}"); } }