Auto merge of #34480 - jseyfried:remove_entry_points, r=nrc

Remove redundant `CompileController` entry points

Remove the `after_expand` and `after_write_deps` `CompileController` entry points.

The only things that separate these entry points from `after_hir_lowering` are dep-info generation and HIR map construction, neither of which is computationally intensive or has the potential to error.

r? @nrc

src/librustc_driver/driver.rs

@@ -116,34 +116,20 @@ pub fn compile_input(sess: &Session,
         let outputs = build_output_filenames(input, outdir, output, &krate.attrs, sess);
         let id = link::find_crate_name(Some(sess), &krate.attrs, input);
         let ExpansionResult { expanded_crate, defs, analysis, resolutions, mut hir_forest } = {
-            let make_glob_map = control.make_glob_map;
+            phase_2_configure_and_expand(
-            phase_2_configure_and_expand(sess, &cstore, krate, &id, addl_plugins, make_glob_map)?
+                sess, &cstore, krate, &id, addl_plugins, control.make_glob_map,
+                |expanded_crate| {
+                    let mut state = CompileState::state_after_expand(
+                        input, sess, outdir, output, &cstore, expanded_crate, &id,
+                    );
+                    controller_entry_point!(after_expand, sess, state, Ok(()));
+                    Ok(())
+                }
+            )?
         };
 
-        controller_entry_point!(after_expand,
-                                sess,
-                                CompileState::state_after_expand(input,
-                                                                 sess,
-                                                                 outdir,
-                                                                 output,
-                                                                 &cstore,
-                                                                 &expanded_crate,
-                                                                 &id),
-                                Ok(()));
-
         write_out_deps(sess, &outputs, &id);
 
-        controller_entry_point!(after_write_deps,
-                                sess,
-                                CompileState::state_after_write_deps(input,
-                                                                     sess,
-                                                                     outdir,
-                                                                     output,
-                                                                     &cstore,
-                                                                     &expanded_crate,
-                                                                     &id),
-                                Ok(()));
-
         let arenas = ty::CtxtArenas::new();
 
         // Construct the HIR map
@@ -285,7 +271,6 @@ pub fn source_name(input: &Input) -> String {
 pub struct CompileController<'a> {
     pub after_parse: PhaseController<'a>,
     pub after_expand: PhaseController<'a>,
-    pub after_write_deps: PhaseController<'a>,
     pub after_hir_lowering: PhaseController<'a>,
     pub after_analysis: PhaseController<'a>,
     pub after_llvm: PhaseController<'a>,
@@ -298,7 +283,6 @@ impl<'a> CompileController<'a> {
         CompileController {
             after_parse: PhaseController::basic(),
             after_expand: PhaseController::basic(),
-            after_write_deps: PhaseController::basic(),
             after_hir_lowering: PhaseController::basic(),
             after_analysis: PhaseController::basic(),
             after_llvm: PhaseController::basic(),
@@ -406,23 +390,6 @@ impl<'a, 'b, 'ast, 'tcx> CompileState<'a, 'b, 'ast, 'tcx> {
         }
     }
 
-    fn state_after_write_deps(input: &'a Input,
-                              session: &'ast Session,
-                              out_dir: &'a Option<PathBuf>,
-                              out_file: &'a Option<PathBuf>,
-                              cstore: &'a CStore,
-                              krate: &'a ast::Crate,
-                              crate_name: &'a str)
-                              -> CompileState<'a, 'b, 'ast, 'tcx> {
-        CompileState {
-            crate_name: Some(crate_name),
-            cstore: Some(cstore),
-            expanded_crate: Some(krate),
-            out_file: out_file.as_ref().map(|s| &**s),
-            ..CompileState::empty(input, session, out_dir)
-        }
-    }
-
     fn state_after_hir_lowering(input: &'a Input,
                                 session: &'ast Session,
                                 out_dir: &'a Option<PathBuf>,
@@ -556,13 +523,16 @@ pub struct ExpansionResult<'a> {
 /// standard library and prelude, and name resolution.
 ///
 /// Returns `None` if we're aborting after handling -W help.
-pub fn phase_2_configure_and_expand<'a>(sess: &Session,
+pub fn phase_2_configure_and_expand<'a, F>(sess: &Session,
-                                        cstore: &CStore,
+                                           cstore: &CStore,
-                                        mut krate: ast::Crate,
+                                           mut krate: ast::Crate,
-                                        crate_name: &'a str,
+                                           crate_name: &'a str,
-                                        addl_plugins: Option<Vec<String>>,
+                                           addl_plugins: Option<Vec<String>>,
-                                        make_glob_map: MakeGlobMap)
+                                           make_glob_map: MakeGlobMap,
-                                        -> Result<ExpansionResult<'a>, usize> {
+                                           after_expand: F)
+                                           -> Result<ExpansionResult<'a>, usize>
+    where F: FnOnce(&ast::Crate) -> CompileResult,
+{
     let time_passes = sess.time_passes();
 
     // strip before anything else because crate metadata may use #[cfg_attr]
@@ -745,9 +715,23 @@ pub fn phase_2_configure_and_expand<'a>(sess: &Session,
          "AST validation",
          || ast_validation::check_crate(sess, &krate));
 
-    time(sess.time_passes(), "name resolution", || {
+    time(sess.time_passes(), "name resolution", || -> CompileResult {
+        // Currently, we ignore the name resolution data structures for the purposes of dependency
+        // tracking. Instead we will run name resolution and include its output in the hash of each
+        // item, much like we do for macro expansion. In other words, the hash reflects not just
+        // its contents but the results of name resolution on those contents. Hopefully we'll push
+        // this back at some point.
+        let _ignore = sess.dep_graph.in_ignore();
+        resolver.build_reduced_graph(&krate);
+        resolver.resolve_imports();
+
+        // Since import resolution will eventually happen in expansion,
+        // don't perform `after_expand` until after import resolution.
+        after_expand(&krate)?;
+
         resolver.resolve_crate(&krate);
-    });
+        Ok(())
+    })?;
 
     // Lower ast -> hir.
     let hir_forest = time(sess.time_passes(), "lowering ast -> hir", || {

src/librustc_driver/lib.rs

@@ -511,7 +511,7 @@ impl<'a> CompilerCalls<'a> for RustcDefaultCalls {
         }
 
         if sess.opts.no_analysis || sess.opts.debugging_opts.ast_json {
-            control.after_write_deps.stop = Compilation::Stop;
+            control.after_hir_lowering.stop = Compilation::Stop;
         }
 
         if !sess.opts.output_types.keys().any(|&i| i == OutputType::Exe) {

src/librustc_driver/test.rs

@@ -116,9 +116,11 @@ fn test_env<F>(source_string: &str,
         input: source_string.to_string(),
     };
     let krate = driver::phase_1_parse_input(&sess, krate_config, &input).unwrap();
-    let driver::ExpansionResult { defs, resolutions, mut hir_forest, .. } =
+    let driver::ExpansionResult { defs, resolutions, mut hir_forest, .. } = {
-        driver::phase_2_configure_and_expand(&sess, &cstore, krate, "test", None, MakeGlobMap::No)
+        driver::phase_2_configure_and_expand(
-            .expect("phase 2 aborted");
+            &sess, &cstore, krate, "test", None, MakeGlobMap::No, |_| Ok(()),
+        ).expect("phase 2 aborted")
+    };
     let _ignore = dep_graph.in_ignore();
 
     let arenas = ty::CtxtArenas::new();

src/librustc_resolve/lib.rs

@@ -1167,18 +1167,6 @@ impl<'a> Resolver<'a> {
 
     /// Entry point to crate resolution.
     pub fn resolve_crate(&mut self, krate: &Crate) {
-        // Currently, we ignore the name resolution data structures for
-        // the purposes of dependency tracking. Instead we will run name
-        // resolution and include its output in the hash of each item,
-        // much like we do for macro expansion. In other words, the hash
-        // reflects not just its contents but the results of name
-        // resolution on those contents. Hopefully we'll push this back at
-        // some point.
-        let _ignore = self.session.dep_graph.in_ignore();
-
-        self.build_reduced_graph(krate);
-        resolve_imports::resolve_imports(self);
-
         self.current_module = self.graph_root;
         visit::walk_crate(self, krate);
 

src/librustc_resolve/resolve_imports.rs

@@ -30,6 +30,12 @@ use syntax_pos::{Span, DUMMY_SP};
 
 use std::cell::{Cell, RefCell};
 
+impl<'a> Resolver<'a> {
+    pub fn resolve_imports(&mut self) {
+        ImportResolver { resolver: self }.resolve_imports();
+    }
+}
+
 /// Contains data for specific types of import directives.
 #[derive(Clone, Debug)]
 pub enum ImportDirectiveSubclass {
@@ -722,8 +728,3 @@ fn import_directive_subclass_to_string(subclass: &ImportDirectiveSubclass) -> St
         GlobImport { .. } => "*".to_string(),
     }
 }
-
-pub fn resolve_imports(resolver: &mut Resolver) {
-    let mut import_resolver = ImportResolver { resolver: resolver };
-    import_resolver.resolve_imports();
-}

src/librustdoc/core.rs

@@ -149,9 +149,9 @@ pub fn run_core(search_paths: SearchPaths,
     let name = link::find_crate_name(Some(&sess), &krate.attrs, &input);
 
     let driver::ExpansionResult { defs, analysis, resolutions, mut hir_forest, .. } = {
-        let make_glob_map = resolve::MakeGlobMap::No;
+        driver::phase_2_configure_and_expand(
-        driver::phase_2_configure_and_expand(&sess, &cstore, krate, &name, None, make_glob_map)
+            &sess, &cstore, krate, &name, None, resolve::MakeGlobMap::No, |_| Ok(()),
-            .expect("phase_2_configure_and_expand aborted in rustdoc!")
+        ).expect("phase_2_configure_and_expand aborted in rustdoc!")
     };
 
     let arenas = ty::CtxtArenas::new();

src/librustdoc/test.rs

@@ -95,9 +95,9 @@ pub fn run(input: &str,
     cfg.extend(config::parse_cfgspecs(cfgs.clone()));
     let krate = panictry!(driver::phase_1_parse_input(&sess, cfg, &input));
     let driver::ExpansionResult { defs, mut hir_forest, .. } = {
-        let make_glob_map = MakeGlobMap::No;
+        phase_2_configure_and_expand(
-        phase_2_configure_and_expand(&sess, &cstore, krate, "rustdoc-test", None, make_glob_map)
+            &sess, &cstore, krate, "rustdoc-test", None, MakeGlobMap::No, |_| Ok(())
-            .expect("phase_2_configure_and_expand aborted in rustdoc!")
+        ).expect("phase_2_configure_and_expand aborted in rustdoc!")
     };
 
     let dep_graph = DepGraph::new(false);

src/test/run-make/execution-engine/test.rs

@@ -241,8 +241,9 @@ fn compile_program(input: &str, sysroot: PathBuf)
         let krate = panictry!(driver::phase_1_parse_input(&sess, cfg, &input));
 
         let driver::ExpansionResult { defs, analysis, resolutions, mut hir_forest, .. } = {
-            driver::phase_2_configure_and_expand(&sess, &cstore, krate, &id, None, MakeGlobMap::No)
+            driver::phase_2_configure_and_expand(
-                .expect("phase_2 returned `None`")
+                &sess, &cstore, krate, &id, None, MakeGlobMap::No, |_| Ok(()),
+            ).expect("phase_2 returned `None`")
         };
 
         let arenas = ty::CtxtArenas::new();