A bunch of cleanups by bjorn3 · Pull Request #133567 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation24 Commits8 Checks6 Files changed
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
[ Show hidden characters]({{ revealButtonHref }})
These are all extracted from a branch I have to get rid of driver queries. Most of the commits are not directly necessary for this, but were found in the process of implementing the removal of driver queries.
Previous PR: #132410
r? @estebank
rustbot has assigned @estebank.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html)
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
Relevant to the rustdoc team, which will review and decide on the PR/issue.
labels
The Miri subtree was changed
cc @rust-lang/miri
Some changes occurred in src/tools/rustfmt
cc @rust-lang/rustfmt
Some changes occurred in src/tools/clippy
cc @rust-lang/clippy
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
And pass this to the individual emitters when necessary.
It was inconsistently done (sometimes even within a single function) and most of the rest of the compiler uses fatal errors instead, which need to be caught using catch_with_exit_code anyway. Using fatal errors instead of ErrorGuaranteed everywhere in the driver simplifies things a bit.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 nits and r=me
| @@ -446,23 +449,21 @@ fn run_compiler( |
|---|
| return early_exit(); |
| } |
| tcx.analysis(())?; |
| let _ = tcx.analysis(()); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should use tcx.ensure().analysis(()).
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The analysis query returns a unit, so skipping returning of the query result doesn't save us anything. I think we do however need to make sure that the query gets executed (rather than just doing nothing if all dependencies are unchanged) to prevent query stealing from going wrong and I'm not sure if .ensure() does that.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
analysis is eval_always, so yes, it will be re-executed in any case.ensure() is more about uniform style. When the return type is not (), let _ = some query should always be replaced by ensure() or ensure_with_value(), to make meaning explicit.
| if ppm.needs_analysis() && ex.tcx().analysis(()).is_err() { |
|---|
| FatalError.raise(); |
| if ppm.needs_analysis() { |
| let _ = ex.tcx().analysis(()); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should use ensure().
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
label
bors added a commit to rust-lang-ci/rust that referenced this pull request
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#133567 - bjorn3:various_cleanups, r=cjgillot
A bunch of cleanups
These are all extracted from a branch I have to get rid of driver queries. Most of the commits are not directly necessary for this, but were found in the process of implementing the removal of driver queries.
Previous PR: rust-lang#132410
github-merge-queue bot pushed a commit to model-checking/kani that referenced this pull request
Upgrade toolchain to 12/12. The only substantive changes are for the 12/10 toolchain; 12/11 and 12/12 are just updating the LLBC tests.
Culprit PR: rust-lang/rust#133567 (specifically this commit and this commit).
Resolves #3770
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…l, r=oli-obk
Stop using driver queries in the public API
Follow up to rust-lang#132410 and rust-lang#133567
The next PR will completely get rid of driver queries. That PR will also contains some non-trivial refactorings enabled by no longer needing to support entering TyCtxt multiple times after it is constructed. The changes in the current PR have been split out to make it easier to review the api changes and to reduce the size of the next PR to review.
Custom driver breaking change
The after_crate_root_parsing and after_expansion callbacks now accept ast::Crate and TyCtxt respectively rather than Queries. The only safe query in Queries to call inside these callbacks are parse() and global_ctxt() respectively which allows you to access the ast::Crate and TyCtxt either way. To fix your custom driver, replace the queries: &'tcx Queries<'tcx> argument with crate_: ast::Crate and tcx: TyCtxt<'tcx> respectively and for after_expansion remove your queries.global_ctxt().unwrap().enter(|tcx| { ... }) call and only keep the contents of the closure.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
A bunch of cleanups (part 2)
Just like rust-lang#133567 these were all found while looking at the respective code, but are not blocking any other changes I want to make in the short term.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#134130 - bjorn3:prepare_driver_query_removal, r=oli-obk
Stop using driver queries in the public API
Follow up to rust-lang#132410 and rust-lang#133567
The next PR will completely get rid of driver queries. That PR will also contains some non-trivial refactorings enabled by no longer needing to support entering TyCtxt multiple times after it is constructed. The changes in the current PR have been split out to make it easier to review the api changes and to reduce the size of the next PR to review.
Custom driver breaking change
The after_crate_root_parsing and after_expansion callbacks now accept ast::Crate and TyCtxt respectively rather than Queries. The only safe query in Queries to call inside these callbacks are parse() and global_ctxt() respectively which allows you to access the ast::Crate and TyCtxt either way. To fix your custom driver, replace the queries: &'tcx Queries<'tcx> argument with crate_: ast::Crate and tcx: TyCtxt<'tcx> respectively and for after_expansion remove your queries.global_ctxt().unwrap().enter(|tcx| { ... }) call and only keep the contents of the closure.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
A bunch of cleanups (part 2)
Just like rust-lang#133567 these were all found while looking at the respective code, but are not blocking any other changes I want to make in the short term.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
A bunch of cleanups (part 2)
Just like rust-lang#133567 these were all found while looking at the respective code, but are not blocking any other changes I want to make in the short term.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#134251 - bjorn3:various_cleanups2, r=oli-obk
A bunch of cleanups (part 2)
Just like rust-lang#133567 these were all found while looking at the respective code, but are not blocking any other changes I want to make in the short term.
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request
A bunch of cleanups (part 2)
Just like rust-lang/rust#133567 these were all found while looking at the respective code, but are not blocking any other changes I want to make in the short term.
smoelius added a commit to trailofbits/dylint that referenced this pull request
smoelius added a commit to trailofbits/dylint that referenced this pull request
smoelius added a commit to trailofbits/dylint that referenced this pull request
flip1995 pushed a commit to flip1995/rust that referenced this pull request
smoelius added a commit to trailofbits/dylint that referenced this pull request
bjorn3 pushed a commit to rust-lang/rustc_codegen_cranelift that referenced this pull request
A bunch of cleanups (part 2)
Just like rust-lang/rust#133567 these were all found while looking at the respective code, but are not blocking any other changes I want to make in the short term.
bors pushed a commit to rust-lang-ci/rust that referenced this pull request
A bunch of cleanups
These are all extracted from a branch I have to get rid of driver queries. Most of the commits are not directly necessary for this, but were found in the process of implementing the removal of driver queries.
Previous PR: rust-lang#132410
copybara-service bot pushed a commit to google/crubit that referenced this pull request
These failures were previously communicated using Result values that returned an ErrorGuaranteed in the error variant. rust-lang/rust#133567 changed these Result returns into immediate panics in more cases.
PiperOrigin-RevId: 715552263 Change-Id: Idf544a22e24d1ea7b13768f056685ece5df709d1
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this pull request
Labels
Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html)
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the compiler team, which will review and decide on the PR/issue.
Relevant to the rustdoc team, which will review and decide on the PR/issue.