From 021d1fbd007456e6a8f2a7b865545352afb66a3f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 7 Oct 2022 13:57:32 +1100 Subject: [PATCH] Clean up rustdoc startup. rustc's startup has several layers, including: - `interface::run_compiler` passes a closure, `f`, to `run_in_thread_pool_with_globals`, which creates a thread pool, sets up session globals, and passes `f` to `create_compiler_and_run`. - `create_compiler_and_run` creates a `Session`, a `Compiler`, sets the source map, and calls `f`. rustdoc is a bit different. - `main_args` calls `main_options` via `run_in_thread_pool_with_globals`, which (again) creates a thread pool (hardcoded to a single thread!) and sets up session globals. - `main_options` has four different paths. - The second one calls `interface::run_compiler`, which redoes the `run_in_thread_pool_with_globals`! This is bad. - The fourth one calls `interface::create_compiler_and_run`, which is reasonable. - The first and third ones don't do anything of note involving the above functions, except for some symbol interning which requires session globals. In other words, rustdoc calls into `rustc_interface` at three different levels. It's a bit confused, and feels like code where functionality has been added by different people at different times without fully understanding how the globally accessible stuff is set up. This commit tidies things up. It removes the `run_in_thread_pool_with_globals` call in `main_args`, and adjust the four paths in `main_options` as follows. - `markdown::test` calls `test::test_main`, which provides its own parallelism and so doesn't need a thread pool. It had one small use of symbol interning, which required session globals, but the commit removes this. - `doctest::run` already calls `interface::run_compiler`, so it doesn't need further adjustment. - `markdown::render` is simple but needs session globals for interning (which can't easily be removed), so it's now wrapped in `create_session_globals_then`. - The fourth path now uses `interface::run_compiler`, which is equivalent to the old `run_in_thread_pool_with_globals` + `create_compiler_and_run` pairing. --- src/librustdoc/doctest.rs | 9 ++++----- src/librustdoc/lib.rs | 14 +++++++------- src/librustdoc/markdown.rs | 5 +++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index ac8b5211878..58f02e3da55 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -18,7 +18,6 @@ use rustc_span::edition::Edition; use rustc_span::source_map::SourceMap; use rustc_span::symbol::sym; -use rustc_span::Symbol; use rustc_span::{BytePos, FileName, Pos, Span, DUMMY_SP}; use rustc_target::spec::TargetTriple; use tempfile::Builder as TempFileBuilder; @@ -124,7 +123,7 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> { let opts = scrape_test_config(crate_attrs); let enable_per_target_ignores = options.enable_per_target_ignores; let mut collector = Collector::new( - tcx.crate_name(LOCAL_CRATE), + tcx.crate_name(LOCAL_CRATE).to_string(), options, false, opts, @@ -908,7 +907,7 @@ pub(crate) struct Collector { rustdoc_options: RustdocOptions, use_headers: bool, enable_per_target_ignores: bool, - crate_name: Symbol, + crate_name: String, opts: GlobalTestOptions, position: Span, source_map: Option>, @@ -920,7 +919,7 @@ pub(crate) struct Collector { impl Collector { pub(crate) fn new( - crate_name: Symbol, + crate_name: String, rustdoc_options: RustdocOptions, use_headers: bool, opts: GlobalTestOptions, @@ -983,7 +982,7 @@ impl Tester for Collector { fn add_test(&mut self, test: String, config: LangString, line: usize) { let filename = self.get_filename(); let name = self.generate_name(line, &filename); - let crate_name = self.crate_name.to_string(); + let crate_name = self.crate_name.clone(); let opts = self.opts.clone(); let edition = config.edition.unwrap_or(self.rustdoc_options.edition); let rustdoc_options = self.rustdoc_options.clone(); diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index ef01b854e5a..ce6f7e817c6 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -700,11 +700,7 @@ fn main_args(at_args: &[String]) -> MainResult { }; } }; - rustc_interface::util::run_in_thread_pool_with_globals( - options.edition, - 1, // this runs single-threaded, even in a parallel compiler - move || main_options(options), - ) + main_options(options) } fn wrap_return(diag: &rustc_errors::Handler, res: Result<(), String>) -> MainResult { @@ -749,9 +745,12 @@ fn main_options(options: config::Options) -> MainResult { (true, true) => return wrap_return(&diag, markdown::test(options)), (true, false) => return doctest::run(options), (false, true) => { + // Session globals are required for symbol interning. return wrap_return( &diag, - markdown::render(&options.input, options.render_options, options.edition), + rustc_span::create_session_globals_then(options.edition, || { + markdown::render(&options.input, options.render_options, options.edition) + }), ); } (false, false) => {} @@ -777,9 +776,10 @@ fn main_options(options: config::Options) -> MainResult { let render_options = options.render_options.clone(); let scrape_examples_options = options.scrape_examples_options.clone(); let document_private = options.render_options.document_private; + let config = core::create_config(options); - interface::create_compiler_and_run(config, |compiler| { + interface::run_compiler(config, |compiler| { let sess = compiler.session(); if sess.opts.describe_lints { diff --git a/src/librustdoc/markdown.rs b/src/librustdoc/markdown.rs index 0b557ef244e..eb64ac455dc 100644 --- a/src/librustdoc/markdown.rs +++ b/src/librustdoc/markdown.rs @@ -5,7 +5,6 @@ use rustc_span::edition::Edition; use rustc_span::source_map::DUMMY_SP; -use rustc_span::Symbol; use crate::config::{Options, RenderOptions}; use crate::doctest::{Collector, GlobalTestOptions}; @@ -36,6 +35,8 @@ fn extract_leading_metadata(s: &str) -> (Vec<&str>, &str) { /// Render `input` (e.g., "foo.md") into an HTML file in `output` /// (e.g., output = "bar" => "bar/foo.html"). +/// +/// Requires session globals to be available, for symbol interning. pub(crate) fn render>( input: P, options: RenderOptions, @@ -133,7 +134,7 @@ pub(crate) fn test(options: Options) -> Result<(), String> { let mut opts = GlobalTestOptions::default(); opts.no_crate_inject = true; let mut collector = Collector::new( - Symbol::intern(&options.input.display().to_string()), + options.input.display().to_string(), options.clone(), true, opts,