From ff88b59e58a03dc142e80f35f4ce6177e07ad8ba Mon Sep 17 00:00:00 2001 From: zredb Date: Mon, 10 Jan 2022 10:44:18 +0800 Subject: [PATCH 1/6] fix #90187 --- src/librustdoc/formats/cache.rs | 2 +- src/librustdoc/html/render/search_index.rs | 23 +++++++++----------- src/librustdoc/passes/collect_trait_impls.rs | 4 ++-- src/librustdoc/passes/stripper.rs | 3 ++- 4 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index a8fef4a317802..53159709586c6 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -303,7 +303,7 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { desc, parent, parent_idx: None, - search_type: get_function_type_for_search(&item, self.tcx), + search_type: get_function_type_for_search(&item, self.tcx, self.cache), aliases: item.attrs.get_doc_aliases(), }); } diff --git a/src/librustdoc/html/render/search_index.rs b/src/librustdoc/html/render/search_index.rs index 87138b9571c2a..0b6eeb5182502 100644 --- a/src/librustdoc/html/render/search_index.rs +++ b/src/librustdoc/html/render/search_index.rs @@ -33,7 +33,7 @@ crate fn build_index<'tcx>(krate: &clean::Crate, cache: &mut Cache, tcx: TyCtxt< desc, parent: Some(did), parent_idx: None, - search_type: get_function_type_for_search(item, tcx), + search_type: get_function_type_for_search(item, tcx, &cache), aliases: item.attrs.get_doc_aliases(), }); } @@ -188,11 +188,12 @@ crate fn build_index<'tcx>(krate: &clean::Crate, cache: &mut Cache, tcx: TyCtxt< crate fn get_function_type_for_search<'tcx>( item: &clean::Item, tcx: TyCtxt<'tcx>, + cache: &Cache, ) -> Option { let (mut inputs, mut output) = match *item.kind { - clean::FunctionItem(ref f) => get_fn_inputs_and_outputs(f, tcx), - clean::MethodItem(ref m, _) => get_fn_inputs_and_outputs(m, tcx), - clean::TyMethodItem(ref m) => get_fn_inputs_and_outputs(m, tcx), + clean::FunctionItem(ref f) => get_fn_inputs_and_outputs(f, tcx, cache), + clean::MethodItem(ref m, _) => get_fn_inputs_and_outputs(m, tcx, cache), + clean::TyMethodItem(ref m) => get_fn_inputs_and_outputs(m, tcx, cache), _ => return None, }; @@ -311,7 +312,7 @@ fn add_generics_and_bounds_as_types<'tcx>( // We remove the name of the full generic because we have no use for it. index_ty.name = Some(String::new()); res.push(TypeWithKind::from((index_ty, ItemType::Generic))); - } else if let Some(kind) = ty.def_id_no_primitives().map(|did| tcx.def_kind(did).into()) { + } else if let Some(kind) = ty.def_id(cache).map(|did| tcx.def_kind(did).into()) { res.push(TypeWithKind::from((index_ty, kind))); } else if ty.is_primitive() { // This is a primitive, let's store it as such. @@ -330,9 +331,7 @@ fn add_generics_and_bounds_as_types<'tcx>( if let Type::Generic(arg_s) = *arg { // First we check if the bounds are in a `where` predicate... if let Some(where_pred) = generics.where_predicates.iter().find(|g| match g { - WherePredicate::BoundPredicate { ty, .. } => { - ty.def_id_no_primitives() == arg.def_id_no_primitives() - } + WherePredicate::BoundPredicate { ty, .. } => ty.def_id(cache) == arg.def_id(cache), _ => false, }) { let mut ty_generics = Vec::new(); @@ -397,6 +396,7 @@ fn add_generics_and_bounds_as_types<'tcx>( fn get_fn_inputs_and_outputs<'tcx>( func: &Function, tcx: TyCtxt<'tcx>, + cache: &Cache, ) -> (Vec, Vec) { let decl = &func.decl; let generics = &func.generics; @@ -411,8 +411,7 @@ fn get_fn_inputs_and_outputs<'tcx>( if !args.is_empty() { all_types.extend(args); } else { - if let Some(kind) = arg.type_.def_id_no_primitives().map(|did| tcx.def_kind(did).into()) - { + if let Some(kind) = arg.type_.def_id(cache).map(|did| tcx.def_kind(did).into()) { all_types.push(TypeWithKind::from((get_index_type(&arg.type_, vec![]), kind))); } } @@ -423,9 +422,7 @@ fn get_fn_inputs_and_outputs<'tcx>( FnRetTy::Return(ref return_type) => { add_generics_and_bounds_as_types(generics, return_type, tcx, 0, &mut ret_types); if ret_types.is_empty() { - if let Some(kind) = - return_type.def_id_no_primitives().map(|did| tcx.def_kind(did).into()) - { + if let Some(kind) = return_type.def_id(cache).map(|did| tcx.def_kind(did).into()) { ret_types.push(TypeWithKind::from((get_index_type(return_type, vec![]), kind))); } } diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index 66ac612ea37c4..fd52d93cb382b 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -102,7 +102,7 @@ crate fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) -> Crate } else if let Some(did) = target.def_id(&cx.cache) { cleaner.items.insert(did.into()); } - if let Some(for_did) = for_.def_id_no_primitives() { + if let Some(for_did) = for_.def_id(&cx.cache) { if type_did_to_deref_target.insert(for_did, target).is_none() { // Since only the `DefId` portion of the `Type` instances is known to be same for both the // `Deref` target type and the impl for type positions, this map of types is keyed by @@ -216,7 +216,7 @@ impl BadImplStripper { true } else if let Some(prim) = ty.primitive_type() { self.prims.contains(&prim) - } else if let Some(did) = ty.def_id_no_primitives() { + } else if let Some(did) = ty.def_id(&cx.cache) { is_deref || self.keep_impl_with_def_id(did.into()) } else { false diff --git a/src/librustdoc/passes/stripper.rs b/src/librustdoc/passes/stripper.rs index 7b07974ae01c6..ed0b2c0a5d725 100644 --- a/src/librustdoc/passes/stripper.rs +++ b/src/librustdoc/passes/stripper.rs @@ -118,6 +118,7 @@ impl<'a> DocFolder for Stripper<'a> { /// This stripper discards all impls which reference stripped items crate struct ImplStripper<'a> { crate retained: &'a ItemIdSet, + crate cache: &'a Cache, } impl<'a> DocFolder for ImplStripper<'a> { @@ -127,7 +128,7 @@ impl<'a> DocFolder for ImplStripper<'a> { if imp.trait_.is_none() && imp.items.is_empty() { return None; } - if let Some(did) = imp.for_.def_id_no_primitives() { + if let Some(did) = imp.for_.def_id(self.cache) { if did.is_local() && !imp.for_.is_assoc_ty() && !self.retained.contains(&did.into()) { debug!("ImplStripper: impl item for stripped type; removing"); From 0dd2703f8a3f8022da1943cdfc3e53df93f5c896 Mon Sep 17 00:00:00 2001 From: zredb Date: Mon, 10 Jan 2022 12:55:21 +0800 Subject: [PATCH 2/6] fix #90187 --- src/librustdoc/clean/types.rs | 1 + src/librustdoc/formats/cache.rs | 2 +- src/librustdoc/html/render/search_index.rs | 23 +++++++++++++++----- src/librustdoc/passes/collect_trait_impls.rs | 12 +++++----- src/librustdoc/passes/strip_hidden.rs | 4 ++-- src/librustdoc/passes/strip_private.rs | 2 +- src/librustdoc/passes/stripper.rs | 1 + 7 files changed, 30 insertions(+), 15 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 00c6e38839f54..125afb2192c44 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -338,6 +338,7 @@ impl ExternalCrate { } /// Indicates where an external crate can be found. +#[derive(Debug)] crate enum ExternalLocation { /// Remote URL root of the external crate Remote(String), diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index 53159709586c6..c9aa326373906 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -25,7 +25,7 @@ use crate::html::render::IndexItem; /// to be a fairly large and expensive structure to clone. Instead this adheres /// to `Send` so it may be stored in an `Arc` instance and shared among the various /// rendering threads. -#[derive(Default)] +#[derive(Default, Debug)] crate struct Cache { /// Maps a type ID to all known implementations for that type. This is only /// recognized for intra-crate [`clean::Type::Path`]s, and is used to print diff --git a/src/librustdoc/html/render/search_index.rs b/src/librustdoc/html/render/search_index.rs index 0b6eeb5182502..fa965ded17698 100644 --- a/src/librustdoc/html/render/search_index.rs +++ b/src/librustdoc/html/render/search_index.rs @@ -248,12 +248,14 @@ fn add_generics_and_bounds_as_types<'tcx>( tcx: TyCtxt<'tcx>, recurse: usize, res: &mut Vec, + cache: &Cache, ) { fn insert_ty( res: &mut Vec, tcx: TyCtxt<'_>, ty: Type, mut generics: Vec, + cache: &Cache, ) { let is_full_generic = ty.is_full_generic(); @@ -347,6 +349,7 @@ fn add_generics_and_bounds_as_types<'tcx>( tcx, recurse + 1, &mut ty_generics, + cache, ) } _ => {} @@ -354,7 +357,7 @@ fn add_generics_and_bounds_as_types<'tcx>( } } } - insert_ty(res, tcx, arg.clone(), ty_generics); + insert_ty(res, tcx, arg.clone(), ty_generics, cache); } // Otherwise we check if the trait bounds are "inlined" like `T: Option`... if let Some(bound) = generics.params.iter().find(|g| g.is_type() && g.name == arg_s) { @@ -368,10 +371,11 @@ fn add_generics_and_bounds_as_types<'tcx>( tcx, recurse + 1, &mut ty_generics, + cache, ); } } - insert_ty(res, tcx, arg.clone(), ty_generics); + insert_ty(res, tcx, arg.clone(), ty_generics, cache); } } else { // This is not a type parameter. So for example if we have `T, U: Option`, and we're @@ -382,10 +386,17 @@ fn add_generics_and_bounds_as_types<'tcx>( let mut ty_generics = Vec::new(); if let Some(arg_generics) = arg.generics() { for gen in arg_generics.iter() { - add_generics_and_bounds_as_types(generics, gen, tcx, recurse + 1, &mut ty_generics); + add_generics_and_bounds_as_types( + generics, + gen, + tcx, + recurse + 1, + &mut ty_generics, + cache, + ); } } - insert_ty(res, tcx, arg.clone(), ty_generics); + insert_ty(res, tcx, arg.clone(), ty_generics, cache); } } @@ -407,7 +418,7 @@ fn get_fn_inputs_and_outputs<'tcx>( continue; } let mut args = Vec::new(); - add_generics_and_bounds_as_types(generics, &arg.type_, tcx, 0, &mut args); + add_generics_and_bounds_as_types(generics, &arg.type_, tcx, 0, &mut args, cache); if !args.is_empty() { all_types.extend(args); } else { @@ -420,7 +431,7 @@ fn get_fn_inputs_and_outputs<'tcx>( let mut ret_types = Vec::new(); match decl.output { FnRetTy::Return(ref return_type) => { - add_generics_and_bounds_as_types(generics, return_type, tcx, 0, &mut ret_types); + add_generics_and_bounds_as_types(generics, return_type, tcx, 0, &mut ret_types, cache); if ret_types.is_empty() { if let Some(kind) = return_type.def_id(cache).map(|did| tcx.def_kind(did).into()) { ret_types.push(TypeWithKind::from((get_index_type(return_type, vec![]), kind))); diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index fd52d93cb382b..7c6559bf01d70 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -4,6 +4,7 @@ use super::Pass; use crate::clean::*; use crate::core::DocContext; +use crate::formats::cache::Cache; use crate::visit::DocVisitor; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; @@ -57,14 +58,14 @@ crate fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) -> Crate } }); - let mut cleaner = BadImplStripper { prims, items: crate_items }; + let mut cleaner = BadImplStripper { prims, items: crate_items, cache: &cx.cache }; let mut type_did_to_deref_target: FxHashMap = FxHashMap::default(); // Follow all `Deref` targets of included items and recursively add them as valid fn add_deref_target( cx: &DocContext<'_>, map: &FxHashMap, - cleaner: &mut BadImplStripper, + cleaner: &mut BadImplStripper<'_>, type_did: DefId, ) { if let Some(target) = map.get(&type_did) { @@ -204,19 +205,20 @@ impl DocVisitor for ItemCollector { } } -struct BadImplStripper { +struct BadImplStripper<'a> { prims: FxHashSet, items: FxHashSet, + crate cache: &'a Cache, } -impl BadImplStripper { +impl<'a> BadImplStripper<'a> { fn keep_impl(&self, ty: &Type, is_deref: bool) -> bool { if let Generic(_) = ty { // keep impls made on generics true } else if let Some(prim) = ty.primitive_type() { self.prims.contains(&prim) - } else if let Some(did) = ty.def_id(&cx.cache) { + } else if let Some(did) = ty.def_id(self.cache) { is_deref || self.keep_impl_with_def_id(did.into()) } else { false diff --git a/src/librustdoc/passes/strip_hidden.rs b/src/librustdoc/passes/strip_hidden.rs index e63534659add7..e7a99ee7bfd84 100644 --- a/src/librustdoc/passes/strip_hidden.rs +++ b/src/librustdoc/passes/strip_hidden.rs @@ -15,7 +15,7 @@ crate const STRIP_HIDDEN: Pass = Pass { }; /// Strip items marked `#[doc(hidden)]` -crate fn strip_hidden(krate: clean::Crate, _: &mut DocContext<'_>) -> clean::Crate { +crate fn strip_hidden(krate: clean::Crate, cx: &mut DocContext<'_>) -> clean::Crate { let mut retained = ItemIdSet::default(); // strip all #[doc(hidden)] items @@ -25,7 +25,7 @@ crate fn strip_hidden(krate: clean::Crate, _: &mut DocContext<'_>) -> clean::Cra }; // strip all impls referencing stripped items - let mut stripper = ImplStripper { retained: &retained }; + let mut stripper = ImplStripper { retained: &retained, cache: &cx.cache }; stripper.fold_crate(krate) } diff --git a/src/librustdoc/passes/strip_private.rs b/src/librustdoc/passes/strip_private.rs index c6b5bec4692dc..ef7e768a51149 100644 --- a/src/librustdoc/passes/strip_private.rs +++ b/src/librustdoc/passes/strip_private.rs @@ -29,6 +29,6 @@ crate fn strip_private(mut krate: clean::Crate, cx: &mut DocContext<'_>) -> clea } // strip all impls referencing private items - let mut stripper = ImplStripper { retained: &retained }; + let mut stripper = ImplStripper { retained: &retained, cache: &cx.cache }; stripper.fold_crate(krate) } diff --git a/src/librustdoc/passes/stripper.rs b/src/librustdoc/passes/stripper.rs index ed0b2c0a5d725..4701d7f3e143a 100644 --- a/src/librustdoc/passes/stripper.rs +++ b/src/librustdoc/passes/stripper.rs @@ -5,6 +5,7 @@ use std::mem; use crate::clean::{self, Item, ItemIdSet}; use crate::fold::{strip_item, DocFolder}; +use crate::formats::cache::Cache; crate struct Stripper<'a> { crate retained: &'a mut ItemIdSet, From be9d6335f74ed6e23ac0b34a3603410ab3abca31 Mon Sep 17 00:00:00 2001 From: zredb Date: Thu, 13 Jan 2022 14:05:52 +0800 Subject: [PATCH 3/6] fix #90187: Replace all def_id_no_primitives with def_id --- src/librustdoc/formats/cache.rs | 2 +- src/librustdoc/html/render/search_index.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index c9aa326373906..53159709586c6 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -25,7 +25,7 @@ use crate::html::render::IndexItem; /// to be a fairly large and expensive structure to clone. Instead this adheres /// to `Send` so it may be stored in an `Arc` instance and shared among the various /// rendering threads. -#[derive(Default, Debug)] +#[derive(Default)] crate struct Cache { /// Maps a type ID to all known implementations for that type. This is only /// recognized for intra-crate [`clean::Type::Path`]s, and is used to print diff --git a/src/librustdoc/html/render/search_index.rs b/src/librustdoc/html/render/search_index.rs index fa965ded17698..af14efe8aa0ff 100644 --- a/src/librustdoc/html/render/search_index.rs +++ b/src/librustdoc/html/render/search_index.rs @@ -241,7 +241,7 @@ fn get_index_type_name(clean_type: &clean::Type) -> Option { /// /// Important note: It goes through generics recursively. So if you have /// `T: Option>`, it'll go into `Option` and then into `Result`. -#[instrument(level = "trace", skip(tcx, res))] +#[instrument(level = "trace", skip(tcx, res, cache))] fn add_generics_and_bounds_as_types<'tcx>( generics: &Generics, arg: &Type, From 16679a616187b6f14b308aba5b1dcef8887b231f Mon Sep 17 00:00:00 2001 From: zredb Date: Sat, 15 Jan 2022 17:00:59 +0800 Subject: [PATCH 4/6] fix #90187 remove the definition of def_id_no_primitives and change; a missing use was modified; narrow the Cache accessibility of BadImplStripper; --- src/librustdoc/clean/types.rs | 14 +------------- src/librustdoc/passes/collect_trait_impls.rs | 2 +- src/librustdoc/passes/stripper.rs | 2 +- 3 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 125afb2192c44..bb8dc7827d62f 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1566,23 +1566,11 @@ impl Type { /// Use this method to get the [DefId] of a [clean] AST node, including [PrimitiveType]s. /// - /// See [`Self::def_id_no_primitives`] for more. - /// /// [clean]: crate::clean + /// crate fn def_id(&self, cache: &Cache) -> Option { self.inner_def_id(Some(cache)) } - - /// Use this method to get the [`DefId`] of a [`clean`] AST node. - /// This will return [`None`] when called on a primitive [`clean::Type`]. - /// Use [`Self::def_id`] if you want to include primitives. - /// - /// [`clean`]: crate::clean - /// [`clean::Type`]: crate::clean::Type - // FIXME: get rid of this function and always use `def_id` - crate fn def_id_no_primitives(&self) -> Option { - self.inner_def_id(None) - } } /// A primitive (aka, builtin) type. diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index 7c6559bf01d70..53280b3df138b 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -208,7 +208,7 @@ impl DocVisitor for ItemCollector { struct BadImplStripper<'a> { prims: FxHashSet, items: FxHashSet, - crate cache: &'a Cache, + cache: &'a Cache, } impl<'a> BadImplStripper<'a> { diff --git a/src/librustdoc/passes/stripper.rs b/src/librustdoc/passes/stripper.rs index 4701d7f3e143a..717dc078b343c 100644 --- a/src/librustdoc/passes/stripper.rs +++ b/src/librustdoc/passes/stripper.rs @@ -144,7 +144,7 @@ impl<'a> DocFolder for ImplStripper<'a> { } if let Some(generics) = imp.trait_.as_ref().and_then(|t| t.generics()) { for typaram in generics { - if let Some(did) = typaram.def_id_no_primitives() { + if let Some(did) = typaram.def_id(self.cache) { if did.is_local() && !self.retained.contains(&did.into()) { debug!( "ImplStripper: stripped item in trait's generics; removing impl" From 0a95cefb62cc39b1e27ac8eecfe13369439330ca Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 17 Jan 2022 14:50:48 +0100 Subject: [PATCH 5/6] Correctly handle pure generics --- src/librustdoc/html/render/search_index.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/html/render/search_index.rs b/src/librustdoc/html/render/search_index.rs index af14efe8aa0ff..0ee67467c383b 100644 --- a/src/librustdoc/html/render/search_index.rs +++ b/src/librustdoc/html/render/search_index.rs @@ -3,7 +3,7 @@ use std::collections::BTreeMap; use rustc_data_structures::fx::FxHashMap; use rustc_middle::ty::TyCtxt; -use rustc_span::symbol::Symbol; +use rustc_span::symbol::{kw, Symbol}; use serde::ser::{Serialize, SerializeStruct, Serializer}; use crate::clean; @@ -220,7 +220,8 @@ fn get_index_type_name(clean_type: &clean::Type) -> Option { let path = &bounds[0].trait_; Some(path.segments.last().unwrap().name) } - clean::Generic(s) => Some(s), + // We return an empty name because we don't care about the generic name itself. + clean::Generic(_) => Some(kw::Empty), clean::Primitive(ref p) => Some(p.as_sym()), clean::BorrowedRef { ref type_, .. } => get_index_type_name(type_), clean::BareFunction(_) @@ -258,9 +259,10 @@ fn add_generics_and_bounds_as_types<'tcx>( cache: &Cache, ) { let is_full_generic = ty.is_full_generic(); + let generics_empty = generics.is_empty(); if is_full_generic { - if generics.is_empty() { + if generics_empty { // This is a type parameter with no trait bounds (for example: `T` in // `fn f(p: T)`, so not useful for the rustdoc search because we would end up // with an empty type with an empty name. Let's just discard it. @@ -307,7 +309,7 @@ fn add_generics_and_bounds_as_types<'tcx>( } } let mut index_ty = get_index_type(&ty, generics); - if index_ty.name.as_ref().map(|s| s.is_empty()).unwrap_or(true) { + if index_ty.name.as_ref().map(|s| s.is_empty() && generics_empty).unwrap_or(true) { return; } if is_full_generic { From 5cc32e7158b582324f7c7697f845b1c0cb5bbe44 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 28 Jan 2022 10:06:41 +0100 Subject: [PATCH 6/6] Remove extra empty doc line --- src/librustdoc/clean/types.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index bb8dc7827d62f..c0dc646ff172f 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1567,7 +1567,6 @@ impl Type { /// Use this method to get the [DefId] of a [clean] AST node, including [PrimitiveType]s. /// /// [clean]: crate::clean - /// crate fn def_id(&self, cache: &Cache) -> Option { self.inner_def_id(Some(cache)) }