Unrolled build for rust-lang#121205

rust-timer · web-flow · commit 8341a7f60da5 · 2024-02-20T04:02:30.000-05:00
Rollup merge of rust-lang#121205 - nnethercote:fix-stable-mir-CompilerError, r=oli-obk Merge `CompilerError::CompilationFailed` and `CompilerError::ICE`. `CompilerError` has `CompilationFailed` and `ICE` variants, which seems reasonable at first. But the way it identifies them is flawed: - If compilation errors out, i.e. `RunCompiler::run` returns an `Err`, it uses `CompilationFailed`, which is reasonable. - If compilation panics with `FatalError`, it catches the panic and uses `ICE`. This is sometimes right, because ICEs do cause `FatalError` panics, but sometimes wrong, because certain compiler errors also cause `FatalError` panics. (The compiler/rustdoc/clippy/whatever just catches the `FatalError` with `catch_with_exit_code` in `main`.) In other words, certain non-ICE compilation failures get miscategorized as ICEs. It's not possible to reliably distinguish the two cases, so this commit merges them. It also renames the combined variant as just `Failed`, to better match the existing `Interrupted` and `Skipped` variants. Here is an example of a non-ICE failure that causes a `FatalError` panic, from `tests/ui/recursion_limit/issue-105700.rs`: ``` #![recursion_limit="4"] #![invalid_attribute] #![invalid_attribute] #![invalid_attribute] #![invalid_attribute] #![invalid_attribute] //~^ERROR recursion limit reached while expanding fn main() {{}} ``` r? ``@spastorino``
diff --git a/compiler/rustc_smir/src/rustc_internal/mod.rs b/compiler/rustc_smir/src/rustc_internal/mod.rs
@@ -347,8 +347,14 @@ macro_rules! run_driver {
                         Err(CompilerError::Interrupted(value))
                     }
                     (Ok(Ok(_)), None) => Err(CompilerError::Skipped),
-                    (Ok(Err(_)), _) => Err(CompilerError::CompilationFailed),
-                    (Err(_), _) => Err(CompilerError::ICE),
+                    // Two cases here:
+                    // - `run` finished normally and returned `Err`
+                    // - `run` panicked with `FatalErr`
+                    // You might think that normal compile errors cause the former, and
+                    // ICEs cause the latter. But some normal compiler errors also cause
+                    // the latter. So we can't meaningfully distinguish them, and group
+                    // them together.
+                    (Ok(Err(_)), _) | (Err(_), _) => Err(CompilerError::Failed),
                 }
             }
         }
diff --git a/compiler/stable_mir/src/error.rs b/compiler/stable_mir/src/error.rs
@@ -15,10 +15,8 @@ macro_rules! error {
 /// An error type used to represent an error that has already been reported by the compiler.
 #[derive(Clone, Copy, PartialEq, Eq)]
 pub enum CompilerError<T> {
-    /// Internal compiler error (I.e.: Compiler crashed).
-    ICE,
-    /// Compilation failed.
-    CompilationFailed,
+    /// Compilation failed, either due to normal errors or ICE.
+    Failed,
     /// Compilation was interrupted.
     Interrupted(T),
     /// Compilation skipped. This happens when users invoke rustc to retrieve information such as
@@ -54,8 +52,7 @@ where
 {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         match self {
-            CompilerError::ICE => write!(f, "Internal Compiler Error"),
-            CompilerError::CompilationFailed => write!(f, "Compilation Failed"),
+            CompilerError::Failed => write!(f, "Compilation Failed"),
             CompilerError::Interrupted(reason) => write!(f, "Compilation Interrupted: {reason}"),
             CompilerError::Skipped => write!(f, "Compilation Skipped"),
         }
@@ -68,8 +65,7 @@ where
 {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         match self {
-            CompilerError::ICE => write!(f, "Internal Compiler Error"),
-            CompilerError::CompilationFailed => write!(f, "Compilation Failed"),
+            CompilerError::Failed => write!(f, "Compilation Failed"),
             CompilerError::Interrupted(reason) => write!(f, "Compilation Interrupted: {reason:?}"),
             CompilerError::Skipped => write!(f, "Compilation Skipped"),
         }
diff --git a/tests/ui-fulldeps/stable-mir/compilation-result.rs b/tests/ui-fulldeps/stable-mir/compilation-result.rs
@@ -55,7 +55,7 @@ fn test_skipped(mut args: Vec<String>) {
 fn test_failed(mut args: Vec<String>) {
     args.push("--cfg=broken".to_string());
     let result = run!(args, || unreachable!() as ControlFlow<()>);
-    assert_eq!(result, Err(stable_mir::CompilerError::CompilationFailed));
+    assert_eq!(result, Err(stable_mir::CompilerError::Failed));
 }
 
 /// Test that we are able to pass a closure and set the return according to the captured value.

Original file line number	Diff line number	Diff line change
`@@ -347,8 +347,14 @@ macro_rules! run_driver {`
`347`	`347`	`Err(CompilerError::Interrupted(value))`
`348`	`348`	`}`
`349`	`349`	`(Ok(Ok(_)), None) => Err(CompilerError::Skipped),`
`350`		`- (Ok(Err(_)), _) => Err(CompilerError::CompilationFailed),`
`351`		`- (Err(_), _) => Err(CompilerError::ICE),`
	`350`	`+ // Two cases here:`
	`351`	+ // - `run` finished normally and returned `Err`
	`352`	+ // - `run` panicked with `FatalErr`
	`353`	`+ // You might think that normal compile errors cause the former, and`
	`354`	`+ // ICEs cause the latter. But some normal compiler errors also cause`
	`355`	`+ // the latter. So we can't meaningfully distinguish them, and group`
	`356`	`+ // them together.`
	`357`	`+ (Ok(Err(_)), _) \| (Err(_), _) => Err(CompilerError::Failed),`
`352`	`358`	`}`
`353`	`359`	`}`
`354`	`360`	`}`
Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,7 @@ fn test_skipped(mut args: Vec<String>) {`
`55`	`55`	`fn test_failed(mut args: Vec<String>) {`
`56`	`56`	`args.push("--cfg=broken".to_string());`
`57`	`57`	`let result = run!(args, \|\| unreachable!() as ControlFlow<()>);`
`58`		`- assert_eq!(result, Err(stable_mir::CompilerError::CompilationFailed));`
	`58`	`+ assert_eq!(result, Err(stable_mir::CompilerError::Failed));`
`59`	`59`	`}`
`60`	`60`
`61`	`61`	`/// Test that we are able to pass a closure and set the return according to the captured value.`