-
Couldn't load subscription status.
- Fork 13.9k
Merge CompilerError::CompilationFailed and CompilerError::ICE.
#121205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merge CompilerError::CompilationFailed and CompilerError::ICE.
#121205
Conversation
|
This PR changes Stable MIR cc @oli-obk, @celinval, @spastorino, @ouz-a |
|
I stumbled across this while working on some changes to how errors are propagated to the outer levels of the compiler. In those changes I increased the number of normal errors that trigger a |
`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() {{}}
```
edfebae to
e72e7e9
Compare
|
@bors r+ rollup |
…rError, 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`
Rollup of 8 pull requests Successful merges: - rust-lang#120718 (Add "algebraic" fast-math intrinsics, based on fast-math ops that cannot return poison) - rust-lang#121195 (unstable-book: Separate testing and production sanitizers) - rust-lang#121205 (Merge `CompilerError::CompilationFailed` and `CompilerError::ICE`.) - rust-lang#121233 (Move the extra directives for `Mode::CoverageRun` into `iter_header`) - rust-lang#121256 (Allow AST and HIR visitors to return `ControlFlow`) - rust-lang#121307 (Drive-by `DUMMY_SP` -> `Span` and fmt changes) - rust-lang#121310 (Remove an old hack for rustdoc) - rust-lang#121311 (Make `is_nonoverlapping` `#[inline]`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 10 pull requests Successful merges: - rust-lang#120716 (Change leak check and suspicious auto trait lint warning messages) - rust-lang#121195 (unstable-book: Separate testing and production sanitizers) - rust-lang#121205 (Merge `CompilerError::CompilationFailed` and `CompilerError::ICE`.) - rust-lang#121233 (Move the extra directives for `Mode::CoverageRun` into `iter_header`) - rust-lang#121256 (Allow AST and HIR visitors to return `ControlFlow`) - rust-lang#121307 (Drive-by `DUMMY_SP` -> `Span` and fmt changes) - rust-lang#121308 (Add regression test for rust-lang#103369) - rust-lang#121310 (Remove an old hack for rustdoc) - rust-lang#121311 (Make `is_nonoverlapping` `#[inline]`) - rust-lang#121319 (return `ty::Error` when equating `ty::Error`) r? `@ghost` `@rustbot` modify labels: rollup
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``
CompilerErrorhasCompilationFailedandICEvariants, which seems reasonable at first. But the way it identifies them is flawed:RunCompiler::runreturns anErr, it usesCompilationFailed, which is reasonable.FatalError, it catches the panic and usesICE. This is sometimes right, because ICEs do causeFatalErrorpanics, but sometimes wrong, because certain compiler errors also causeFatalErrorpanics. (The compiler/rustdoc/clippy/whatever just catches theFatalErrorwithcatch_with_exit_codeinmain.)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 existingInterruptedandSkippedvariants.Here is an example of a non-ICE failure that causes a
FatalErrorpanic, fromtests/ui/recursion_limit/issue-105700.rs:r? @spastorino