Auto merge of #66211 - kinnison:kinnison/fix-66159, r=GuillaumeGomez

Fix ICE when documentation includes intra-doc-link

When collecting intra-doc-links we could trigger the loading of extra crates into the crate store due to name resolution finding crates referred to in documentation but not in code.  This might be due to
configuration differences or simply referring to something else.

This would cause an ICE because the newly loaded crate metadata existed in a crate store associated with the rustdoc global context, but the resolver had its own crate store cloned just before the documentation processing began and as such it could try and look up crates in a store which lacked them.

In this PR, I add support for `--extern-private` to the `rustdoc` tool so that it is supported for `compiletest` to then pass the crates in; and then I fix the issue by forcing the resolver to look over all the crates before we then lower the input ready for processing into documentation.

The first commit (the `--extern-private`) could be replaced with a commit which adds support for `--extern` to `compiletest` if preferred, though I think that adding `--extern-private` to `rustdoc` is more useful anyway since it makes the CLI a little more like `rustc`'s which might help reduce surprise for someone running it by hand or in their own test code.

The PR is meant to fix #66159 though it may also fix #65840.

cc @GuillaumeGomez

5 files changed, 43 insertions, 2 deletions

diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs
index 255d5f9f35d..3e1c57182b9 100644
--- a/src/librustdoc/config.rs
+++ b/src/librustdoc/config.rs
@@ -608,10 +608,12 @@ fn parse_extern_html_roots(
 /// Extracts `--extern CRATE=PATH` arguments from `matches` and
 /// returns a map mapping crate names to their paths or else an
 /// error message.
+/// Also handles `--extern-private` which for the purposes of rustdoc
+/// we can treat as `--extern`
 // FIXME(eddyb) This shouldn't be duplicated with `rustc::session`.
 fn parse_externs(matches: &getopts::Matches) -> Result<Externs, String> {
     let mut externs: BTreeMap<_, ExternEntry> = BTreeMap::new();
-    for arg in &matches.opt_strs("extern") {
+    for arg in matches.opt_strs("extern").iter().chain(matches.opt_strs("extern-private").iter()) {
         let mut parts = arg.splitn(2, '=');
         let name = parts.next().ok_or("--extern value must not be empty".to_string())?;
         let location = parts.next().map(|s| s.to_string());
diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs
index 0f508966a4c..a40325448b1 100644
--- a/src/librustdoc/core.rs
+++ b/src/librustdoc/core.rs
@@ -1,5 +1,6 @@
 use rustc_lint;
 use rustc::session::{self, config};
+use rustc::hir::def::Namespace::TypeNS;
 use rustc::hir::def_id::{DefId, DefIndex, CrateNum, LOCAL_CRATE};
 use rustc::hir::HirId;
 use rustc::middle::cstore::CrateStore;
@@ -13,11 +14,13 @@ use rustc_interface::interface;
 use rustc_driver::abort_on_err;
 use rustc_resolve as resolve;
 
+use syntax::ast::CRATE_NODE_ID;
 use syntax::source_map;
 use syntax::attr;
 use syntax::feature_gate::UnstableFeatures;
 use syntax::json::JsonEmitter;
 use syntax::symbol::sym;
+use syntax_pos::DUMMY_SP;
 use errors;
 use errors::emitter::{Emitter, EmitterWriter};
 
@@ -246,6 +249,8 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
         ..
     } = options;
 
+    let extern_names: Vec<String> = externs.iter().map(|(s,_)| s).cloned().collect();
+
     // Add the rustdoc cfg into the doc build.
     cfgs.push("doc".to_string());
 
@@ -344,7 +349,25 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
         // 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 = abort_on_err(compiler.expansion(), sess).peek().1.borrow().clone();
+        let resolver = {
+            let parts = abort_on_err(compiler.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| {
+                for extern_name in &extern_names {
+                    resolver.resolve_str_path_error(DUMMY_SP, extern_name, TypeNS, CRATE_NODE_ID)
+                        .unwrap_or_else(
+                            |()| panic!("Unable to resolve external crate {}", extern_name)
+                        );
+                }
+            });
+
+            // Now we're good to clone the resolver because everything should be loaded
+            resolver.clone()
+        };
 
         if sess.has_errors() {
             sess.fatal("Compilation failed, aborting rustdoc");
diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs
index 86e0f22db50..4f102dbf128 100644
--- a/src/librustdoc/lib.rs
+++ b/src/librustdoc/lib.rs
@@ -143,6 +143,10 @@ fn opts() -> Vec<RustcOptGroup> {
         stable("extern", |o| {
             o.optmulti("", "extern", "pass an --extern to rustc", "NAME[=PATH]")
         }),
+        stable("extern-private", |o| {
+            o.optmulti("", "extern-private",
+                       "pass an --extern to rustc (compatibility only)", "NAME=PATH")
+        }),
         unstable("extern-html-root-url", |o| {
             o.optmulti("", "extern-html-root-url",
                        "base URL to use for dependencies", "NAME=URL")
diff --git a/src/test/rustdoc/auxiliary/issue-66159-1.rs b/src/test/rustdoc/auxiliary/issue-66159-1.rs
new file mode 100644
index 00000000000..2f3d069bd51
--- /dev/null
+++ b/src/test/rustdoc/auxiliary/issue-66159-1.rs
@@ -0,0 +1,2 @@
+/// This will be referred to by the test docstring
+pub struct Something;
diff --git a/src/test/rustdoc/issue-66159.rs b/src/test/rustdoc/issue-66159.rs
new file mode 100644
index 00000000000..88eca733870
--- /dev/null
+++ b/src/test/rustdoc/issue-66159.rs
@@ -0,0 +1,10 @@
+// aux-build:issue-66159-1.rs
+// extern-private:issue_66159_1
+
+// 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 auxilliary files, we can't actually
+// verify that the struct is linked correctly.
+
+// @has issue_66159/index.html
+//! [issue_66159_1::Something]