From d61605cef866008a19e33eb596df1d76ff1571f3 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 30 Aug 2019 09:03:58 +1000 Subject: [PATCH 1/4] Don't call `self.parse()` in `Compiler::crate_name()` unless necessary. --- src/librustc_interface/queries.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/librustc_interface/queries.rs b/src/librustc_interface/queries.rs index ed50dadb60099..996e9fae0db56 100644 --- a/src/librustc_interface/queries.rs +++ b/src/librustc_interface/queries.rs @@ -126,17 +126,18 @@ impl Compiler { pub fn crate_name(&self) -> Result<&Query> { self.queries.crate_name.compute(|| { - let parse_result = self.parse()?; - let krate = parse_result.peek(); - let result = match self.crate_name { + Ok(match self.crate_name { Some(ref crate_name) => crate_name.clone(), - None => rustc_codegen_utils::link::find_crate_name( - Some(self.session()), - &krate.attrs, - &self.input - ), - }; - Ok(result) + None => { + let parse_result = self.parse()?; + let krate = parse_result.peek(); + rustc_codegen_utils::link::find_crate_name( + Some(self.session()), + &krate.attrs, + &self.input + ) + } + }) }) } From cd0c21b0e5b68e29c63c3c98db9147cb0e5a3bc8 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 30 Aug 2019 16:24:01 +1000 Subject: [PATCH 2/4] Remove `lower_to_hir()` call from `prepare_output()`. It's a false dependency. The result isn't used and there are no relevant side-effects. --- src/librustc_interface/queries.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc_interface/queries.rs b/src/librustc_interface/queries.rs index 996e9fae0db56..c281bc5360704 100644 --- a/src/librustc_interface/queries.rs +++ b/src/librustc_interface/queries.rs @@ -195,7 +195,6 @@ impl Compiler { pub fn prepare_outputs(&self) -> Result<&Query> { self.queries.prepare_outputs.compute(|| { - self.lower_to_hir()?; let krate = self.expansion()?; let krate = krate.peek(); let crate_name = self.crate_name()?; From d264a56068d7fd881b2e082c4d80d81d22c4ce79 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 30 Aug 2019 16:53:34 +1000 Subject: [PATCH 3/4] Move call site of `dep_graph_future()`. `Compiler::register_plugins()` calls `passes::register_plugins()`, which calls `Compiler::dep_graph_future()`. This is the only way in which a `passes` function calls a `Compiler` function. This commit moves the `dep_graph_future()` call out of `passes::register_plugins()` and into `Compiler::register_plugins()`, which is a more sensible spot for it. This will delay the loading of the dep graph slightly -- from the middle of plugin registration to the end of plugin registration -- but plugin registration is fast enough (especially compared to expansion) that the impact should be neglible. --- src/librustc_interface/passes.rs | 4 ---- src/librustc_interface/queries.rs | 14 +++++++++++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 24b44964e4fd2..200da05c57561 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -223,7 +223,6 @@ pub struct PluginInfo { } pub fn register_plugins<'a>( - compiler: &Compiler, sess: &'a Session, cstore: &'a CStore, mut krate: ast::Crate, @@ -261,9 +260,6 @@ pub fn register_plugins<'a>( }); } - // If necessary, compute the dependency graph (in the background). - compiler.dep_graph_future().ok(); - time(sess, "recursion limit", || { middle::recursion_limit::update_limits(sess, &krate); }); diff --git a/src/librustc_interface/queries.rs b/src/librustc_interface/queries.rs index c281bc5360704..e056d3feb66ec 100644 --- a/src/librustc_interface/queries.rs +++ b/src/librustc_interface/queries.rs @@ -114,13 +114,21 @@ impl Compiler { let crate_name = self.crate_name()?.peek().clone(); let krate = self.parse()?.take(); - passes::register_plugins( - self, + let result = passes::register_plugins( self.session(), self.cstore(), krate, &crate_name, - ) + ); + + // Compute the dependency graph (in the background). We want to do + // this as early as possible, to give the DepGraph maximum time to + // load before dep_graph() is called, but it also can't happen + // until after rustc_incremental::prepare_session_directory() is + // called, which happens within passes::register_plugins(). + self.dep_graph_future().ok(); + + result }) } From 25211894386a34db1639fbd69680e8f7b35ee1a4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 30 Aug 2019 17:27:35 +1000 Subject: [PATCH 4/4] Add a comment to `Compiler::compile()`. `Compiler::compile()` is different to all the other `Compiler` methods because it lacks a `Queries` entry. It only has one call site, which is in a test that doesn't need its specific characteristics. This patch replaces that call with a call to `Compile::link()`, which is similar enough for the test's purposes. It also notes that the method is an illustrative example of how `Compiler` can be used. --- src/librustc_interface/queries.rs | 9 +++++++-- src/test/run-make-fulldeps/issue-19371/foo.rs | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/librustc_interface/queries.rs b/src/librustc_interface/queries.rs index e056d3feb66ec..ff5cd8b8c695d 100644 --- a/src/librustc_interface/queries.rs +++ b/src/librustc_interface/queries.rs @@ -275,6 +275,11 @@ impl Compiler { }) } + // This method is different to all the other methods in `Compiler` because + // it lacks a `Queries` entry. It's also not currently used. It does serve + // as an example of how `Compiler` can be used, with additional steps added + // between some passes. And see `rustc_driver::run_compiler` for a more + // complex example. pub fn compile(&self) -> Result<()> { self.prepare_outputs()?; @@ -286,12 +291,12 @@ impl Compiler { self.global_ctxt()?; - // Drop AST after creating GlobalCtxt to free memory + // Drop AST after creating GlobalCtxt to free memory. mem::drop(self.expansion()?.take()); self.ongoing_codegen()?; - // Drop GlobalCtxt after starting codegen to free memory + // Drop GlobalCtxt after starting codegen to free memory. mem::drop(self.global_ctxt()?.take()); self.link().map(|_| ()) diff --git a/src/test/run-make-fulldeps/issue-19371/foo.rs b/src/test/run-make-fulldeps/issue-19371/foo.rs index afc92638fda97..e290f7fa6b13a 100644 --- a/src/test/run-make-fulldeps/issue-19371/foo.rs +++ b/src/test/run-make-fulldeps/issue-19371/foo.rs @@ -62,6 +62,7 @@ fn compile(code: String, output: PathBuf, sysroot: PathBuf) { }; interface::run_compiler(config, |compiler| { - compiler.compile().ok(); + // This runs all the passes prior to linking, too. + compiler.link().ok(); }); }