Skip to content

Commit 3f07f1c

Browse files
committed
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
2 parents ded5ee0 + 33ded3e commit 3f07f1c

File tree

5 files changed

+43
-2
lines changed

5 files changed

+43
-2
lines changed

src/librustdoc/config.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -608,10 +608,12 @@ fn parse_extern_html_roots(
608608
/// Extracts `--extern CRATE=PATH` arguments from `matches` and
609609
/// returns a map mapping crate names to their paths or else an
610610
/// error message.
611+
/// Also handles `--extern-private` which for the purposes of rustdoc
612+
/// we can treat as `--extern`
611613
// FIXME(eddyb) This shouldn't be duplicated with `rustc::session`.
612614
fn parse_externs(matches: &getopts::Matches) -> Result<Externs, String> {
613615
let mut externs: BTreeMap<_, ExternEntry> = BTreeMap::new();
614-
for arg in &matches.opt_strs("extern") {
616+
for arg in matches.opt_strs("extern").iter().chain(matches.opt_strs("extern-private").iter()) {
615617
let mut parts = arg.splitn(2, '=');
616618
let name = parts.next().ok_or("--extern value must not be empty".to_string())?;
617619
let location = parts.next().map(|s| s.to_string());

src/librustdoc/core.rs

+24-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use rustc_lint;
22
use rustc::session::{self, config};
3+
use rustc::hir::def::Namespace::TypeNS;
34
use rustc::hir::def_id::{DefId, DefIndex, CrateNum, LOCAL_CRATE};
45
use rustc::hir::HirId;
56
use rustc::middle::cstore::CrateStore;
@@ -13,11 +14,13 @@ use rustc_interface::interface;
1314
use rustc_driver::abort_on_err;
1415
use rustc_resolve as resolve;
1516

17+
use syntax::ast::CRATE_NODE_ID;
1618
use syntax::source_map;
1719
use syntax::attr;
1820
use syntax::feature_gate::UnstableFeatures;
1921
use syntax::json::JsonEmitter;
2022
use syntax::symbol::sym;
23+
use syntax_pos::DUMMY_SP;
2124
use errors;
2225
use errors::emitter::{Emitter, EmitterWriter};
2326

@@ -246,6 +249,8 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
246249
..
247250
} = options;
248251

252+
let extern_names: Vec<String> = externs.iter().map(|(s,_)| s).cloned().collect();
253+
249254
// Add the rustdoc cfg into the doc build.
250255
cfgs.push("doc".to_string());
251256

@@ -344,7 +349,25 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
344349
// We need to hold on to the complete resolver, so we cause everything to be
345350
// cloned for the analysis passes to use. Suboptimal, but necessary in the
346351
// current architecture.
347-
let resolver = abort_on_err(compiler.expansion(), sess).peek().1.borrow().clone();
352+
let resolver = {
353+
let parts = abort_on_err(compiler.expansion(), sess).peek();
354+
let resolver = parts.1.borrow();
355+
356+
// Before we actually clone it, let's force all the extern'd crates to
357+
// actually be loaded, just in case they're only referred to inside
358+
// intra-doc-links
359+
resolver.borrow_mut().access(|resolver| {
360+
for extern_name in &extern_names {
361+
resolver.resolve_str_path_error(DUMMY_SP, extern_name, TypeNS, CRATE_NODE_ID)
362+
.unwrap_or_else(
363+
|()| panic!("Unable to resolve external crate {}", extern_name)
364+
);
365+
}
366+
});
367+
368+
// Now we're good to clone the resolver because everything should be loaded
369+
resolver.clone()
370+
};
348371

349372
if sess.has_errors() {
350373
sess.fatal("Compilation failed, aborting rustdoc");

src/librustdoc/lib.rs

+4
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,10 @@ fn opts() -> Vec<RustcOptGroup> {
143143
stable("extern", |o| {
144144
o.optmulti("", "extern", "pass an --extern to rustc", "NAME[=PATH]")
145145
}),
146+
stable("extern-private", |o| {
147+
o.optmulti("", "extern-private",
148+
"pass an --extern to rustc (compatibility only)", "NAME=PATH")
149+
}),
146150
unstable("extern-html-root-url", |o| {
147151
o.optmulti("", "extern-html-root-url",
148152
"base URL to use for dependencies", "NAME=URL")
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
/// This will be referred to by the test docstring
2+
pub struct Something;

src/test/rustdoc/issue-66159.rs

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// aux-build:issue-66159-1.rs
2+
// extern-private:issue_66159_1
3+
4+
// The issue was an ICE which meant that we never actually generated the docs
5+
// so if we have generated the docs, we're okay.
6+
// Since we don't generate the docs for the auxilliary files, we can't actually
7+
// verify that the struct is linked correctly.
8+
9+
// @has issue_66159/index.html
10+
//! [issue_66159_1::Something]

0 commit comments

Comments
 (0)