diff options
| author | Nicholas Nethercote <n.nethercote@gmail.com> | 2024-02-17 09:13:45 +1100 |
|---|---|---|
| committer | Nicholas Nethercote <n.nethercote@gmail.com> | 2024-02-17 09:40:44 +1100 |
| commit | e72e7e9ae3a6938f1a926ca10f693d89948134cb (patch) | |
| tree | bde4ce12be4df4cb5a0881d74f71d644ea05a1a3 | |
| parent | cefa14bf2fc4cbc23a0f5e7d1daa04d258f8d62b (diff) | |
| download | rust-e72e7e9ae3a6938f1a926ca10f693d89948134cb.tar.gz rust-e72e7e9ae3a6938f1a926ca10f693d89948134cb.zip | |
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() {{}}
```
| -rw-r--r-- | compiler/rustc_smir/src/rustc_internal/mod.rs | 10 | ||||
| -rw-r--r-- | compiler/stable_mir/src/error.rs | 12 | ||||
| -rw-r--r-- | tests/ui-fulldeps/stable-mir/compilation-result.rs | 2 |
3 files changed, 13 insertions, 11 deletions
diff --git a/compiler/rustc_smir/src/rustc_internal/mod.rs b/compiler/rustc_smir/src/rustc_internal/mod.rs index 6bb8c5452b9..6e870728baf 100644 --- 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 index 7085fa937c9..9e3f4936944 100644 --- 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 index e6dd9fa132d..cd61d599eb4 100644 --- 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. |
