Skip to content
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

Remove some unchecked_claim_error_was_emitted calls #120735

Merged
merged 6 commits into from
Feb 7, 2024

Conversation

nnethercote
Copy link
Contributor

We want to drive the number of these calls down as much as possible. This PR gets rid of a bunch of them.

r? @oli-obk

When `catch_fatal_errors` catches a `FatalErrorMarker`, it returns an
`ErrorGuaranteed` that is conjured out of thin air with
`unchecked_claim_error_was_emitted`. But that `ErrorGuaranteed` is never
used.

This commit changes it to instead conjure a `FatalError` out of thin
air. (A non-deprecated action!) This makes more sense because
`FatalError` and `FatalErrorMarker` are a natural pairing -- a
`FatalErrorMarker` is created by calling `FatalError::raise`, so this is
effectively getting back the original `FatalError`.

This requires a tiny change in `catch_with_exit_code`. The old result of
the `catch_fatal_errors` call there was
`Result<Result<(), ErrorGuaranteed>, ErrorGuaranteed>` which could be
`flatten`ed into `Result<(), ErrorGuaranteed>`. The new result of the
`catch_fatal_errors` calls is
`Result<Result<(), ErrorGuaranteed>, FatalError>`, which can't be
`flatten`ed but is still easily matched for the success case.
By making non-unicode arguments a fatal error instead of a warning, we
don't need to handle what comes after, which avoids the need for an
`unchecked_claim_error_was_emitted` call.
`main_args` calls `from_matches`, which does lots of initialization. If
anything goes wrong, `from_matches` emits an error message and returns
`Err(1)` (or `Err(3)`). `main_args` then turns the `Err(1)` into
`Err(ErrorGuaranteed)`, because that's what `catch_with_exit_code`
requires on error. But `catch_with_exit_code` doesn't do anything with
the `ErrorGuaranteed`, it just exits with `EXIT_FAILURE`.

We can avoid the creation of the `ErrorGuaranteed` (which requires
an undesirable `unchecked_claim_error_was_emitted` call), by changing
`from_matches` to instead eagerly abort if anything goes wrong. The
behaviour from the user's point of view is the same: an early abort with
an `EXIT_FAILURE` exit code.

And we can also simplify `from_matches` to return an `Option` instead of
a `Result`:
- Old `Err(0)` case --> `None`
- Old `Err(_)` case --> fatal error.

This requires similar changes to `ScrapeExamplesOptions::new` and
`load_call_locations`.
- In `emit_producing_error_guaranteed`, only allow `Level::Error`.
- In `emit_diagnostic`, only produce `ErrorGuaranteed` for `Level` and
  `DelayedBug`. (Not `Bug` or `Fatal`. They don't need it, because the
  relevant `emit` methods abort.)
- Add/update various comments.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Feb 7, 2024
@oli-obk
Copy link
Contributor

oli-obk commented Feb 7, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 7, 2024

📌 Commit 6889fe3 has been approved by oli-obk

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 7, 2024
Nadrieril added a commit to Nadrieril/rust that referenced this pull request Feb 7, 2024
…ms, r=oli-obk

Remove some `unchecked_claim_error_was_emitted` calls

We want to drive the number of these calls down as much as possible. This PR gets rid of a bunch of them.

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 7, 2024
Rollup of 13 pull requests

Successful merges:

 - rust-lang#110482 (Add armv8r-none-eabihf target for the Cortex-R52.)
 - rust-lang#119162 (Add unstable `-Z direct-access-external-data` cmdline flag for `rustc`)
 - rust-lang#120302 (various const interning cleanups)
 - rust-lang#120455 ( Add FileCheck annotations to MIR-opt SROA tests)
 - rust-lang#120470 (Mark "unused binding" suggestion as maybe incorrect)
 - rust-lang#120479 (Suggest turning `if let` into irrefutable `let` if appropriate)
 - rust-lang#120564 (coverage: Split out counter increment sites from BCB node/edge counters)
 - rust-lang#120633 (pattern_analysis: gather up place-relevant info)
 - rust-lang#120664 (Add parallel rustc ui tests)
 - rust-lang#120721 (fix `llvm_out` to use the correct LLVM root)
 - rust-lang#120726 (Don't use bashism in checktools.sh)
 - rust-lang#120733 (MirPass: make name more const)
 - rust-lang#120735 (Remove some `unchecked_claim_error_was_emitted` calls)

Failed merges:

 - rust-lang#120727 (exhaustiveness: Prefer "`0..MAX` not covered" to "`_` not covered")

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 7, 2024
Rollup of 13 pull requests

Successful merges:

 - rust-lang#110482 (Add armv8r-none-eabihf target for the Cortex-R52.)
 - rust-lang#119162 (Add unstable `-Z direct-access-external-data` cmdline flag for `rustc`)
 - rust-lang#120302 (various const interning cleanups)
 - rust-lang#120455 ( Add FileCheck annotations to MIR-opt SROA tests)
 - rust-lang#120470 (Mark "unused binding" suggestion as maybe incorrect)
 - rust-lang#120479 (Suggest turning `if let` into irrefutable `let` if appropriate)
 - rust-lang#120564 (coverage: Split out counter increment sites from BCB node/edge counters)
 - rust-lang#120633 (pattern_analysis: gather up place-relevant info)
 - rust-lang#120664 (Add parallel rustc ui tests)
 - rust-lang#120726 (Don't use bashism in checktools.sh)
 - rust-lang#120733 (MirPass: make name more const)
 - rust-lang#120735 (Remove some `unchecked_claim_error_was_emitted` calls)
 - rust-lang#120746 (Record coroutine kind in coroutine generics)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit cb040f5 into rust-lang:master Feb 7, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 7, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 7, 2024
Rollup merge of rust-lang#120735 - nnethercote:rm-some-unchecked_claims, r=oli-obk

Remove some `unchecked_claim_error_was_emitted` calls

We want to drive the number of these calls down as much as possible. This PR gets rid of a bunch of them.

r? ``@oli-obk``
@nnethercote nnethercote deleted the rm-some-unchecked_claims branch February 8, 2024 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants