Simplify BuildResult enum

This removes the BuildResult::Failure variant. It was used
interchangeably with BuildResult::Success, and only returned in rustc()
routine when executed in-process compiler panicked or returned with a
non-zero exit code.

Since internally the compiler driver also panics on compile failure, the
BuildResult::{Failure, Success} variant distinction didn't give any
meaningful information, as the driver treats ICEs and regular compile
failures the same way. In both cases BuildResult::Failure was returned.

More correct solution would be not to panic on regular compile failure
in run_compiler, so the caller can either catch a panic and deduce that
an ICE was encountered, or receive a non-zero (101) exit code in case
errors were encountered.
Then a BuildResult::Err variant would be returned on an ICE, or
BuildResult::Success otherwise.

Author: Xanewok

src/actions/post_build.rs

@@ -49,8 +49,7 @@ impl<O: Output> PostBuildHandler<O> {
         ));
 
         match result {
-            BuildResult::Success(messages, new_analysis)
-            | BuildResult::Failure(messages, new_analysis) => {
+            BuildResult::Success(messages, new_analysis) => {
                 thread::spawn(move || {
                     trace!("build - Success");
 

src/build/cargo.rs

@@ -476,8 +476,7 @@ impl Executor for RlsExecutor {
                 self.config.clone(),
                 env_lock,
             ) {
-                BuildResult::Success(mut messages, mut analysis)
-                | BuildResult::Failure(mut messages, mut analysis) => {
+                BuildResult::Success(mut messages, mut analysis) => {
                     self.compiler_messages.lock().unwrap().append(&mut messages);
                     self.analysis.lock().unwrap().append(&mut analysis);
                 }

src/build/mod.rs

@@ -92,10 +92,9 @@ struct Internals {
 /// The result of a build request.
 #[derive(Debug)]
 pub enum BuildResult {
-    /// Build was successful, argument is warnings.
+    /// Build was performed without any internal errors. The payload
+    /// contains emitted raw diagnostics and Analysis data.
     Success(Vec<String>, Vec<Analysis>),
-    /// Build finished with errors, argument is errors and warnings.
-    Failure(Vec<String>, Vec<Analysis>),
     /// Build was coalesced with another build.
     Squashed,
     /// There was an error attempting to build.
@@ -421,7 +420,7 @@ impl Internals {
         // now. It's possible that a build was scheduled with given files, but
         // user later changed them. These should still be left as dirty (not built).
         match *&result {
-            BuildResult::Success(_, _) | BuildResult::Failure(_, _) => {
+            BuildResult::Success(_, _) => {
                 let mut dirty_files = self.dirty_files.lock().unwrap();
                 dirty_files.retain(|file, dirty_version| {
                     built_files

src/build/plan.rs

@@ -361,17 +361,16 @@ impl JobQueue {
                 internals.config.clone(),
                 internals.env_lock.as_facade(),
             ) {
-                BuildResult::Success(mut messages, mut analysis)
-                | BuildResult::Failure(mut messages, mut analysis) => {
+                BuildResult::Success(mut messages, mut analysis) => {
                     compiler_messages.append(&mut messages);
                     analyses.append(&mut analysis);
                 }
                 BuildResult::Err => return BuildResult::Err,
                 _ => {}
             }
         }
-        // TODO: Should we combine with ::Failure? What's the difference between those two?
-        return BuildResult::Success(compiler_messages, analyses);
+
+        BuildResult::Success(compiler_messages, analyses)
     }
 }
 

src/build/rustc.rs

@@ -74,7 +74,11 @@ pub fn rustc(
 
     let mut controller = RlsRustcCalls::new(analysis.clone());
 
-    let exit_code = ::std::panic::catch_unwind(|| {
+    // rustc explicitly panics in run_compiler() on compile failure, regardless
+    // if it encounters an ICE (internal compiler error) or not.
+    // TODO: Change librustc_driver behaviour to distinguish between ICEs and
+    // regular compilation failure with errors?
+    let _ = ::std::panic::catch_unwind(|| {
         run(move || {
             // Replace stderr so we catch most errors.
             run_compiler(
@@ -94,10 +98,8 @@ pub fn rustc(
 
     let analysis = analysis.lock().unwrap().clone();
     let analysis = analysis.map(|analysis| vec![analysis]).unwrap_or(vec![]);
-    match exit_code {
-        Ok(0) => BuildResult::Success(stderr_json_msgs, analysis),
-        _ => BuildResult::Failure(stderr_json_msgs, analysis),
-    }
+
+    BuildResult::Success(stderr_json_msgs, analysis)
 }
 
 // Our compiler controller. We mostly delegate to the default rustc