about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJoshua Nelson <jyn514@gmail.com>2021-03-31 22:33:07 -0400
committerJoshua Nelson <jyn514@gmail.com>2021-04-02 16:34:53 -0400
commite4244e37103d83b2cd54637ca2ac4f1c76c35a43 (patch)
tree7d8752990958f1b9a2c34e041383ba180bc6f097
parenta5029ac0ab372aec515db2e718da6d7787f3d122 (diff)
downloadrust-e4244e37103d83b2cd54637ca2ac4f1c76c35a43.tar.gz
rust-e4244e37103d83b2cd54637ca2ac4f1c76c35a43.zip
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.
-rw-r--r--src/librustdoc/core.rs86
-rw-r--r--src/librustdoc/lib.rs4
-rw-r--r--src/librustdoc/passes/collect_intra_doc_links.rs4
-rw-r--r--src/librustdoc/passes/mod.rs2
-rw-r--r--src/test/rustdoc-ui/auxiliary/panic-item.rs17
-rw-r--r--src/test/rustdoc-ui/unused-extern-crate.rs3
-rw-r--r--src/test/rustdoc/intra-doc/auxiliary/issue-66159-1.rs (renamed from src/test/rustdoc/auxiliary/issue-66159-1.rs)0
-rw-r--r--src/test/rustdoc/intra-doc/extern-crate-only-used-in-link.rs8
-rw-r--r--src/test/rustdoc/issue-66159.rs10
-rw-r--r--src/tools/compiletest/src/header.rs4
10 files changed, 90 insertions, 48 deletions
diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs
index 5a022b2d40c..3d216534357 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<RefCell<interface::BoxedResolver>> {
-    let extern_names: Vec<String> = 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<RefCell<interface::BoxedResolver>>,
+    }
+    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 dabc21e3a44..a3b83b9701f 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 55978ca551b..437f42b26dd 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<usize> {
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
 /// Disambiguators for a link.
-enum Disambiguator {
+crate enum Disambiguator {
     /// `prim@`
     ///
     /// This is buggy, see <https://github.com/rust-lang/rust/pull/77875#discussion_r503583103>
@@ -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<Option<(Self, &str)>, (String, Range<usize>)> {
+    crate fn from_str(link: &str) -> Result<Option<(Self, &str)>, (String, Range<usize>)> {
         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 4c639c8496d..755217a4629 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 00000000000..17b26850d4d
--- /dev/null
+++ b/src/test/rustdoc-ui/auxiliary/panic-item.rs
@@ -0,0 +1,17 @@
+// no-prefer-dynamic
+#![crate_type = "lib"]
+#![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 00000000000..f703a183790
--- /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
index 2f3d069bd51..2f3d069bd51 100644
--- a/src/test/rustdoc/auxiliary/issue-66159-1.rs
+++ b/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 00000000000..0964c79de06
--- /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 003d079a470..00000000000
--- 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 83ea676e8f4..531a23d76a2 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(),
             )
         })
     }