Skip to content

Commit

Permalink
Auto merge of #83738 - jyn514:only-load-some-crates, r=petrochenkov
Browse files Browse the repository at this point in the history
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 #82496.

Closes #68427. Let me know if you want me to open a separate issue for not freezing the resolver.
r? `@petrochenkov` cc `@eddyb` `@ollie27`
  • Loading branch information
bors committed Apr 3, 2021
2 parents ccd9975 + e4244e3 commit 640ce99
Show file tree
Hide file tree
Showing 10 changed files with 90 additions and 48 deletions.
86 changes: 55 additions & 31 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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");
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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('@') {
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
17 changes: 17 additions & 0 deletions src/test/rustdoc-ui/auxiliary/panic-item.rs
Original file line number Diff line number Diff line change
@@ -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() {}
3 changes: 3 additions & 0 deletions src/test/rustdoc-ui/unused-extern-crate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// check-pass
// aux-crate:panic_item=panic-item.rs
// @has unused_extern_crate/index.html
File renamed without changes.
8 changes: 8 additions & 0 deletions src/test/rustdoc/intra-doc/extern-crate-only-used-in-link.rs
Original file line number Diff line number Diff line change
@@ -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]
10 changes: 0 additions & 10 deletions src/test/rustdoc/issue-66159.rs

This file was deleted.

4 changes: 2 additions & 2 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
)
})
}
Expand Down

0 comments on commit 640ce99

Please sign in to comment.