From cc69c652072ed8c544b288b2c45a183e57dd052d Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 31 Mar 2021 22:33:07 -0400 Subject: [PATCH] Don't load all extern crates unconditionally Instead, only load the crates that are linked to with intra-doc links. This doesn't help very much with any of rustdoc's fundamental issues with freezing the resolver, but it at least fixes a stable-to-stable regression, and makes the crate loading model somewhat more consistent with rustc's. --- src/librustdoc/core.rs | 86 ++++++++++++------- src/librustdoc/lib.rs | 4 +- .../passes/collect_intra_doc_links.rs | 4 +- src/librustdoc/passes/mod.rs | 2 +- src/test/rustdoc-ui/auxiliary/panic-item.rs | 15 ++++ src/test/rustdoc-ui/unused-extern-crate.rs | 3 + .../auxiliary/issue-66159-1.rs | 0 .../extern-crate-only-used-in-link.rs | 8 ++ src/test/rustdoc/issue-66159.rs | 10 --- src/tools/compiletest/src/header.rs | 4 +- 10 files changed, 88 insertions(+), 48 deletions(-) create mode 100644 src/test/rustdoc-ui/auxiliary/panic-item.rs create mode 100644 src/test/rustdoc-ui/unused-extern-crate.rs rename src/test/rustdoc/{ => intra-doc}/auxiliary/issue-66159-1.rs (100%) create mode 100644 src/test/rustdoc/intra-doc/extern-crate-only-used-in-link.rs delete mode 100644 src/test/rustdoc/issue-66159.rs diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 5a022b2d40c56..3d2165343572e 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -1,3 +1,4 @@ +use rustc_ast as ast; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::sync::{self, Lrc}; use rustc_driver::abort_on_err; @@ -22,7 +23,7 @@ use rustc_session::DiagnosticOutput; use rustc_session::Session; use rustc_span::source_map; use rustc_span::symbol::sym; -use rustc_span::{Span, DUMMY_SP}; +use rustc_span::Span; use std::cell::RefCell; use std::collections::hash_map::Entry; @@ -348,42 +349,65 @@ crate fn create_config( } crate fn create_resolver<'a>( - externs: config::Externs, queries: &Queries<'a>, sess: &Session, ) -> Rc> { - let extern_names: Vec = externs - .iter() - .filter(|(_, entry)| entry.add_prelude) - .map(|(name, _)| name) - .cloned() - .collect(); - let parts = abort_on_err(queries.expansion(), sess).peek(); - let resolver = parts.1.borrow(); - - // Before we actually clone it, let's force all the extern'd crates to - // actually be loaded, just in case they're only referred to inside - // intra-doc links - resolver.borrow_mut().access(|resolver| { - sess.time("load_extern_crates", || { - for extern_name in &extern_names { - debug!("loading extern crate {}", extern_name); - if let Err(()) = resolver - .resolve_str_path_error( - DUMMY_SP, - extern_name, - TypeNS, - LocalDefId { local_def_index: CRATE_DEF_INDEX }.to_def_id(), - ) { - warn!("unable to resolve external crate {} (do you have an unused `--extern` crate?)", extern_name) - } + let (krate, resolver, _) = &*parts; + let resolver = resolver.borrow().clone(); + + // Letting the resolver escape at the end of the function leads to inconsistencies between the + // crates the TyCtxt sees and the resolver sees (because the resolver could load more crates + // after escaping). Hopefully `IntraLinkCrateLoader` gets all the crates we need ... + struct IntraLinkCrateLoader { + current_mod: DefId, + resolver: Rc>, + } + impl ast::visit::Visitor<'_> for IntraLinkCrateLoader { + fn visit_attribute(&mut self, attr: &ast::Attribute) { + use crate::html::markdown::{markdown_links, MarkdownLink}; + use crate::passes::collect_intra_doc_links::Disambiguator; + + if let Some(doc) = attr.doc_str() { + for MarkdownLink { link, .. } in markdown_links(&doc.as_str()) { + // FIXME: this misses a *lot* of the preprocessing done in collect_intra_doc_links + // I think most of it shouldn't be necessary since we only need the crate prefix? + let path_str = match Disambiguator::from_str(&link) { + Ok(x) => x.map_or(link.as_str(), |(_, p)| p), + Err(_) => continue, + }; + self.resolver.borrow_mut().access(|resolver| { + let _ = resolver.resolve_str_path_error( + attr.span, + path_str, + TypeNS, + self.current_mod, + ); + }); + } } - }); - }); + ast::visit::walk_attribute(self, attr); + } + + fn visit_item(&mut self, item: &ast::Item) { + use rustc_ast_lowering::ResolverAstLowering; + + if let ast::ItemKind::Mod(..) = item.kind { + let new_mod = + self.resolver.borrow_mut().access(|resolver| resolver.local_def_id(item.id)); + let old_mod = mem::replace(&mut self.current_mod, new_mod.to_def_id()); + ast::visit::walk_item(self, item); + self.current_mod = old_mod; + } else { + ast::visit::walk_item(self, item); + } + } + } + let crate_id = LocalDefId { local_def_index: CRATE_DEF_INDEX }.to_def_id(); + let mut loader = IntraLinkCrateLoader { current_mod: crate_id, resolver }; + ast::visit::walk_crate(&mut loader, krate); - // Now we're good to clone the resolver because everything should be loaded - resolver.clone() + loader.resolver } crate fn run_global_ctxt( diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index dabc21e3a447c..a3b83b9701f56 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -31,6 +31,7 @@ extern crate tracing; // // Dependencies listed in Cargo.toml do not need `extern crate`. extern crate rustc_ast; +extern crate rustc_ast_lowering; extern crate rustc_ast_pretty; extern crate rustc_attr; extern crate rustc_data_structures; @@ -637,7 +638,6 @@ fn main_options(options: config::Options) -> MainResult { let default_passes = options.default_passes; let output_format = options.output_format; // FIXME: fix this clone (especially render_options) - let externs = options.externs.clone(); let manual_passes = options.manual_passes.clone(); let render_options = options.render_options.clone(); let config = core::create_config(options); @@ -649,7 +649,7 @@ fn main_options(options: config::Options) -> MainResult { // We need to hold on to the complete resolver, so we cause everything to be // cloned for the analysis passes to use. Suboptimal, but necessary in the // current architecture. - let resolver = core::create_resolver(externs, queries, &sess); + let resolver = core::create_resolver(queries, &sess); if sess.has_errors() { sess.fatal("Compilation failed, aborting rustdoc"); diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 55978ca551b05..437f42b26dd11 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1517,7 +1517,7 @@ fn range_between_backticks(ori_link: &MarkdownLink) -> Range { #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] /// Disambiguators for a link. -enum Disambiguator { +crate enum Disambiguator { /// `prim@` /// /// This is buggy, see @@ -1546,7 +1546,7 @@ impl Disambiguator { /// This returns `Ok(Some(...))` if a disambiguator was found, /// `Ok(None)` if no disambiguator was found, or `Err(...)` /// if there was a problem with the disambiguator. - fn from_str(link: &str) -> Result, (String, Range)> { + crate fn from_str(link: &str) -> Result, (String, Range)> { use Disambiguator::{Kind, Namespace as NS, Primitive}; if let Some(idx) = link.find('@') { diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 4c639c8496db3..755217a4629f4 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -30,7 +30,7 @@ crate use self::unindent_comments::UNINDENT_COMMENTS; mod propagate_doc_cfg; crate use self::propagate_doc_cfg::PROPAGATE_DOC_CFG; -mod collect_intra_doc_links; +crate mod collect_intra_doc_links; crate use self::collect_intra_doc_links::COLLECT_INTRA_DOC_LINKS; mod doc_test_lints; diff --git a/src/test/rustdoc-ui/auxiliary/panic-item.rs b/src/test/rustdoc-ui/auxiliary/panic-item.rs new file mode 100644 index 0000000000000..ef8e912738c1a --- /dev/null +++ b/src/test/rustdoc-ui/auxiliary/panic-item.rs @@ -0,0 +1,15 @@ +#![no_std] +#![feature(lang_items)] + +use core::panic::PanicInfo; +use core::sync::atomic::{self, Ordering}; + +#[panic_handler] +fn panic(_info: &PanicInfo) -> ! { + loop { + atomic::compiler_fence(Ordering::SeqCst); + } +} + +#[lang = "eh_personality"] +fn foo() {} diff --git a/src/test/rustdoc-ui/unused-extern-crate.rs b/src/test/rustdoc-ui/unused-extern-crate.rs new file mode 100644 index 0000000000000..f703a18379074 --- /dev/null +++ b/src/test/rustdoc-ui/unused-extern-crate.rs @@ -0,0 +1,3 @@ +// check-pass +// aux-crate:panic_item=panic-item.rs +// @has unused_extern_crate/index.html diff --git a/src/test/rustdoc/auxiliary/issue-66159-1.rs b/src/test/rustdoc/intra-doc/auxiliary/issue-66159-1.rs similarity index 100% rename from src/test/rustdoc/auxiliary/issue-66159-1.rs rename to src/test/rustdoc/intra-doc/auxiliary/issue-66159-1.rs diff --git a/src/test/rustdoc/intra-doc/extern-crate-only-used-in-link.rs b/src/test/rustdoc/intra-doc/extern-crate-only-used-in-link.rs new file mode 100644 index 0000000000000..0964c79de068f --- /dev/null +++ b/src/test/rustdoc/intra-doc/extern-crate-only-used-in-link.rs @@ -0,0 +1,8 @@ +// aux-build:issue-66159-1.rs +// aux-crate:priv:issue_66159_1=issue-66159-1.rs +// build-aux-docs +// compile-flags:-Z unstable-options + +// @has extern_crate_only_used_in_link/index.html +// @has - '//a[@href="../issue_66159_1/struct.Something.html"]' 'issue_66159_1::Something' +//! [issue_66159_1::Something] diff --git a/src/test/rustdoc/issue-66159.rs b/src/test/rustdoc/issue-66159.rs deleted file mode 100644 index 003d079a470c0..0000000000000 --- a/src/test/rustdoc/issue-66159.rs +++ /dev/null @@ -1,10 +0,0 @@ -// aux-crate:priv:issue_66159_1=issue-66159-1.rs -// compile-flags:-Z unstable-options - -// The issue was an ICE which meant that we never actually generated the docs -// so if we have generated the docs, we're okay. -// Since we don't generate the docs for the auxiliary files, we can't actually -// verify that the struct is linked correctly. - -// @has issue_66159/index.html -//! [issue_66159_1::Something] diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 83ea676e8f4ee..531a23d76a27b 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -708,8 +708,8 @@ impl Config { self.parse_name_value_directive(line, "aux-crate").map(|r| { let mut parts = r.trim().splitn(2, '='); ( - parts.next().expect("aux-crate name").to_string(), - parts.next().expect("aux-crate value").to_string(), + parts.next().expect("missing aux-crate name (e.g. log=log.rs)").to_string(), + parts.next().expect("missing aux-crate value (e.g. log=log.rs)").to_string(), ) }) }