Auto merge of #83738 - jyn514:only-load-some-crates, r=petrochenkov

rustdoc: 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. I tested and it unfortunately does not help at all with https://github.com/rust-lang/rust/pull/82496.

Closes https://github.com/rust-lang/rust/issues/68427. Let me know if you want me to open a separate issue for not freezing the resolver.
r? `@petrochenkov` cc `@eddyb` `@ollie27`

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();
+    let (krate, resolver, _) = &*parts;
+    let resolver = resolver.borrow().clone();
 
-    // 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)
-                  }
+    // 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);
+        }
 
-    // Now we're good to clone the resolver because everything should be loaded
-    resolver.clone()
+        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);
+
+    loader.resolver
 }
 
 crate fn run_global_ctxt(

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;
@@ -645,7 +646,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);
@@ -657,7 +657,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");

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('@') {

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;

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() {}

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

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]

src/test/rustdoc/issue-66159.rs

@@ -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]

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(),
             )
         })
     }