From 175474549ca2add0dec0bea1cca2ce0e32c24dc6 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 18 Oct 2022 15:49:04 +0400 Subject: [PATCH] rustdoc: Eliminate uses of `EarlyDocLinkResolver::all_traits` --- .../src/rmeta/decoder/cstore_impl.rs | 5 - src/librustdoc/clean/blanket_impl.rs | 196 +++++++++--------- src/librustdoc/core.rs | 17 +- .../passes/collect_intra_doc_links/early.rs | 32 ++- 4 files changed, 117 insertions(+), 133 deletions(-) diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index c4dff8b3f48ed..8fee844c534ba 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -597,11 +597,6 @@ impl CStore { self.get_crate_data(cnum).get_proc_macro_quoted_span(id, sess) } - /// Decodes all traits in the crate (for rustdoc). - pub fn traits_in_crate_untracked(&self, cnum: CrateNum) -> impl Iterator + '_ { - self.get_crate_data(cnum).get_traits() - } - /// Decodes all trait impls in the crate (for rustdoc). pub fn trait_impls_in_crate_untracked( &self, diff --git a/src/librustdoc/clean/blanket_impl.rs b/src/librustdoc/clean/blanket_impl.rs index 95061ae61e3f7..7c59e785752dc 100644 --- a/src/librustdoc/clean/blanket_impl.rs +++ b/src/librustdoc/clean/blanket_impl.rs @@ -13,116 +13,124 @@ pub(crate) struct BlanketImplFinder<'a, 'tcx> { impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> { pub(crate) fn get_blanket_impls(&mut self, item_def_id: DefId) -> Vec { - let param_env = self.cx.tcx.param_env(item_def_id); - let ty = self.cx.tcx.bound_type_of(item_def_id); + let cx = &mut self.cx; + let param_env = cx.tcx.param_env(item_def_id); + let ty = cx.tcx.bound_type_of(item_def_id); trace!("get_blanket_impls({:?})", ty); let mut impls = Vec::new(); - self.cx.with_all_traits(|cx, all_traits| { - for &trait_def_id in all_traits { - if !cx.cache.access_levels.is_public(trait_def_id) - || cx.generated_synthetics.get(&(ty.0, trait_def_id)).is_some() - { + for trait_def_id in cx.tcx.all_traits() { + if !cx.cache.access_levels.is_public(trait_def_id) + || cx.generated_synthetics.get(&(ty.0, trait_def_id)).is_some() + { + continue; + } + // NOTE: doesn't use `for_each_relevant_impl` to avoid looking at anything besides blanket impls + let trait_impls = cx.tcx.trait_impls_of(trait_def_id); + 'blanket_impls: for &impl_def_id in trait_impls.blanket_impls() { + trace!( + "get_blanket_impls: Considering impl for trait '{:?}' {:?}", + trait_def_id, + impl_def_id + ); + let trait_ref = cx.tcx.bound_impl_trait_ref(impl_def_id).unwrap(); + if !matches!(trait_ref.0.self_ty().kind(), ty::Param(_)) { continue; } - // NOTE: doesn't use `for_each_relevant_impl` to avoid looking at anything besides blanket impls - let trait_impls = cx.tcx.trait_impls_of(trait_def_id); - 'blanket_impls: for &impl_def_id in trait_impls.blanket_impls() { - trace!( - "get_blanket_impls: Considering impl for trait '{:?}' {:?}", - trait_def_id, - impl_def_id - ); - let trait_ref = cx.tcx.bound_impl_trait_ref(impl_def_id).unwrap(); - if !matches!(trait_ref.0.self_ty().kind(), ty::Param(_)) { - continue; - } - let infcx = cx.tcx.infer_ctxt().build(); - let substs = infcx.fresh_substs_for_item(DUMMY_SP, item_def_id); - let impl_ty = ty.subst(infcx.tcx, substs); - let param_env = EarlyBinder(param_env).subst(infcx.tcx, substs); + let infcx = cx.tcx.infer_ctxt().build(); + let substs = infcx.fresh_substs_for_item(DUMMY_SP, item_def_id); + let impl_ty = ty.subst(infcx.tcx, substs); + let param_env = EarlyBinder(param_env).subst(infcx.tcx, substs); - let impl_substs = infcx.fresh_substs_for_item(DUMMY_SP, impl_def_id); - let impl_trait_ref = trait_ref.subst(infcx.tcx, impl_substs); + let impl_substs = infcx.fresh_substs_for_item(DUMMY_SP, impl_def_id); + let impl_trait_ref = trait_ref.subst(infcx.tcx, impl_substs); - // Require the type the impl is implemented on to match - // our type, and ignore the impl if there was a mismatch. - let cause = traits::ObligationCause::dummy(); - let Ok(eq_result) = infcx.at(&cause, param_env).eq(impl_trait_ref.self_ty(), impl_ty) else { + // Require the type the impl is implemented on to match + // our type, and ignore the impl if there was a mismatch. + let cause = traits::ObligationCause::dummy(); + let Ok(eq_result) = infcx.at(&cause, param_env).eq(impl_trait_ref.self_ty(), impl_ty) else { continue }; - let InferOk { value: (), obligations } = eq_result; - // FIXME(eddyb) ignoring `obligations` might cause false positives. - drop(obligations); + let InferOk { value: (), obligations } = eq_result; + // FIXME(eddyb) ignoring `obligations` might cause false positives. + drop(obligations); - trace!( - "invoking predicate_may_hold: param_env={:?}, impl_trait_ref={:?}, impl_ty={:?}", + trace!( + "invoking predicate_may_hold: param_env={:?}, impl_trait_ref={:?}, impl_ty={:?}", + param_env, + impl_trait_ref, + impl_ty + ); + let predicates = cx + .tcx + .predicates_of(impl_def_id) + .instantiate(cx.tcx, impl_substs) + .predicates + .into_iter() + .chain(Some( + ty::Binder::dummy(impl_trait_ref) + .to_poly_trait_predicate() + .map_bound(ty::PredicateKind::Trait) + .to_predicate(infcx.tcx), + )); + for predicate in predicates { + debug!("testing predicate {:?}", predicate); + let obligation = traits::Obligation::new( + traits::ObligationCause::dummy(), param_env, - impl_trait_ref, - impl_ty + predicate, ); - let predicates = cx - .tcx - .predicates_of(impl_def_id) - .instantiate(cx.tcx, impl_substs) - .predicates - .into_iter() - .chain(Some( - ty::Binder::dummy(impl_trait_ref) - .to_poly_trait_predicate() - .map_bound(ty::PredicateKind::Trait) - .to_predicate(infcx.tcx), - )); - for predicate in predicates { - debug!("testing predicate {:?}", predicate); - let obligation = traits::Obligation::new( - traits::ObligationCause::dummy(), - param_env, - predicate, - ); - match infcx.evaluate_obligation(&obligation) { - Ok(eval_result) if eval_result.may_apply() => {} - Err(traits::OverflowError::Canonical) => {} - Err(traits::OverflowError::ErrorReporting) => {} - _ => continue 'blanket_impls, - } + match infcx.evaluate_obligation(&obligation) { + Ok(eval_result) if eval_result.may_apply() => {} + Err(traits::OverflowError::Canonical) => {} + Err(traits::OverflowError::ErrorReporting) => {} + _ => continue 'blanket_impls, } - debug!( - "get_blanket_impls: found applicable impl for trait_ref={:?}, ty={:?}", - trait_ref, ty - ); + } + debug!( + "get_blanket_impls: found applicable impl for trait_ref={:?}, ty={:?}", + trait_ref, ty + ); - cx.generated_synthetics.insert((ty.0, trait_def_id)); + cx.generated_synthetics.insert((ty.0, trait_def_id)); - impls.push(Item { - name: None, - attrs: Default::default(), - visibility: Inherited, - item_id: ItemId::Blanket { impl_id: impl_def_id, for_: item_def_id }, - kind: Box::new(ImplItem(Box::new(Impl { - unsafety: hir::Unsafety::Normal, - generics: clean_ty_generics( - cx, - cx.tcx.generics_of(impl_def_id), - cx.tcx.explicit_predicates_of(impl_def_id), - ), - // FIXME(eddyb) compute both `trait_` and `for_` from - // the post-inference `trait_ref`, as it's more accurate. - trait_: Some(clean_trait_ref_with_bindings(cx, trait_ref.0, ThinVec::new())), - for_: clean_middle_ty(ty.0, cx, None), - items: cx.tcx - .associated_items(impl_def_id) - .in_definition_order() - .map(|x| clean_middle_assoc_item(x, cx)) - .collect::>(), - polarity: ty::ImplPolarity::Positive, - kind: ImplKind::Blanket(Box::new(clean_middle_ty(trait_ref.0.self_ty(), cx, None))), - }))), - cfg: None, - }); - } + impls.push(Item { + name: None, + attrs: Default::default(), + visibility: Inherited, + item_id: ItemId::Blanket { impl_id: impl_def_id, for_: item_def_id }, + kind: Box::new(ImplItem(Box::new(Impl { + unsafety: hir::Unsafety::Normal, + generics: clean_ty_generics( + cx, + cx.tcx.generics_of(impl_def_id), + cx.tcx.explicit_predicates_of(impl_def_id), + ), + // FIXME(eddyb) compute both `trait_` and `for_` from + // the post-inference `trait_ref`, as it's more accurate. + trait_: Some(clean_trait_ref_with_bindings( + cx, + trait_ref.0, + ThinVec::new(), + )), + for_: clean_middle_ty(ty.0, cx, None), + items: cx + .tcx + .associated_items(impl_def_id) + .in_definition_order() + .map(|x| clean_middle_assoc_item(x, cx)) + .collect::>(), + polarity: ty::ImplPolarity::Positive, + kind: ImplKind::Blanket(Box::new(clean_middle_ty( + trait_ref.0.self_ty(), + cx, + None, + ))), + }))), + cfg: None, + }); } - }); + } impls } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 858e939bd96e8..8232353f915b9 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -38,7 +38,6 @@ pub(crate) struct ResolverCaches { /// Traits in scope for a given module. /// See `collect_intra_doc_links::traits_implemented_by` for more details. pub(crate) traits_in_scope: DefIdMap>, - pub(crate) all_traits: Option>, pub(crate) all_trait_impls: Option>, pub(crate) all_macro_rules: FxHashMap>, } @@ -134,12 +133,6 @@ impl<'tcx> DocContext<'tcx> { } } - pub(crate) fn with_all_traits(&mut self, f: impl FnOnce(&mut Self, &[DefId])) { - let all_traits = self.resolver_caches.all_traits.take(); - f(self, all_traits.as_ref().expect("`all_traits` are already borrowed")); - self.resolver_caches.all_traits = all_traits; - } - pub(crate) fn with_all_trait_impls(&mut self, f: impl FnOnce(&mut Self, &[DefId])) { let all_trait_impls = self.resolver_caches.all_trait_impls.take(); f(self, all_trait_impls.as_ref().expect("`all_trait_impls` are already borrowed")); @@ -353,14 +346,8 @@ pub(crate) fn run_global_ctxt( }); rustc_passes::stability::check_unused_or_stable_features(tcx); - let auto_traits = resolver_caches - .all_traits - .as_ref() - .expect("`all_traits` are already borrowed") - .iter() - .copied() - .filter(|&trait_def_id| tcx.trait_is_auto(trait_def_id)) - .collect(); + let auto_traits = + tcx.all_traits().filter(|&trait_def_id| tcx.trait_is_auto(trait_def_id)).collect(); let access_levels = tcx.privacy_access_levels(()).map_id(Into::into); let mut ctxt = DocContext { diff --git a/src/librustdoc/passes/collect_intra_doc_links/early.rs b/src/librustdoc/passes/collect_intra_doc_links/early.rs index 50dc26d768cd2..d121a3e2aa4a9 100644 --- a/src/librustdoc/passes/collect_intra_doc_links/early.rs +++ b/src/librustdoc/passes/collect_intra_doc_links/early.rs @@ -37,7 +37,6 @@ pub(crate) fn early_resolve_intra_doc_links( markdown_links: Default::default(), doc_link_resolutions: Default::default(), traits_in_scope: Default::default(), - all_traits: Default::default(), all_trait_impls: Default::default(), all_macro_rules: Default::default(), document_private_items, @@ -63,7 +62,6 @@ pub(crate) fn early_resolve_intra_doc_links( markdown_links: Some(link_resolver.markdown_links), doc_link_resolutions: link_resolver.doc_link_resolutions, traits_in_scope: link_resolver.traits_in_scope, - all_traits: Some(link_resolver.all_traits), all_trait_impls: Some(link_resolver.all_trait_impls), all_macro_rules: link_resolver.all_macro_rules, } @@ -81,7 +79,6 @@ struct EarlyDocLinkResolver<'r, 'ra> { markdown_links: FxHashMap>, doc_link_resolutions: FxHashMap<(Symbol, Namespace, DefId), Option>>, traits_in_scope: DefIdMap>, - all_traits: Vec, all_trait_impls: Vec, all_macro_rules: FxHashMap>, document_private_items: bool, @@ -122,8 +119,6 @@ impl<'ra> EarlyDocLinkResolver<'_, 'ra> { loop { let crates = Vec::from_iter(self.resolver.cstore().crates_untracked()); for &cnum in &crates[start_cnum..] { - let all_traits = - Vec::from_iter(self.resolver.cstore().traits_in_crate_untracked(cnum)); let all_trait_impls = Vec::from_iter(self.resolver.cstore().trait_impls_in_crate_untracked(cnum)); let all_inherent_impls = @@ -132,20 +127,18 @@ impl<'ra> EarlyDocLinkResolver<'_, 'ra> { self.resolver.cstore().incoherent_impls_in_crate_untracked(cnum), ); - // Querying traits in scope is expensive so we try to prune the impl and traits lists - // using privacy, private traits and impls from other crates are never documented in + // Querying traits in scope is expensive so we try to prune the impl lists using + // privacy, private traits and impls from other crates are never documented in // the current crate, and links in their doc comments are not resolved. - for &def_id in &all_traits { - if self.resolver.cstore().visibility_untracked(def_id).is_public() { - self.resolve_doc_links_extern_impl(def_id, false); - } - } for &(trait_def_id, impl_def_id, simplified_self_ty) in &all_trait_impls { if self.resolver.cstore().visibility_untracked(trait_def_id).is_public() && simplified_self_ty.and_then(|ty| ty.def()).map_or(true, |ty_def_id| { self.resolver.cstore().visibility_untracked(ty_def_id).is_public() }) { + if self.visited_mods.insert(trait_def_id) { + self.resolve_doc_links_extern_impl(trait_def_id, false); + } self.resolve_doc_links_extern_impl(impl_def_id, false); } } @@ -158,7 +151,6 @@ impl<'ra> EarlyDocLinkResolver<'_, 'ra> { self.resolve_doc_links_extern_impl(impl_def_id, true); } - self.all_traits.extend(all_traits); self.all_trait_impls .extend(all_trait_impls.into_iter().map(|(_, def_id, _)| def_id)); } @@ -307,15 +299,20 @@ impl<'ra> EarlyDocLinkResolver<'_, 'ra> { { if let Some(def_id) = child.res.opt_def_id() && !def_id.is_local() { let scope_id = match child.res { - Res::Def(DefKind::Variant, ..) => self.resolver.parent(def_id), + Res::Def( + DefKind::Variant + | DefKind::AssocTy + | DefKind::AssocFn + | DefKind::AssocConst, + .., + ) => self.resolver.parent(def_id), _ => def_id, }; self.resolve_doc_links_extern_outer(def_id, scope_id); // Outer attribute scope if let Res::Def(DefKind::Mod, ..) = child.res { self.resolve_doc_links_extern_inner(def_id); // Inner attribute scope } - // `DefKind::Trait`s are processed in `process_extern_impls`. - if let Res::Def(DefKind::Mod | DefKind::Enum, ..) = child.res { + if let Res::Def(DefKind::Mod | DefKind::Enum | DefKind::Trait, ..) = child.res { self.process_module_children_or_reexports(def_id); } if let Res::Def(DefKind::Struct | DefKind::Union | DefKind::Variant, _) = @@ -357,9 +354,6 @@ impl Visitor<'_> for EarlyDocLinkResolver<'_, '_> { self.parent_scope.module = old_module; } else { match &item.kind { - ItemKind::Trait(..) => { - self.all_traits.push(self.resolver.local_def_id(item.id).to_def_id()); - } ItemKind::Impl(box ast::Impl { of_trait: Some(..), .. }) => { self.all_trait_impls.push(self.resolver.local_def_id(item.id).to_def_id()); }