Rollup merge of #88215 - jyn514:lazy-loading, r=petrochenkov

Reland #83738: "rustdoc: Don't load all extern crates unconditionally"

I hopefully found all the bugs :crossed_fingers: time for a take two. See the last commit for details on what went wrong before.

r? `@petrochenkov` (but feel free to reassign to Guillaume if you don't have time.)

Closes https://github.com/rust-lang/rust/issues/68427. Includes a fix for https://github.com/rust-lang/rust/issues/84738.

src/librustdoc/core.rs

@@ -4,9 +4,8 @@
 use rustc_errors::emitter::{Emitter, EmitterWriter};
 use rustc_errors::json::JsonEmitter;
 use rustc_feature::UnstableFeatures;
-use rustc_hir::def::Namespace::TypeNS;
 use rustc_hir::def::Res;
-use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_INDEX};
+use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_hir::HirId;
 use rustc_hir::{
     intravisit::{self, NestedVisitorMap, Visitor},
@@ -23,7 +22,7 @@
 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::mem;
@@ -301,41 +300,13 @@ impl<'tcx> DocContext<'tcx> {
 }
 
 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 (_, resolver, _) = &*abort_on_err(queries.expansion(), sess).peek();
-
-    // 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, _) = &*abort_on_err(queries.expansion(), sess).peek();
+    let resolver = resolver.clone();
 
-    // Now we're good to clone the resolver because everything should be loaded
-    resolver.clone()
+    crate::passes::collect_intra_doc_links::load_intra_link_crates(resolver, krate)
 }
 
 crate fn run_global_ctxt(

src/librustdoc/lib.rs

@@ -30,6 +30,7 @@
 // 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;
@@ -724,7 +725,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);
@@ -742,7 +742,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

@@ -37,6 +37,9 @@
 use crate::lint::{BROKEN_INTRA_DOC_LINKS, PRIVATE_INTRA_DOC_LINKS};
 use crate::passes::Pass;
 
+mod early;
+crate use early::load_intra_link_crates;
+
 crate const COLLECT_INTRA_DOC_LINKS: Pass = Pass {
     name: "collect-intra-doc-links",
     run: collect_intra_doc_links,

src/librustdoc/passes/collect_intra_doc_links/early.rs

+use rustc_ast as ast;
+use rustc_hir::def::Namespace::TypeNS;
+use rustc_hir::def_id::{LocalDefId, CRATE_DEF_ID};
+use rustc_interface::interface;
+use rustc_span::Span;
+
+use std::cell::RefCell;
+use std::mem;
+use std::rc::Rc;
+
+type Resolver = Rc<RefCell<interface::BoxedResolver>>;
+// 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 ...
+crate fn load_intra_link_crates(resolver: Resolver, krate: &ast::Crate) -> Resolver {
+    let mut loader = IntraLinkCrateLoader { current_mod: CRATE_DEF_ID, resolver };
+    // `walk_crate` doesn't visit the crate itself for some reason.
+    loader.load_links_in_attrs(&krate.attrs, krate.span);
+    ast::visit::walk_crate(&mut loader, krate);
+    loader.resolver
+}
+
+struct IntraLinkCrateLoader {
+    current_mod: LocalDefId,
+    resolver: Rc<RefCell<interface::BoxedResolver>>,
+}
+
+impl IntraLinkCrateLoader {
+    fn load_links_in_attrs(&mut self, attrs: &[ast::Attribute], span: Span) {
+        use crate::html::markdown::markdown_links;
+        use crate::passes::collect_intra_doc_links::preprocess_link;
+
+        // FIXME: this probably needs to consider inlining
+        let attrs = crate::clean::Attributes::from_ast(attrs, None);
+        for (parent_module, doc) in attrs.collapsed_doc_value_by_module_level() {
+            debug!(?doc);
+            for link in markdown_links(&doc.as_str()) {
+                debug!(?link.link);
+                let path_str = if let Some(Ok(x)) = preprocess_link(&link) {
+                    x.path_str
+                } else {
+                    continue;
+                };
+                self.resolver.borrow_mut().access(|resolver| {
+                    let _ = resolver.resolve_str_path_error(
+                        span,
+                        &path_str,
+                        TypeNS,
+                        parent_module.unwrap_or(self.current_mod.to_def_id()),
+                    );
+                });
+            }
+        }
+    }
+}
+
+impl ast::visit::Visitor<'_> for IntraLinkCrateLoader {
+    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);
+
+            self.load_links_in_attrs(&item.attrs, item.span);
+            ast::visit::walk_item(self, item);
+
+            self.current_mod = old_mod;
+        } else {
+            self.load_links_in_attrs(&item.attrs, item.span);
+            ast::visit::walk_item(self, item);
+        }
+    }
+}

src/librustdoc/passes/mod.rs

@@ -30,7 +30,7 @@
 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

+// 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

+// check-pass
+// aux-crate:panic_item=panic-item.rs
+// @has unused_extern_crate/index.html

src/test/rustdoc/auxiliary/issue-66159-1.rs

-/// This will be referred to by the test docstring
-pub struct Something;

src/test/rustdoc/intra-doc/auxiliary/pub-struct.rs

+pub struct SomeStruct;

src/test/rustdoc/intra-doc/extern-reference-link.rs

+// compile-flags: --extern pub_struct
+// aux-build:pub-struct.rs
+
+/// [SomeStruct]
+///
+/// [SomeStruct]: pub_struct::SomeStruct
+pub fn foo() {}

src/test/rustdoc/issue-66159.rs → src/test/rustdoc/intra-doc/issue-66159.rs

-// aux-crate:priv:issue_66159_1=issue-66159-1.rs
+// aux-crate:priv:pub_struct=pub-struct.rs
 // compile-flags:-Z unstable-options
 
 // The issue was an ICE which meant that we never actually generated the docs
@@ -7,4 +7,4 @@
 // verify that the struct is linked correctly.
 
 // @has issue_66159/index.html
-//! [issue_66159_1::Something]
+//! [pub_struct::SomeStruct]