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.

Sign up for GitHub

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

Jump to bottom

More refactorings to rustc_interface #127184

Merged
merged 4 commits into from
Jul 4, 2024
Merged

More refactorings to rustc_interface #127184

merged 4 commits into from
Jul 4, 2024

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Jun 30, 2024

Follow up to #126834

@rustbot
Copy link
Collaborator

rustbot commented Jun 30, 2024

r? @Nadrieril

rustbot has assigned @Nadrieril.
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

@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. labels Jun 30, 2024
@bjorn3
Copy link
Member Author

bjorn3 commented Jun 30, 2024

This doesn't yet remove driver queries entirely as I'm not sure how to deal with the various finalization things in Compiler::enter. Shifting the responsibility to call them to custom drivers seems like it would be forgotten too easily.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Nadrieril
Copy link
Member

Looks good, just a test to update.

I'm curious: what do you mean by "remove driver queries"?

@bjorn3
Copy link
Member Author

bjorn3 commented Jun 30, 2024

The methods on https://doc.rust-lang.org/nightly/nightly-rustc/rustc_interface/struct.Queries.html are what I call driver queries. Just like the queries in TyCtxt they are cached after their first call, but they are not serialized in the incr comp cache. They are purely interacted with by rustc_driver_impl and custom drivers. IMO they complicate things as it makes it easy for custom drivers to create the TyCtxt or HIR before the driver callback where you are supposed to access these, skipping various driver steps. In fact most custom drivers I have seen make this mistake at least once.

@Nadrieril
Copy link
Member

I see, thanks! Pretty sure I've done that myself :>

@Nadrieril Nadrieril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 30, 2024
@bjorn3
Copy link
Member Author

bjorn3 commented Jul 1, 2024

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 1, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Nadrieril
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Jul 2, 2024

📌 Commit f276459 has been approved by Nadrieril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jul 2, 2024

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@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 Jul 2, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 2, 2024
@matthiaskrgr
Rollup merge of rust-lang#127184 - bjorn3:interface_refactor2, r=Nadr…
b0d8e78 
…ieril

More refactorings to rustc_interface

Follow up to rust-lang#126834
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 2, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 3, 2024
@bors
Auto merge of rust-lang#127249 - matthiaskrgr:rollup-pnlimrc, r=matth…
dd71485 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#126403 (Actually report normalization-based type errors correctly for alias-relate obligations in new solver)
 - rust-lang#126803 (Change `asm-comments` to `verbose-asm`, always emit user comments)
 - rust-lang#126917 (Disable rmake test `inaccessible-temp-dir` on riscv64)
 - rust-lang#127050 (Make mtime of reproducible tarballs dependent on git commit)
 - rust-lang#127145 (Add `as_lang_item` to `LanguageItems`, new trait solver)
 - rust-lang#127184 (More refactorings to rustc_interface)
 - rust-lang#127202 (Remove global error count checks from typeck)
 - rust-lang#127233 (Some parser cleanups)
 - rust-lang#127245 (Add a test for `generic_const_exprs`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 3, 2024
@bors
Auto merge of rust-lang#127249 - matthiaskrgr:rollup-pnlimrc, r=matth…
04cd3f1 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#126403 (Actually report normalization-based type errors correctly for alias-relate obligations in new solver)
 - rust-lang#126803 (Change `asm-comments` to `verbose-asm`, always emit user comments)
 - rust-lang#126917 (Disable rmake test `inaccessible-temp-dir` on riscv64)
 - rust-lang#127050 (Make mtime of reproducible tarballs dependent on git commit)
 - rust-lang#127145 (Add `as_lang_item` to `LanguageItems`, new trait solver)
 - rust-lang#127184 (More refactorings to rustc_interface)
 - rust-lang#127202 (Remove global error count checks from typeck)
 - rust-lang#127233 (Some parser cleanups)
 - rust-lang#127245 (Add a test for `generic_const_exprs`)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 3, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 3, 2024
@bors
Auto merge of rust-lang#127296 - matthiaskrgr:rollup-1t1isa7, r=matth…
b454012 
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#127092 (Change return-type-notation to use `(..)`)
 - rust-lang#127184 (More refactorings to rustc_interface)
 - rust-lang#127190 (Update LLVM submodule)
 - rust-lang#127253 (Fix incorrect suggestion for extra argument with a type error)
 - rust-lang#127280 (Disable rmake test rustdoc-io-error on riscv64gc-gnu)
 - rust-lang#127294 (Less magic number for corountine)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9f8cf64 into rust-lang:master Jul 4, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 4, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 4, 2024
@rust-timer
Unrolled build for rust-lang#127184
0f56949 
Rollup merge of rust-lang#127184 - bjorn3:interface_refactor2, r=Nadrieril

More refactorings to rustc_interface

Follow up to rust-lang#126834
@bjorn3 bjorn3 deleted the interface_refactor2 branch July 4, 2024 06:40
@bjorn3 bjorn3 mentioned this pull request Oct 31, 2024
Merged
joboet added a commit to joboet/rust that referenced this pull request Nov 27, 2024
@joboet
Rollup merge of rust-lang#132410 - bjorn3:yet_another_driver_refactor…
d8c2a3a 
…_round, r=cjgillot

Some more refactorings towards removing driver queries

Follow up to rust-lang#127184

## Custom driver breaking change

The `after_analysis` callback is changed to accept `TyCtxt` instead of `Queries`. The only safe query in `Queries` to call at this point is `global_ctxt()` which allows you to enter the `TyCtxt` either way. To fix your custom driver, replace the `queries: &'tcx Queries<'tcx>` argument with `tcx: TyCtxt<'tcx>` and remove your `queries.global_ctxt().unwrap().enter(|tcx| { ... })` call and only keep the contents of the closure.

## Custom driver deprecation

The `after_crate_root_parsing` callback is now deprecated. Several custom drivers are incorrectly calling `queries.global_ctxt()` from inside of it, which causes some driver code to be skipped. As such I would like to either remove it in the future or if custom drivers still need it, change it to accept an `&rustc_ast::Crate` instead.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 27, 2024
@matthiaskrgr
Rollup merge of rust-lang#132410 - bjorn3:yet_another_driver_refactor…
af1ca15 
…_round, r=cjgillot

Some more refactorings towards removing driver queries

Follow up to rust-lang#127184

## Custom driver breaking change

The `after_analysis` callback is changed to accept `TyCtxt` instead of `Queries`. The only safe query in `Queries` to call at this point is `global_ctxt()` which allows you to enter the `TyCtxt` either way. To fix your custom driver, replace the `queries: &'tcx Queries<'tcx>` argument with `tcx: TyCtxt<'tcx>` and remove your `queries.global_ctxt().unwrap().enter(|tcx| { ... })` call and only keep the contents of the closure.

## Custom driver deprecation

The `after_crate_root_parsing` callback is now deprecated. Several custom drivers are incorrectly calling `queries.global_ctxt()` from inside of it, which causes some driver code to be skipped. As such I would like to either remove it in the future or if custom drivers still need it, change it to accept an `&rustc_ast::Crate` instead.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 28, 2024
@rust-timer
Unrolled build for rust-lang#132410
f5df064 
Rollup merge of rust-lang#132410 - bjorn3:yet_another_driver_refactor_round, r=cjgillot

Some more refactorings towards removing driver queries

Follow up to rust-lang#127184

## Custom driver breaking change

The `after_analysis` callback is changed to accept `TyCtxt` instead of `Queries`. The only safe query in `Queries` to call at this point is `global_ctxt()` which allows you to enter the `TyCtxt` either way. To fix your custom driver, replace the `queries: &'tcx Queries<'tcx>` argument with `tcx: TyCtxt<'tcx>` and remove your `queries.global_ctxt().unwrap().enter(|tcx| { ... })` call and only keep the contents of the closure.

## Custom driver deprecation

The `after_crate_root_parsing` callback is now deprecated. Several custom drivers are incorrectly calling `queries.global_ctxt()` from inside of it, which causes some driver code to be skipped. As such I would like to either remove it in the future or if custom drivers still need it, change it to accept an `&rustc_ast::Crate` instead.
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Nov 28, 2024
@matthiaskrgr
Rollup merge of #132410 - bjorn3:yet_another_driver_refactor_round, r…
c14cb34 
…=cjgillot

Some more refactorings towards removing driver queries

Follow up to rust-lang/rust#127184

## Custom driver breaking change

The `after_analysis` callback is changed to accept `TyCtxt` instead of `Queries`. The only safe query in `Queries` to call at this point is `global_ctxt()` which allows you to enter the `TyCtxt` either way. To fix your custom driver, replace the `queries: &'tcx Queries<'tcx>` argument with `tcx: TyCtxt<'tcx>` and remove your `queries.global_ctxt().unwrap().enter(|tcx| { ... })` call and only keep the contents of the closure.

## Custom driver deprecation

The `after_crate_root_parsing` callback is now deprecated. Several custom drivers are incorrectly calling `queries.global_ctxt()` from inside of it, which causes some driver code to be skipped. As such I would like to either remove it in the future or if custom drivers still need it, change it to accept an `&rustc_ast::Crate` instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Nadrieril Nadrieril

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.
Projects
None yet
Milestone
1.81.0
Development

Successfully merging this pull request may close these issues.
5 participants
@bjorn3 @rustbot @rust-log-analyzer @Nadrieril @bors