Move emit_stashed_diagnostic call in rustfmt.
#121615
Merged
Conversation
This file contains hidden or 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
This call was added to `parse_crate_mod` in rust-lang#121487, to fix a case where a stashed diagnostic wasn't emitted. But there is another path where a stashed diagnostic might fail to be emitted if there's a parse error, if the `build` call in `parse_crate_inner` fails before `parse_crate_mod` is reached. So this commit moves the `emit_stashed_diagnostic` call outwards, from `parse_crate_mod` to `format_project`, just after the `Parser::parse_crate` call. This should be far out enough to catch any parsing errors. Fixes rust-lang#121517.
|
Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt |
rustbot
added
the
S-waiting-on-review
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
|
@nnethercote your explanation makes sense to me. Thank you for working on this fix! |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 26, 2024
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#121389 (llvm-wrapper: fix few warnings) - rust-lang#121493 (By changing some attributes to only_local, reducing encoding attributes in the crate metadate.) - rust-lang#121615 (Move `emit_stashed_diagnostic` call in rustfmt.) - rust-lang#121617 (Actually use the right closure kind when checking async Fn goals) - rust-lang#121628 (Do not const prop unions) - rust-lang#121629 (fix some references to no-longer-existing ReprOptions.layout_seed) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 26, 2024
Rollup merge of rust-lang#121615 - nnethercote:fix-121517, r=oli-obk Move `emit_stashed_diagnostic` call in rustfmt. This call was added to `parse_crate_mod` in rust-lang#121487, to fix a case where a stashed diagnostic wasn't emitted. But there is another path where a stashed diagnostic might fail to be emitted if there's a parse error, if the `build` call in `parse_crate_inner` fails before `parse_crate_mod` is reached. So this commit moves the `emit_stashed_diagnostic` call outwards, from `parse_crate_mod` to `format_project`, just after the `Parser::parse_crate` call. This should be far out enough to catch any parsing errors. Fixes rust-lang#121517. r? `@oli-obk` cc `@ytmimi`
nnethercote
added a commit
to nnethercote/rust
that referenced
this pull request
Feb 28, 2024
I removed it in rust-lang#121206 because I thought thought it wasn't necessary. But then I had to add an `emit_stashed_diagnostics` call elsewhere in rustfmt to avoid the assertion failure (which took two attempts to get right, rust-lang#121487 and rust-lang#121615), and now there's an assertion failure in clippy as well (rust-lang/rust-clippy#12364). So this commit just reinstates the call in `DiagCtxtInner::drop`. It also reverts the rustfmt changes from rust-lang#121487 and rust-lang#121615, though it keeps the tests added for those PRs.
nnethercote
added a commit
to nnethercote/rust
that referenced
this pull request
Feb 29, 2024
I removed it in rust-lang#121206 because I thought thought it wasn't necessary. But then I had to add an `emit_stashed_diagnostics` call elsewhere in rustfmt to avoid the assertion failure (which took two attempts to get right, rust-lang#121487 and rust-lang#121615), and now there's an assertion failure in clippy as well (rust-lang/rust-clippy#12364). So this commit just reinstates the call in `DiagCtxtInner::drop`. It also reverts the rustfmt changes from rust-lang#121487 and rust-lang#121615, though it keeps the tests added for those PRs.
nnethercote
added a commit
to nnethercote/rust
that referenced
this pull request
Mar 1, 2024
I removed it in rust-lang#121206 because I thought thought it wasn't necessary. But then I had to add an `emit_stashed_diagnostics` call elsewhere in rustfmt to avoid the assertion failure (which took two attempts to get right, rust-lang#121487 and rust-lang#121615), and now there's an assertion failure in clippy as well (rust-lang/rust-clippy#12364). So this commit just reinstates the call in `DiagCtxtInner::drop`. It also reverts the rustfmt changes from rust-lang#121487 and rust-lang#121615, though it keeps the tests added for those PRs.
calebcartwright
pushed a commit
to calebcartwright/rust
that referenced
this pull request
Jun 22, 2024
Move `emit_stashed_diagnostic` call in rustfmt. This call was added to `parse_crate_mod` in rust-lang#121487, to fix a case where a stashed diagnostic wasn't emitted. But there is another path where a stashed diagnostic might fail to be emitted if there's a parse error, if the `build` call in `parse_crate_inner` fails before `parse_crate_mod` is reached. So this commit moves the `emit_stashed_diagnostic` call outwards, from `parse_crate_mod` to `format_project`, just after the `Parser::parse_crate` call. This should be far out enough to catch any parsing errors. Fixes rust-lang#121517. r? `@oli-obk` cc `@ytmimi`
calebcartwright
pushed a commit
to calebcartwright/rust
that referenced
this pull request
Jun 22, 2024
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#121389 (llvm-wrapper: fix few warnings) - rust-lang#121493 (By changing some attributes to only_local, reducing encoding attributes in the crate metadate.) - rust-lang#121615 (Move `emit_stashed_diagnostic` call in rustfmt.) - rust-lang#121617 (Actually use the right closure kind when checking async Fn goals) - rust-lang#121628 (Do not const prop unions) - rust-lang#121629 (fix some references to no-longer-existing ReprOptions.layout_seed) r? `@ghost` `@rustbot` modify labels: rollup
calebcartwright
pushed a commit
to calebcartwright/rust
that referenced
this pull request
Jun 22, 2024
I removed it in rust-lang#121206 because I thought thought it wasn't necessary. But then I had to add an `emit_stashed_diagnostics` call elsewhere in rustfmt to avoid the assertion failure (which took two attempts to get right, rust-lang#121487 and rust-lang#121615), and now there's an assertion failure in clippy as well (rust-lang/rust-clippy#12364). So this commit just reinstates the call in `DiagCtxtInner::drop`. It also reverts the rustfmt changes from rust-lang#121487 and rust-lang#121615, though it keeps the tests added for those PRs.
# for free
to join this conversation on GitHub.
Already have an account?
# 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.
This call was added to
parse_crate_modin #121487, to fix a case where a stashed diagnostic wasn't emitted. But there is another path where a stashed diagnostic might fail to be emitted if there's a parse error, if thebuildcall inparse_crate_innerfails beforeparse_crate_modis reached.So this commit moves the
emit_stashed_diagnosticcall outwards, fromparse_crate_modtoformat_project, just after theParser::parse_cratecall. This should be far out enough to catch any parsing errors.Fixes #121517.
r? @oli-obk
cc @ytmimi