Initial work towards abort-free compilation

Nick Cameron · Nick Cameron · commit 0b511e82abe4 · 2016-01-26T17:51:11.000+13:00
The goal is that the compiler will pass `Result`s around rather than using abort_if_errors. To preserve behaviour we currently abort at the top level. I've removed all other aborts from the driver, but haven't touched any of the nested aborts.
diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs
@@ -176,15 +176,28 @@ impl Session {
     pub fn abort_if_errors(&self) {
         self.diagnostic().abort_if_errors();
     }
-    pub fn abort_if_new_errors<F, T>(&self, f: F) -> T
+    pub fn track_errors<F, T>(&self, f: F) -> Result<T, usize>
         where F: FnOnce() -> T
     {
-        let count = self.err_count();
+        let mut count = self.err_count();
         let result = f();
-        if self.err_count() > count {
-            self.abort_if_errors();
+        count -= self.err_count();
+        if count == 0 {
+            Ok(result)
+        } else {
+            Err(count)
+        }
+    }
+    pub fn abort_if_new_errors<F, T>(&self, f: F) -> T
+        where F: FnOnce() -> T
+    {
+        match self.track_errors(f) {
+            Ok(result) => result,
+            Err(_) => {
+                self.abort_if_errors();
+                unreachable!();
+            }
         }
-        result
     }
     pub fn span_warn(&self, sp: Span, msg: &str) {
         self.diagnostic().span_warn(sp, msg)
diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs
diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs
@@ -104,57 +104,103 @@ pub mod target_features;
 const BUG_REPORT_URL: &'static str = "https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.\
                                       md#bug-reports";
 
+// Err(0) means compilation was stopped, but no errors were found.
+// This would be better as a dedicated enum, but using try! is so convenient.
+pub type CompileResult = Result<(), usize>;
+
+pub fn compile_result_from_err_count(err_count: usize) -> CompileResult {
+    if err_count == 0 {
+        Ok(())
+    } else {
+        Err(err_count)
+    }
+}
+
+#[inline]
+fn abort_msg(err_count: usize) -> String {
+    match err_count {
+        0 => "aborting with no errors (maybe a bug?)".to_owned(),
+        1 => "aborting due to previous error".to_owned(),
+        e => format!("aborting due to {} previous errors", e),
+    }
+}
+
+pub fn abort_on_err<T>(result: Result<T, usize>, sess: &Session) -> T {
+    match result {
+        Err(err_count) => {
+            sess.fatal(&abort_msg(err_count));
+        }
+        Ok(x) => x,
+    }
+}
+
 pub fn run(args: Vec<String>) -> isize {
-    monitor(move || run_compiler(&args, &mut RustcDefaultCalls));
+    monitor(move || {
+        let (result, session) = run_compiler(&args, &mut RustcDefaultCalls);
+        if let Err(err_count) = result {
+            if err_count > 0 {
+                match session {
+                    Some(sess) => sess.fatal(&abort_msg(err_count)),
+                    None => {
+                        let mut emitter =
+                            errors::emitter::BasicEmitter::stderr(errors::ColorConfig::Auto);
+                        emitter.emit(None, &abort_msg(err_count), None, errors::Level::Fatal);
+                        panic!(errors::FatalError);
+                    }
+                }
+            }
+        }
+    });
     0
 }
 
 // Parse args and run the compiler. This is the primary entry point for rustc.
 // See comments on CompilerCalls below for details about the callbacks argument.
-pub fn run_compiler<'a>(args: &[String], callbacks: &mut CompilerCalls<'a>) {
-    macro_rules! do_or_return {($expr: expr) => {
+pub fn run_compiler<'a>(args: &[String],
+                        callbacks: &mut CompilerCalls<'a>)
+                        -> (CompileResult, Option<Session>) {
+    macro_rules! do_or_return {($expr: expr, $sess: expr) => {
         match $expr {
-            Compilation::Stop => return,
+            Compilation::Stop => return (Ok(()), $sess),
             Compilation::Continue => {}
         }
     }}
 
     let matches = match handle_options(args.to_vec()) {
         Some(matches) => matches,
-        None => return,
+        None => return (Ok(()), None),
     };
 
     let sopts = config::build_session_options(&matches);
 
     let descriptions = diagnostics_registry();
 
-    do_or_return!(callbacks.early_callback(&matches, &descriptions, sopts.error_format));
+    do_or_return!(callbacks.early_callback(&matches, &descriptions, sopts.error_format), None);
 
     let (odir, ofile) = make_output(&matches);
     let (input, input_file_path) = match make_input(&matches.free) {
         Some((input, input_file_path)) => callbacks.some_input(input, input_file_path),
         None => match callbacks.no_input(&matches, &sopts, &odir, &ofile, &descriptions) {
             Some((input, input_file_path)) => (input, input_file_path),
-            None => return,
+            None => return (Ok(()), None),
         },
     };
 
     let cstore = Rc::new(CStore::new(token::get_ident_interner()));
-    let sess = build_session(sopts, input_file_path, descriptions,
-                                 cstore.clone());
+    let sess = build_session(sopts, input_file_path, descriptions, cstore.clone());
     rustc_lint::register_builtins(&mut sess.lint_store.borrow_mut(), Some(&sess));
     let mut cfg = config::build_configuration(&sess);
     target_features::add_configuration(&mut cfg, &sess);
 
-    do_or_return!(callbacks.late_callback(&matches, &sess, &input, &odir, &ofile));
+    do_or_return!(callbacks.late_callback(&matches, &sess, &input, &odir, &ofile), Some(sess));
 
     // It is somewhat unfortunate that this is hardwired in - this is forced by
     // the fact that pretty_print_input requires the session by value.
     let pretty = callbacks.parse_pretty(&sess, &matches);
     match pretty {
         Some((ppm, opt_uii)) => {
             pretty::pretty_print_input(sess, &cstore, cfg, &input, ppm, opt_uii, ofile);
-            return;
+            return (Ok(()), None);
         }
         None => {
             // continue
@@ -163,8 +209,9 @@ pub fn run_compiler<'a>(args: &[String], callbacks: &mut CompilerCalls<'a>) {
 
     let plugins = sess.opts.debugging_opts.extra_plugins.clone();
     let control = callbacks.build_controller(&sess);
-    driver::compile_input(sess, &cstore, cfg, &input, &odir, &ofile,
-                          Some(plugins), control);
+    (driver::compile_input(&sess, &cstore, cfg, &input, &odir, &ofile,
+                           Some(plugins), control),
+     Some(sess))
 }
 
 // Extract output directory and file from matches.
diff --git a/src/librustc_driver/pretty.rs b/src/librustc_driver/pretty.rs
@@ -17,7 +17,7 @@ use self::NodesMatchingUII::*;
 
 use rustc_trans::back::link;
 
-use driver;
+use {driver, abort_on_err};
 
 use rustc::middle::ty;
 use rustc::middle::cfg;
@@ -194,21 +194,21 @@ impl PpSourceMode {
                 f(&annotation, payload, &ast_map.forest.krate)
             }
             PpmTyped => {
-                driver::phase_3_run_analysis_passes(sess,
-                                                    cstore,
-                                                    ast_map.clone(),
-                                                    arenas,
-                                                    id,
-                                                    resolve::MakeGlobMap::No,
-                                                    |tcx, _, _| {
-                                                        let annotation = TypedAnnotation {
-                                                            tcx: tcx,
-                                                        };
-                                                        let _ignore = tcx.dep_graph.in_ignore();
-                                                        f(&annotation,
-                                                          payload,
-                                                          &ast_map.forest.krate)
-                                                    })
+                abort_on_err(driver::phase_3_run_analysis_passes(sess,
+                                                                 cstore,
+                                                                 ast_map.clone(),
+                                                                 arenas,
+                                                                 id,
+                                                                 resolve::MakeGlobMap::No,
+                                                                 |tcx, _, _| {
+                    let annotation = TypedAnnotation {
+                        tcx: tcx,
+                    };
+                    let _ignore = tcx.dep_graph.in_ignore();
+                    f(&annotation,
+                      payload,
+                      &ast_map.forest.krate)
+                }), sess)
             }
             _ => panic!("Should use call_with_pp_support"),
         }
@@ -694,8 +694,8 @@ pub fn pretty_print_input(sess: Session,
     let compute_ast_map = needs_ast_map(&ppm, &opt_uii);
     let krate = if compute_ast_map {
         match driver::phase_2_configure_and_expand(&sess, &cstore, krate, &id[..], None) {
-            None => return,
-            Some(k) => driver::assign_node_ids(&sess, k),
+            Err(_) => return,
+            Ok(k) => driver::assign_node_ids(&sess, k),
         }
     } else {
         krate
@@ -818,19 +818,19 @@ pub fn pretty_print_input(sess: Session,
             match code {
                 Some(code) => {
                     let variants = gather_flowgraph_variants(&sess);
-                    driver::phase_3_run_analysis_passes(&sess,
-                                                        &cstore,
-                                                        ast_map,
-                                                        &arenas,
-                                                        &id,
-                                                        resolve::MakeGlobMap::No,
-                                                        |tcx, _, _| {
-                                                            print_flowgraph(variants,
-                                                                            tcx,
-                                                                            code,
-                                                                            mode,
-                                                                            out)
-                                                        })
+                    abort_on_err(driver::phase_3_run_analysis_passes(&sess,
+                                                                     &cstore,
+                                                                     ast_map,
+                                                                     &arenas,
+                                                                     &id,
+                                                                     resolve::MakeGlobMap::No,
+                                                                     |tcx, _, _| {
+                        print_flowgraph(variants,
+                                        tcx,
+                                        code,
+                                        mode,
+                                        out)
+                    }), &sess)
                 }
                 None => {
                     let message = format!("--pretty=flowgraph needs block, fn, or method; got \
diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs
@@ -10,7 +10,7 @@
 pub use self::MaybeTyped::*;
 
 use rustc_lint;
-use rustc_driver::{driver, target_features};
+use rustc_driver::{driver, target_features, abort_on_err};
 use rustc::session::{self, config};
 use rustc::middle::def_id::DefId;
 use rustc::middle::privacy::AccessLevels;
@@ -147,13 +147,13 @@ pub fn run_core(search_paths: SearchPaths, cfgs: Vec<String>, externs: Externs,
     let arenas = ty::CtxtArenas::new();
     let hir_map = driver::make_map(&sess, &mut hir_forest);
 
-    driver::phase_3_run_analysis_passes(&sess,
-                                        &cstore,
-                                        hir_map,
-                                        &arenas,
-                                        &name,
-                                        resolve::MakeGlobMap::No,
-                                        |tcx, _, analysis| {
+    abort_on_err(driver::phase_3_run_analysis_passes(&sess,
+                                                     &cstore,
+                                                     hir_map,
+                                                     &arenas,
+                                                     &name,
+                                                     resolve::MakeGlobMap::No,
+                                                     |tcx, _, analysis| {
         let _ignore = tcx.dep_graph.in_ignore();
         let ty::CrateAnalysis { access_levels, .. } = analysis;
 
@@ -200,5 +200,5 @@ pub fn run_core(search_paths: SearchPaths, cfgs: Vec<String>, externs: Externs,
         *analysis.inlined.borrow_mut() = map;
         analysis.deref_trait_did = ctxt.deref_trait_did.get();
         (krate, analysis)
-    })
+    }), &sess)
 }
diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs
@@ -250,7 +250,12 @@ fn runtest(test: &str, cratename: &str, cfgs: Vec<String>, libs: SearchPaths,
     if no_run {
         control.after_analysis.stop = Compilation::Stop;
     }
-    driver::compile_input(sess, &cstore, cfg, &input, &out, &None, None, control);
+    let result = driver::compile_input(&sess, &cstore, cfg, &input,
+                                       &out, &None, None, control);
+    match result {
+        Err(count) if count > 0 => sess.fatal("aborting due to previous error(s)"),
+        _ => {}
+    }
 
     if no_run { return }
 
diff --git a/src/test/run-make/execution-engine/test.rs b/src/test/run-make/execution-engine/test.rs
@@ -32,7 +32,7 @@ use rustc::middle::cstore::{CrateStore, LinkagePreference};
 use rustc::middle::ty;
 use rustc::session::config::{self, basic_options, build_configuration, Input, Options};
 use rustc::session::build_session;
-use rustc_driver::driver;
+use rustc_driver::{driver, abort_on_err};
 use rustc_front::lowering::{lower_crate, LoweringContext};
 use rustc_resolve::MakeGlobMap;
 use rustc_metadata::cstore::CStore;
@@ -234,7 +234,7 @@ fn compile_program(input: &str, sysroot: PathBuf)
         let arenas = ty::CtxtArenas::new();
         let ast_map = driver::make_map(&sess, &mut hir_forest);
 
-        driver::phase_3_run_analysis_passes(
+        abort_on_err(driver::phase_3_run_analysis_passes(
             &sess, &cstore, ast_map, &arenas, &id,
             MakeGlobMap::No, |tcx, mir_map, analysis| {
 
@@ -254,7 +254,7 @@ fn compile_program(input: &str, sysroot: PathBuf)
             let modp = llmod as usize;
 
             (modp, deps)
-        })
+        }), &sess)
     }).unwrap();
 
     match handle.join() {
diff --git a/src/test/run-make/issue-19371/foo.rs b/src/test/run-make/issue-19371/foo.rs
@@ -65,7 +65,7 @@ fn compile(code: String, output: PathBuf, sysroot: PathBuf) {
     let cfg = build_configuration(&sess);
     let control = CompileController::basic();
 
-    compile_input(sess, &cstore,
+    compile_input(&sess, &cstore,
             cfg,
             &Input::Str(code),
             &None,
