Remove DiagnosticBuilder::buffer
#119841
Merged
Remove DiagnosticBuilder::buffer
#119841
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
|
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
They have one and two call sites respectively, and they just make the code harder to read.
Errors in `DiagCtxtInner::emit_diagnostic` are never set to `Level::Bug`, because the condition never succeeds, because `self.treat_err_as_bug()` is called *before* the error counts are incremented. This commit switches to `self.treat_next_err_as_bug()`, fixing the problem. This changes the error message output to actually say "internal compiler error".
No point computing `warnings` and `errors` if we're going to return early before they're used.
… consideration. It seems very wrong to have a `-Ztreat-err-as-bug` check here before the error is even emitted. Once that's done: - `into_diagnostic` is infallible, so its return type doesn't need the `Option`; - the `&'a DiagCtxt` also isn't needed, because only one callsite uses it, and it already have access to it via `self.dcx`; - the comments about dcx disabling buffering are no longer true, this is unconditional now; - and the `debug!` seems unnecessary... the comment greatly overstates its importance because few diagnostics come through `into_diagnostic`, and `-Ztrack-diagnostics` exists anyway.
Each of these has a single call site: `source_file_to_parser`, `try_file_to_source_file`, `file_to_source_file`. Having them separate just makes the code longer and harder to read. Also, `maybe_file_to_stream` doesn't need to be `pub`.
It's a little more concise, and the standard way to do it.
This requires cancelling the "secondary" errors when they're not emitted, to prevent panics due to unconsumed `DiagnosticBuilder`s.
But we can't easily switch from `Vec<Diagnostic>` to `Vec<DiagnosticBuilder<G>>` because there's a mix of errors and warnings which result in different `G` types. So we must make `DiagnosticBuilder::into_diagnostic` public, but that's ok, and it will get more use in subsequent commits.
Two different lifetimes are conflated. This doesn't matter right now, but needs to be fixed for the next commit to work. And the more descriptive lifetime names make the code easier to read.
nnethercote
force-pushed
the
rm-DiagnosticBuilder-buffer
branch
from
7a3a877
to
8918130
Compare
This comment has been minimized.
This comment has been minimized.
One consequence is that errors returned by `maybe_new_parser_from_source_str` now must be consumed, so a bunch of places that previously ignored those errors now cancel them. (Most of them explicitly dropped the errors before. I guess that was to indicate "we are explicitly ignoring these", though I'm not 100% sure.)
All its uses have been removed.
nnethercote
force-pushed
the
rm-DiagnosticBuilder-buffer
branch
from
8918130
to
4fd1db1
Compare
|
Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt |
oli-obk
approved these changes
Jan 11, 2024
|
@bors r+ |
bors
added
S-waiting-on-bors
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Jan 11, 2024
…uffer, r=oli-obk Remove `DiagnosticBuilder::buffer` `DiagnosticBuilder::buffer` doesn't do much, and part of what it does (for `-Ztreat-err-as-bug`) it shouldn't. This PR strips it back, replaces its uses, and finally removes it, making a few cleanups in the vicinity along the way. r? `@oli-obk`
This was referenced Jan 11, 2024
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 11, 2024
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#119448 (annotate-snippets: update to 0.10) - rust-lang#119813 (Silence some follow-up errors [2/x]) - rust-lang#119836 (chore: remove unnecessary blank line) - rust-lang#119841 (Remove `DiagnosticBuilder::buffer`) - rust-lang#119842 (coverage: Add enums to accommodate other kinds of coverage mappings) - rust-lang#119845 (rint: further doc tweaks) - rust-lang#119852 (give const-err4 a more descriptive name) - rust-lang#119853 (rustfmt.toml: don't ignore just any tests path, only root one) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 11, 2024
Rollup merge of rust-lang#119841 - nnethercote:rm-DiagnosticBuilder-buffer, r=oli-obk Remove `DiagnosticBuilder::buffer` `DiagnosticBuilder::buffer` doesn't do much, and part of what it does (for `-Ztreat-err-as-bug`) it shouldn't. This PR strips it back, replaces its uses, and finally removes it, making a few cleanups in the vicinity along the way. r? ``@oli-obk``
flip1995
pushed a commit
to flip1995/rust
that referenced
this pull request
Jan 25, 2024
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#119448 (annotate-snippets: update to 0.10) - rust-lang#119813 (Silence some follow-up errors [2/x]) - rust-lang#119836 (chore: remove unnecessary blank line) - rust-lang#119841 (Remove `DiagnosticBuilder::buffer`) - rust-lang#119842 (coverage: Add enums to accommodate other kinds of coverage mappings) - rust-lang#119845 (rint: further doc tweaks) - rust-lang#119852 (give const-err4 a more descriptive name) - rust-lang#119853 (rustfmt.toml: don't ignore just any tests path, only root one) r? `@ghost` `@rustbot` modify labels: rollup
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DiagnosticBuilder::bufferdoesn't do much, and part of what it does (for-Ztreat-err-as-bug) it shouldn't.This PR strips it back, replaces its uses, and finally removes it, making a few cleanups in the vicinity along the way.
r? @oli-obk