From e4ac499b7e5161178a8a38f8ee0390c8b4b95dfa Mon Sep 17 00:00:00 2001 From: Camelid Date: Fri, 19 Feb 2021 14:27:30 -0800 Subject: [PATCH] Remove many RefCells from DocContext I left some of them so this change doesn't balloon in size and because removing the RefCell in `DocContext.resolver` would require compiler changes. Thanks to `@jyn514` for making this a lot easier with #82020! --- src/librustdoc/clean/auto_trait.rs | 2 +- src/librustdoc/clean/blanket_impl.rs | 6 ++-- src/librustdoc/clean/inline.rs | 21 ++++++----- src/librustdoc/clean/mod.rs | 14 ++++---- src/librustdoc/clean/utils.rs | 13 ++++--- src/librustdoc/core.rs | 35 +++++++++++-------- .../passes/calculate_doc_coverage.rs | 2 +- src/librustdoc/passes/collect_trait_impls.rs | 5 ++- src/librustdoc/passes/doc_test_lints.rs | 3 +- src/librustdoc/passes/strip_private.rs | 2 +- src/librustdoc/visit_ast.rs | 11 ++---- src/librustdoc/visit_lib.rs | 2 +- 12 files changed, 56 insertions(+), 60 deletions(-) diff --git a/src/librustdoc/clean/auto_trait.rs b/src/librustdoc/clean/auto_trait.rs index 711b0298565d7..a24cb0a0f93a1 100644 --- a/src/librustdoc/clean/auto_trait.rs +++ b/src/librustdoc/clean/auto_trait.rs @@ -41,7 +41,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { ) -> Option { let tcx = self.cx.tcx; let trait_ref = ty::TraitRef { def_id: trait_def_id, substs: tcx.mk_substs_trait(ty, &[]) }; - if !self.cx.generated_synthetics.borrow_mut().insert((ty, trait_def_id)) { + if !self.cx.generated_synthetics.insert((ty, trait_def_id)) { debug!("get_auto_trait_impl_for({:?}): already generated, aborting", trait_ref); return None; } diff --git a/src/librustdoc/clean/blanket_impl.rs b/src/librustdoc/clean/blanket_impl.rs index a9d19a725c44f..94b82037e75e9 100644 --- a/src/librustdoc/clean/blanket_impl.rs +++ b/src/librustdoc/clean/blanket_impl.rs @@ -22,8 +22,8 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> { debug!("get_blanket_impls({:?})", ty); let mut impls = Vec::new(); for &trait_def_id in self.cx.tcx.all_traits(LOCAL_CRATE).iter() { - if !self.cx.renderinfo.borrow().access_levels.is_public(trait_def_id) - || self.cx.generated_synthetics.borrow_mut().get(&(ty, trait_def_id)).is_some() + if !self.cx.renderinfo.access_levels.is_public(trait_def_id) + || self.cx.generated_synthetics.get(&(ty, trait_def_id)).is_some() { continue; } @@ -94,7 +94,7 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> { return; } - self.cx.generated_synthetics.borrow_mut().insert((ty, trait_def_id)); + self.cx.generated_synthetics.insert((ty, trait_def_id)); let provided_trait_methods = self .cx .tcx diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index fded0499ba6a8..ea75d1614bd80 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -122,7 +122,7 @@ crate fn try_inline( let target_attrs = load_attrs(cx, did); let attrs = box merge_attrs(cx, Some(parent_module), target_attrs, attrs_clone); - cx.renderinfo.borrow_mut().inlined.insert(did); + cx.renderinfo.inlined.insert(did); let what_rustc_thinks = clean::Item::from_def_id_and_parts(did, Some(name), kind, cx); ret.push(clean::Item { attrs, ..what_rustc_thinks }); Some(ret) @@ -156,7 +156,7 @@ crate fn load_attrs<'hir>(cx: &DocContext<'hir>, did: DefId) -> Attrs<'hir> { /// /// These names are used later on by HTML rendering to generate things like /// source links back to the original item. -crate fn record_extern_fqn(cx: &DocContext<'_>, did: DefId, kind: clean::TypeKind) { +crate fn record_extern_fqn(cx: &mut DocContext<'_>, did: DefId, kind: clean::TypeKind) { let crate_name = cx.tcx.crate_name(did.krate).to_string(); let relative = cx.tcx.def_path(did).data.into_iter().filter_map(|elem| { @@ -181,9 +181,9 @@ crate fn record_extern_fqn(cx: &DocContext<'_>, did: DefId, kind: clean::TypeKin }; if did.is_local() { - cx.renderinfo.borrow_mut().exact_paths.insert(did, fqn); + cx.renderinfo.exact_paths.insert(did, fqn); } else { - cx.renderinfo.borrow_mut().external_paths.insert(did, (fqn, kind)); + cx.renderinfo.external_paths.insert(did, (fqn, kind)); } } @@ -317,7 +317,7 @@ crate fn build_impl( attrs: Option>, ret: &mut Vec, ) { - if !cx.renderinfo.borrow_mut().inlined.insert(did) { + if !cx.renderinfo.inlined.insert(did) { return; } @@ -329,7 +329,7 @@ crate fn build_impl( if !did.is_local() { if let Some(traitref) = associated_trait { let did = traitref.def_id; - if !cx.renderinfo.borrow().access_levels.is_public(did) { + if !cx.renderinfo.access_levels.is_public(did) { return; } @@ -361,7 +361,7 @@ crate fn build_impl( // reachable in rustdoc generated documentation if !did.is_local() { if let Some(did) = for_.def_id() { - if !cx.renderinfo.borrow().access_levels.is_public(did) { + if !cx.renderinfo.access_levels.is_public(did) { return; } @@ -613,20 +613,19 @@ crate fn record_extern_trait(cx: &mut DocContext<'_>, did: DefId) { } { - if cx.external_traits.borrow().contains_key(&did) - || cx.active_extern_traits.borrow().contains(&did) + if cx.external_traits.borrow().contains_key(&did) || cx.active_extern_traits.contains(&did) { return; } } { - cx.active_extern_traits.borrow_mut().insert(did); + cx.active_extern_traits.insert(did); } debug!("record_extern_trait: {:?}", did); let trait_ = build_external_trait(cx, did); cx.external_traits.borrow_mut().insert(did, trait_); - cx.active_extern_traits.borrow_mut().remove(&did); + cx.active_extern_traits.remove(&did); } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 98e1299df2f26..b6e7046210596 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -357,7 +357,7 @@ impl Clean for hir::Lifetime { | rl::Region::LateBound(_, node_id, _) | rl::Region::Free(_, node_id), ) => { - if let Some(lt) = cx.lt_substs.borrow().get(&node_id).cloned() { + if let Some(lt) = cx.lt_substs.get(&node_id).cloned() { return lt; } } @@ -644,7 +644,7 @@ impl Clean for hir::Generics<'_> { match param.kind { GenericParamDefKind::Lifetime => unreachable!(), GenericParamDefKind::Type { did, ref bounds, .. } => { - cx.impl_trait_bounds.borrow_mut().insert(did.into(), bounds.clone()); + cx.impl_trait_bounds.insert(did.into(), bounds.clone()); } GenericParamDefKind::Const { .. } => unreachable!(), } @@ -803,7 +803,7 @@ impl<'a, 'tcx> Clean for (&'a ty::Generics, ty::GenericPredicates<'tcx unreachable!(); } - cx.impl_trait_bounds.borrow_mut().insert(param, bounds); + cx.impl_trait_bounds.insert(param, bounds); } // Now that `cx.impl_trait_bounds` is populated, we can process @@ -1291,10 +1291,10 @@ fn clean_qpath(hir_ty: &hir::Ty<'_>, cx: &mut DocContext<'_>) -> Type { match qpath { hir::QPath::Resolved(None, ref path) => { if let Res::Def(DefKind::TyParam, did) = path.res { - if let Some(new_ty) = cx.ty_substs.borrow().get(&did).cloned() { + if let Some(new_ty) = cx.ty_substs.get(&did).cloned() { return new_ty; } - if let Some(bounds) = cx.impl_trait_bounds.borrow_mut().remove(&did.into()) { + if let Some(bounds) = cx.impl_trait_bounds.remove(&did.into()) { return ImplTrait(bounds); } } @@ -1304,7 +1304,7 @@ fn clean_qpath(hir_ty: &hir::Ty<'_>, cx: &mut DocContext<'_>) -> Type { // Substitute private type aliases if let Some(def_id) = def_id.as_local() { let hir_id = cx.tcx.hir().local_def_id_to_hir_id(def_id); - if !cx.renderinfo.borrow().access_levels.is_exported(def_id.to_def_id()) { + if !cx.renderinfo.access_levels.is_exported(def_id.to_def_id()) { alias = Some(&cx.tcx.hir().expect_item(hir_id).kind); } } @@ -1651,7 +1651,7 @@ impl<'tcx> Clean for Ty<'tcx> { ty::Projection(ref data) => data.clean(cx), ty::Param(ref p) => { - if let Some(bounds) = cx.impl_trait_bounds.borrow_mut().remove(&p.index.into()) { + if let Some(bounds) = cx.impl_trait_bounds.remove(&p.index.into()) { ImplTrait(bounds) } else { Generic(p.name) diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index c7bfd363a129b..d2eee49f0c968 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -23,10 +23,9 @@ crate fn krate(mut cx: &mut DocContext<'_>) -> Crate { let krate = cx.tcx.hir().krate(); let module = crate::visit_ast::RustdocVisitor::new(&mut cx).visit(krate); - let mut r = cx.renderinfo.get_mut(); - r.deref_trait_did = cx.tcx.lang_items().deref_trait(); - r.deref_mut_trait_did = cx.tcx.lang_items().deref_mut_trait(); - r.owned_box_did = cx.tcx.lang_items().owned_box(); + cx.renderinfo.deref_trait_did = cx.tcx.lang_items().deref_trait(); + cx.renderinfo.deref_mut_trait_did = cx.tcx.lang_items().deref_mut_trait(); + cx.renderinfo.owned_box_did = cx.tcx.lang_items().owned_box(); let mut externs = Vec::new(); for &cnum in cx.tcx.crates().iter() { @@ -494,10 +493,10 @@ crate fn enter_impl_trait(cx: &mut DocContext<'_>, f: F) -> R where F: FnOnce(&mut DocContext<'_>) -> R, { - let old_bounds = mem::take(&mut *cx.impl_trait_bounds.get_mut()); + let old_bounds = mem::take(&mut cx.impl_trait_bounds); let r = f(cx); - assert!(cx.impl_trait_bounds.borrow().is_empty()); - *cx.impl_trait_bounds.get_mut() = old_bounds; + assert!(cx.impl_trait_bounds.is_empty()); + cx.impl_trait_bounds = old_bounds; r } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index dbf202a732108..8fceb00eeae51 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -42,32 +42,37 @@ crate type ExternalPaths = FxHashMap, clean::TypeKind)>; crate struct DocContext<'tcx> { crate tcx: TyCtxt<'tcx>, + /// Name resolver. Used for intra-doc links. + /// + /// The `Rc>` wrapping is needed because that is what's returned by + /// [`Queries::expansion()`]. + // FIXME: see if we can get rid of this RefCell somehow crate resolver: Rc>, /// Used for normalization. /// /// Most of this logic is copied from rustc_lint::late. crate param_env: ParamEnv<'tcx>, /// Later on moved into `cache` - crate renderinfo: RefCell, + crate renderinfo: RenderInfo, /// Later on moved through `clean::Crate` into `cache` crate external_traits: Rc>>, /// Used while populating `external_traits` to ensure we don't process the same trait twice at /// the same time. - crate active_extern_traits: RefCell>, + crate active_extern_traits: FxHashSet, // The current set of type and lifetime substitutions, // for expanding type aliases at the HIR level: /// Table `DefId` of type parameter -> substituted type - crate ty_substs: RefCell>, + crate ty_substs: FxHashMap, /// Table `DefId` of lifetime parameter -> substituted lifetime - crate lt_substs: RefCell>, + crate lt_substs: FxHashMap, /// Table `DefId` of const parameter -> substituted const - crate ct_substs: RefCell>, + crate ct_substs: FxHashMap, /// Table synthetic type parameter for `impl Trait` in argument position -> bounds - crate impl_trait_bounds: RefCell>>, + crate impl_trait_bounds: FxHashMap>, crate fake_def_ids: FxHashMap, /// Auto-trait or blanket impls processed so far, as `(self_ty, trait_def_id)`. // FIXME(eddyb) make this a `ty::TraitRef<'tcx>` set. - crate generated_synthetics: RefCell, DefId)>>, + crate generated_synthetics: FxHashSet<(Ty<'tcx>, DefId)>, crate auto_traits: Vec, /// The options given to rustdoc that could be relevant to a pass. crate render_options: RenderOptions, @@ -112,14 +117,14 @@ impl<'tcx> DocContext<'tcx> { F: FnOnce(&mut Self) -> R, { let (old_tys, old_lts, old_cts) = ( - mem::replace(&mut *self.ty_substs.get_mut(), ty_substs), - mem::replace(&mut *self.lt_substs.get_mut(), lt_substs), - mem::replace(&mut *self.ct_substs.get_mut(), ct_substs), + mem::replace(&mut self.ty_substs, ty_substs), + mem::replace(&mut self.lt_substs, lt_substs), + mem::replace(&mut self.ct_substs, ct_substs), ); let r = f(self); - *self.ty_substs.get_mut() = old_tys; - *self.lt_substs.get_mut() = old_lts; - *self.ct_substs.get_mut() = old_cts; + self.ty_substs = old_tys; + self.lt_substs = old_lts; + self.ct_substs = old_cts; r } @@ -509,7 +514,7 @@ crate fn run_global_ctxt( param_env: ParamEnv::empty(), external_traits: Default::default(), active_extern_traits: Default::default(), - renderinfo: RefCell::new(renderinfo), + renderinfo, ty_substs: Default::default(), lt_substs: Default::default(), ct_substs: Default::default(), @@ -642,7 +647,7 @@ crate fn run_global_ctxt( // The main crate doc comments are always collapsed. krate.collapsed = true; - (krate, ctxt.renderinfo.into_inner(), ctxt.render_options) + (krate, ctxt.renderinfo, ctxt.render_options) } /// Due to , diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index 542cf6d2c275a..c3365b844ecb8 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -127,7 +127,7 @@ impl<'a, 'b> CoverageCalculator<'a, 'b> { } fn print_results(&self) { - let output_format = self.ctx.renderinfo.borrow().output_format; + let output_format = self.ctx.renderinfo.output_format; if output_format.is_json() { println!("{}", self.to_json()); return; diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index 0b6d81d1b447e..0271a5b78a7ef 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -48,11 +48,10 @@ crate fn collect_trait_impls(krate: Crate, cx: &mut DocContext<'_>) -> Crate { if !cx.tcx.get_attrs(def_id).lists(sym::doc).has_word(sym::hidden) { let self_ty = cx.tcx.type_of(def_id); let impls = get_auto_trait_and_blanket_impls(cx, self_ty, def_id); - let mut renderinfo = cx.renderinfo.borrow_mut(); - new_items.extend(impls.filter(|i| renderinfo.inlined.insert(i.def_id))); + new_items.extend(impls.filter(|i| cx.renderinfo.inlined.insert(i.def_id))); } - }) + }); } } diff --git a/src/librustdoc/passes/doc_test_lints.rs b/src/librustdoc/passes/doc_test_lints.rs index 042a895d2fa2f..e8e1bead84fb6 100644 --- a/src/librustdoc/passes/doc_test_lints.rs +++ b/src/librustdoc/passes/doc_test_lints.rs @@ -97,8 +97,7 @@ crate fn look_for_tests<'tcx>(cx: &DocContext<'tcx>, dox: &str, item: &Item) { |lint| lint.build("missing code example in this documentation").emit(), ); } - } else if tests.found_tests > 0 && !cx.renderinfo.borrow().access_levels.is_public(item.def_id) - { + } else if tests.found_tests > 0 && !cx.renderinfo.access_levels.is_public(item.def_id) { cx.tcx.struct_span_lint_hir( lint::builtin::PRIVATE_DOC_TESTS, hir_id, diff --git a/src/librustdoc/passes/strip_private.rs b/src/librustdoc/passes/strip_private.rs index c0bb05af3edb5..f83eab6799ee6 100644 --- a/src/librustdoc/passes/strip_private.rs +++ b/src/librustdoc/passes/strip_private.rs @@ -17,7 +17,7 @@ crate const STRIP_PRIVATE: Pass = Pass { crate fn strip_private(mut krate: clean::Crate, cx: &mut DocContext<'_>) -> clean::Crate { // This stripper collects all *retained* nodes. let mut retained = DefIdSet::default(); - let access_levels = cx.renderinfo.borrow().access_levels.clone(); + let access_levels = cx.renderinfo.access_levels.clone(); // strip all private items { diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index e92ea55caa737..4d42c181d8cf2 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -113,7 +113,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { assert_eq!(cur_mod_def_id, macro_parent_def_id); cur_mod.macros.push((def, None)); } - self.cx.renderinfo.get_mut().exact_paths = self.exact_paths; + self.cx.renderinfo.exact_paths = self.exact_paths; top_level_module } @@ -199,12 +199,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { } else { // All items need to be handled here in case someone wishes to link // to them with intra-doc links - self.cx - .renderinfo - .get_mut() - .access_levels - .map - .insert(did, AccessLevel::Public); + self.cx.renderinfo.access_levels.map.insert(did, AccessLevel::Public); } } } @@ -216,7 +211,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { None => return false, }; - let is_private = !self.cx.renderinfo.borrow().access_levels.is_public(res_did); + let is_private = !self.cx.renderinfo.access_levels.is_public(res_did); let is_hidden = inherits_doc_hidden(self.cx, res_hir_id); // Only inline if requested or if the item would otherwise be stripped. diff --git a/src/librustdoc/visit_lib.rs b/src/librustdoc/visit_lib.rs index 0bf22562eaede..daed5bd107db1 100644 --- a/src/librustdoc/visit_lib.rs +++ b/src/librustdoc/visit_lib.rs @@ -25,7 +25,7 @@ impl<'a, 'tcx> LibEmbargoVisitor<'a, 'tcx> { crate fn new(cx: &'a mut crate::core::DocContext<'tcx>) -> LibEmbargoVisitor<'a, 'tcx> { LibEmbargoVisitor { tcx: cx.tcx, - access_levels: &mut cx.renderinfo.get_mut().access_levels, + access_levels: &mut cx.renderinfo.access_levels, prev_level: Some(AccessLevel::Public), visited_mods: FxHashSet::default(), }