From b3c4e25e1740da69686f940165dd3c9d563d54e5 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 22 Feb 2021 09:31:41 -0500 Subject: [PATCH 1/2] Don't require a `DocContext` for `report_diagnostic` This is needed for the next commit, which needs access to the `cx` from within the `decorate` closure. - Change `as_local_hir_id` to an associated function, since it only needs a `TyCtxt` - Change `source_span_for_markdown_range` to only take a `TyCtxt` --- src/librustdoc/core.rs | 6 +++--- .../passes/check_code_block_syntax.rs | 9 ++++++--- .../passes/collect_intra_doc_links.rs | 20 +++++++++---------- src/librustdoc/passes/doc_test_lints.rs | 2 +- src/librustdoc/passes/html_tags.rs | 8 ++++---- src/librustdoc/passes/mod.rs | 5 +++-- src/librustdoc/passes/non_autolinks.rs | 4 ++-- 7 files changed, 29 insertions(+), 25 deletions(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 9cb0649b314f5..5ff50e2538050 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -169,13 +169,13 @@ impl<'tcx> DocContext<'tcx> { /// Like `hir().local_def_id_to_hir_id()`, but skips calling it on fake DefIds. /// (This avoids a slice-index-out-of-bounds panic.) - crate fn as_local_hir_id(&self, def_id: DefId) -> Option { + crate fn as_local_hir_id(tcx: TyCtxt<'_>, def_id: DefId) -> Option { if MAX_DEF_IDX.with(|m| { m.borrow().get(&def_id.krate).map(|&idx| idx <= def_id.index).unwrap_or(false) }) { None } else { - def_id.as_local().map(|def_id| self.tcx.hir().local_def_id_to_hir_id(def_id)) + def_id.as_local().map(|def_id| tcx.hir().local_def_id_to_hir_id(def_id)) } } } @@ -479,7 +479,7 @@ crate fn run_global_ctxt( https://doc.rust-lang.org/nightly/rustdoc/how-to-write-documentation.html"; tcx.struct_lint_node( crate::lint::MISSING_CRATE_LEVEL_DOCS, - ctxt.as_local_hir_id(m.def_id).unwrap(), + DocContext::as_local_hir_id(tcx, m.def_id).unwrap(), |lint| { let mut diag = lint.build("no documentation found for this crate's top-level module"); diff --git a/src/librustdoc/passes/check_code_block_syntax.rs b/src/librustdoc/passes/check_code_block_syntax.rs index c85490864ec70..98886139f3088 100644 --- a/src/librustdoc/passes/check_code_block_syntax.rs +++ b/src/librustdoc/passes/check_code_block_syntax.rs @@ -48,9 +48,12 @@ impl<'a, 'tcx> SyntaxChecker<'a, 'tcx> { let buffer = buffer.borrow(); if buffer.has_errors || is_empty { - let mut diag = if let Some(sp) = - super::source_span_for_markdown_range(self.cx, &dox, &code_block.range, &item.attrs) - { + let mut diag = if let Some(sp) = super::source_span_for_markdown_range( + self.cx.tcx, + &dox, + &code_block.range, + &item.attrs, + ) { let (warning_message, suggest_using_text) = if buffer.has_errors { ("could not parse code block as Rust code", true) } else { diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 38efecb393b79..93aa069593694 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1146,7 +1146,7 @@ impl LinkCollector<'_, '_> { suggest_disambiguator(resolved, diag, path_str, dox, sp, &ori_link.range); }; report_diagnostic( - self.cx, + self.cx.tcx, BROKEN_INTRA_DOC_LINKS, &msg, &item, @@ -1220,7 +1220,7 @@ impl LinkCollector<'_, '_> { && !self.cx.tcx.features().intra_doc_pointers { let span = super::source_span_for_markdown_range( - self.cx, + self.cx.tcx, dox, &ori_link.range, &item.attrs, @@ -1674,7 +1674,7 @@ impl Suggestion { /// parameter of the callback will contain it, and the primary span of the diagnostic will be set /// to it. fn report_diagnostic( - cx: &DocContext<'_>, + tcx: TyCtxt<'_>, lint: &'static Lint, msg: &str, item: &Item, @@ -1682,7 +1682,7 @@ fn report_diagnostic( link_range: &Range, decorate: impl FnOnce(&mut DiagnosticBuilder<'_>, Option), ) { - let hir_id = match cx.as_local_hir_id(item.def_id) { + let hir_id = match DocContext::as_local_hir_id(tcx, item.def_id) { Some(hir_id) => hir_id, None => { // If non-local, no need to check anything. @@ -1694,10 +1694,10 @@ fn report_diagnostic( let attrs = &item.attrs; let sp = span_of_attrs(attrs).unwrap_or(item.source.span()); - cx.tcx.struct_span_lint_hir(lint, hir_id, sp, |lint| { + tcx.struct_span_lint_hir(lint, hir_id, sp, |lint| { let mut diag = lint.build(msg); - let span = super::source_span_for_markdown_range(cx, dox, link_range, attrs); + let span = super::source_span_for_markdown_range(tcx, dox, link_range, attrs); if let Some(sp) = span { diag.set_span(sp); @@ -1742,7 +1742,7 @@ fn resolution_failure( ) { let tcx = collector.cx.tcx; report_diagnostic( - collector.cx, + tcx, BROKEN_INTRA_DOC_LINKS, &format!("unresolved link to `{}`", path_str), item, @@ -1973,7 +1973,7 @@ fn anchor_failure( ), }; - report_diagnostic(cx, BROKEN_INTRA_DOC_LINKS, &msg, item, dox, &link_range, |diag, sp| { + report_diagnostic(cx.tcx, BROKEN_INTRA_DOC_LINKS, &msg, item, dox, &link_range, |diag, sp| { if let Some(sp) = sp { diag.span_label(sp, "contains invalid anchor"); } @@ -2013,7 +2013,7 @@ fn ambiguity_error( } } - report_diagnostic(cx, BROKEN_INTRA_DOC_LINKS, &msg, item, dox, &link_range, |diag, sp| { + report_diagnostic(cx.tcx, BROKEN_INTRA_DOC_LINKS, &msg, item, dox, &link_range, |diag, sp| { if let Some(sp) = sp { diag.span_label(sp, "ambiguous link"); } else { @@ -2066,7 +2066,7 @@ fn privacy_error(cx: &DocContext<'_>, item: &Item, path_str: &str, dox: &str, li let msg = format!("public documentation for `{}` links to private item `{}`", item_name, path_str); - report_diagnostic(cx, PRIVATE_INTRA_DOC_LINKS, &msg, item, dox, &link.range, |diag, sp| { + report_diagnostic(cx.tcx, PRIVATE_INTRA_DOC_LINKS, &msg, item, dox, &link.range, |diag, sp| { if let Some(sp) = sp { diag.span_label(sp, "this item is private"); } diff --git a/src/librustdoc/passes/doc_test_lints.rs b/src/librustdoc/passes/doc_test_lints.rs index 81104236314d9..ed6d2d59845f2 100644 --- a/src/librustdoc/passes/doc_test_lints.rs +++ b/src/librustdoc/passes/doc_test_lints.rs @@ -73,7 +73,7 @@ crate fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) -> boo } crate fn look_for_tests<'tcx>(cx: &DocContext<'tcx>, dox: &str, item: &Item) { - let hir_id = match cx.as_local_hir_id(item.def_id) { + let hir_id = match DocContext::as_local_hir_id(cx.tcx, item.def_id) { Some(hir_id) => hir_id, None => { // If non-local, no need to check anything. diff --git a/src/librustdoc/passes/html_tags.rs b/src/librustdoc/passes/html_tags.rs index 27e669aa44fc0..5fff7c8def31c 100644 --- a/src/librustdoc/passes/html_tags.rs +++ b/src/librustdoc/passes/html_tags.rs @@ -167,7 +167,8 @@ fn extract_tags( impl<'a, 'tcx> DocFolder for InvalidHtmlTagsLinter<'a, 'tcx> { fn fold_item(&mut self, item: Item) -> Option { - let hir_id = match self.cx.as_local_hir_id(item.def_id) { + let tcx = self.cx.tcx; + let hir_id = match DocContext::as_local_hir_id(tcx, item.def_id) { Some(hir_id) => hir_id, None => { // If non-local, no need to check anything. @@ -176,13 +177,12 @@ impl<'a, 'tcx> DocFolder for InvalidHtmlTagsLinter<'a, 'tcx> { }; let dox = item.attrs.collapsed_doc_value().unwrap_or_default(); if !dox.is_empty() { - let cx = &self.cx; let report_diag = |msg: &str, range: &Range| { - let sp = match super::source_span_for_markdown_range(cx, &dox, range, &item.attrs) { + let sp = match super::source_span_for_markdown_range(tcx, &dox, range, &item.attrs) { Some(sp) => sp, None => span_of_attrs(&item.attrs).unwrap_or(item.source.span()), }; - cx.tcx.struct_span_lint_hir(crate::lint::INVALID_HTML_TAGS, hir_id, sp, |lint| { + tcx.struct_span_lint_hir(crate::lint::INVALID_HTML_TAGS, hir_id, sp, |lint| { lint.build(msg).emit() }); }; diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 5813732facb6e..4c639c8496db3 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -1,6 +1,7 @@ //! Contains information about "passes", used to modify crate information during the documentation //! process. +use rustc_middle::ty::TyCtxt; use rustc_span::{InnerSpan, Span, DUMMY_SP}; use std::ops::Range; @@ -167,7 +168,7 @@ crate fn span_of_attrs(attrs: &clean::Attributes) -> Option { /// attributes are not all sugared doc comments. It's difficult to calculate the correct span in /// that case due to escaping and other source features. crate fn source_span_for_markdown_range( - cx: &DocContext<'_>, + tcx: TyCtxt<'_>, markdown: &str, md_range: &Range, attrs: &clean::Attributes, @@ -179,7 +180,7 @@ crate fn source_span_for_markdown_range( return None; } - let snippet = cx.sess().source_map().span_to_snippet(span_of_attrs(attrs)?).ok()?; + let snippet = tcx.sess.source_map().span_to_snippet(span_of_attrs(attrs)?).ok()?; let starting_line = markdown[..md_range.start].matches('\n').count(); let ending_line = starting_line + markdown[md_range.start..md_range.end].matches('\n').count(); diff --git a/src/librustdoc/passes/non_autolinks.rs b/src/librustdoc/passes/non_autolinks.rs index 09a1959fa113c..16ad940c62717 100644 --- a/src/librustdoc/passes/non_autolinks.rs +++ b/src/librustdoc/passes/non_autolinks.rs @@ -60,7 +60,7 @@ crate fn check_non_autolinks(krate: Crate, cx: &mut DocContext<'_>) -> Crate { impl<'a, 'tcx> DocFolder for NonAutolinksLinter<'a, 'tcx> { fn fold_item(&mut self, item: Item) -> Option { - let hir_id = match self.cx.as_local_hir_id(item.def_id) { + let hir_id = match DocContext::as_local_hir_id(self.cx.tcx, item.def_id) { Some(hir_id) => hir_id, None => { // If non-local, no need to check anything. @@ -70,7 +70,7 @@ impl<'a, 'tcx> DocFolder for NonAutolinksLinter<'a, 'tcx> { let dox = item.attrs.collapsed_doc_value().unwrap_or_default(); if !dox.is_empty() { let report_diag = |cx: &DocContext<'_>, msg: &str, url: &str, range: Range| { - let sp = super::source_span_for_markdown_range(cx, &dox, &range, &item.attrs) + let sp = super::source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs) .or_else(|| span_of_attrs(&item.attrs)) .unwrap_or(item.source.span()); cx.tcx.struct_span_lint_hir(crate::lint::NON_AUTOLINKS, hir_id, sp, |lint| { From 675edd0231fd799f130ae2d46e079b3753475449 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 22 Feb 2021 09:21:47 -0500 Subject: [PATCH 2/2] Remove RefCell around module_trait_cache --- src/librustdoc/core.rs | 4 +- .../passes/collect_intra_doc_links.rs | 47 ++++++++++--------- src/librustdoc/passes/html_tags.rs | 3 +- 3 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 5ff50e2538050..bcbc6e557d0a9 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -76,7 +76,7 @@ crate struct DocContext<'tcx> { /// /// See `collect_intra_doc_links::traits_implemented_by` for more details. /// `map>` - crate module_trait_cache: RefCell>>, + crate module_trait_cache: FxHashMap>, /// This same cache is used throughout rustdoc, including in [`crate::html::render`]. crate cache: Cache, /// Used by [`clean::inline`] to tell if an item has already been inlined. @@ -450,7 +450,7 @@ crate fn run_global_ctxt( .cloned() .filter(|trait_def_id| tcx.trait_is_auto(*trait_def_id)) .collect(), - module_trait_cache: RefCell::new(FxHashMap::default()), + module_trait_cache: FxHashMap::default(), cache: Cache::new(access_levels, render_options.document_private), inlined: FxHashSet::default(), output_format, diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 93aa069593694..ec5994b762b8a 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -484,13 +484,13 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { /// Resolves a string as a path within a particular namespace. Returns an /// optional URL fragment in the case of variants and methods. fn resolve<'path>( - &self, + &mut self, path_str: &'path str, ns: Namespace, module_id: DefId, extra_fragment: &Option, ) -> Result<(Res, Option), ErrorKind<'path>> { - let cx = &self.cx; + let tcx = self.cx.tcx; if let Some(res) = self.resolve_path(path_str, ns, module_id) { match res { @@ -498,7 +498,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // item a separate function. Res::Def(DefKind::AssocFn | DefKind::AssocConst, _) => assert_eq!(ns, ValueNS), Res::Def(DefKind::AssocTy, _) => assert_eq!(ns, TypeNS), - Res::Def(DefKind::Variant, _) => return handle_variant(cx, res, extra_fragment), + Res::Def(DefKind::Variant, _) => { + return handle_variant(self.cx, res, extra_fragment); + } // Not a trait item; just return what we found. Res::Primitive(ty) => { if extra_fragment.is_some() { @@ -565,13 +567,12 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ) => { debug!("looking for associated item named {} for item {:?}", item_name, did); // Checks if item_name belongs to `impl SomeItem` - let assoc_item = cx - .tcx + let assoc_item = tcx .inherent_impls(did) .iter() .flat_map(|&imp| { - cx.tcx.associated_items(imp).find_by_name_and_namespace( - cx.tcx, + tcx.associated_items(imp).find_by_name_and_namespace( + tcx, Ident::with_dummy_span(item_name), ns, imp, @@ -587,7 +588,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // something like [`ambi_fn`](::ambi_fn) .or_else(|| { let kind = - resolve_associated_trait_item(did, module_id, item_name, ns, &self.cx); + resolve_associated_trait_item(did, module_id, item_name, ns, self.cx); debug!("got associated item kind {:?}", kind); kind }); @@ -611,7 +612,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { debug!("looking for variants or fields named {} for {:?}", item_name, did); // FIXME(jynelson): why is this different from // `variant_field`? - match cx.tcx.type_of(did).kind() { + match tcx.type_of(did).kind() { ty::Adt(def, _) => { let field = if def.is_enum() { def.all_fields().find(|item| item.ident.name == item_name) @@ -652,10 +653,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { None } } - Res::Def(DefKind::Trait, did) => cx - .tcx + Res::Def(DefKind::Trait, did) => tcx .associated_items(did) - .find_by_name_and_namespace(cx.tcx, Ident::with_dummy_span(item_name), ns, did) + .find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, did) .map(|item| { let kind = match item.kind { ty::AssocKind::Const => "associatedconstant", @@ -699,7 +699,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { /// This returns the `Res` even if it was erroneous for some reason /// (such as having invalid URL fragments or being in the wrong namespace). fn check_full_res( - &self, + &mut self, ns: Namespace, path_str: &str, module_id: DefId, @@ -733,7 +733,7 @@ fn resolve_associated_trait_item( module: DefId, item_name: Symbol, ns: Namespace, - cx: &DocContext<'_>, + cx: &mut DocContext<'_>, ) -> Option<(ty::AssocKind, DefId)> { // FIXME: this should also consider blanket impls (`impl X for T`). Unfortunately // `get_auto_trait_and_blanket_impls` is broken because the caching behavior is wrong. In the @@ -758,10 +758,10 @@ fn resolve_associated_trait_item( /// /// NOTE: this cannot be a query because more traits could be available when more crates are compiled! /// So it is not stable to serialize cross-crate. -fn traits_implemented_by(cx: &DocContext<'_>, type_: DefId, module: DefId) -> FxHashSet { - let mut cache = cx.module_trait_cache.borrow_mut(); - let in_scope_traits = cache.entry(module).or_insert_with(|| { - cx.enter_resolver(|resolver| { +fn traits_implemented_by(cx: &mut DocContext<'_>, type_: DefId, module: DefId) -> FxHashSet { + let mut resolver = cx.resolver.borrow_mut(); + let in_scope_traits = cx.module_trait_cache.entry(module).or_insert_with(|| { + resolver.access(|resolver| { let parent_scope = &ParentScope::module(resolver.get_module(module), resolver); resolver .traits_in_scope(None, parent_scope, SyntaxContext::root(), None) @@ -771,13 +771,14 @@ fn traits_implemented_by(cx: &DocContext<'_>, type_: DefId, module: DefId) -> Fx }) }); - let ty = cx.tcx.type_of(type_); + let tcx = cx.tcx; + let ty = tcx.type_of(type_); let iter = in_scope_traits.iter().flat_map(|&trait_| { trace!("considering explicit impl for trait {:?}", trait_); // Look at each trait implementation to see if it's an impl for `did` - cx.tcx.find_map_relevant_impl(trait_, ty, |impl_| { - let trait_ref = cx.tcx.impl_trait_ref(impl_).expect("this is not an inherent impl"); + tcx.find_map_relevant_impl(trait_, ty, |impl_| { + let trait_ref = tcx.impl_trait_ref(impl_).expect("this is not an inherent impl"); // Check if these are the same type. let impl_type = trait_ref.self_ty(); trace!( @@ -1308,7 +1309,7 @@ impl LinkCollector<'_, '_> { /// After parsing the disambiguator, resolve the main part of the link. // FIXME(jynelson): wow this is just so much fn resolve_with_disambiguator( - &self, + &mut self, key: &ResolutionInfo, diag: DiagnosticInfo<'_>, ) -> Option<(Res, Option)> { @@ -1732,7 +1733,7 @@ fn report_diagnostic( /// handled earlier. For example, if passed `Item::Crate(std)` and `path_str` /// `std::io::Error::x`, this will resolve `std::io::Error`. fn resolution_failure( - collector: &LinkCollector<'_, '_>, + collector: &mut LinkCollector<'_, '_>, item: &Item, path_str: &str, disambiguator: Option, diff --git a/src/librustdoc/passes/html_tags.rs b/src/librustdoc/passes/html_tags.rs index 5fff7c8def31c..23364b6fec9d1 100644 --- a/src/librustdoc/passes/html_tags.rs +++ b/src/librustdoc/passes/html_tags.rs @@ -178,7 +178,8 @@ impl<'a, 'tcx> DocFolder for InvalidHtmlTagsLinter<'a, 'tcx> { let dox = item.attrs.collapsed_doc_value().unwrap_or_default(); if !dox.is_empty() { let report_diag = |msg: &str, range: &Range| { - let sp = match super::source_span_for_markdown_range(tcx, &dox, range, &item.attrs) { + let sp = match super::source_span_for_markdown_range(tcx, &dox, range, &item.attrs) + { Some(sp) => sp, None => span_of_attrs(&item.attrs).unwrap_or(item.source.span()), };