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 the remains of query categories #81609

Merged
merged 2 commits into from
Feb 2, 2021

Conversation

Julian-Wollersberger
Copy link
Contributor

@Julian-Wollersberger Julian-Wollersberger commented Jan 31, 2021

Back in October 2020 in #77830 @cjgillot removed the query categories information from the profiler, but the actual definitions which query was in which category remained, although unused.
Here I clean that up, to simplify the query definitions even further.

It's unfortunate that this loses all the context for git blame, but I'm working on moving those query definitions into rustc_query_system, which will lose that context anyway. EDIT: Might not work out.

The functional changes are in the first commit. The second one only changes the indentation.

@rust-highfive
Copy link
Collaborator

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 31, 2021
@cjgillot
Copy link
Contributor

A bit of context: I originally kept the query category syntax for a future attempt to split the monolithic query system into per-group subsystems. I since changed my mind, and did not get around to cleaning this up. Thanks @Julian-Wollersberger.

About the blame information: git is able to reconstruct the blame information when performing a git mv. If you are lucky, you won't lose anything when moving all this. We could chose to keep the indentation as-is in rustc_middle::query if this blame information is valuable.

About moving the query declarations to rustc_query_system: how will it interact with #70951? (I still intend to fix the perf regression and get that PR merged some day).

@Julian-Wollersberger
Copy link
Contributor Author

Julian-Wollersberger commented Jan 31, 2021

You're welcome :)

About the blame information

I tried that once, but it doesn't seem to work with GitHub.

About moving the query declarations to rustc_query_system: how will it interact with #70951? (I still intend to fix the perf regression and get that PR merged some day).

I believe it wouldn't have any impact, just that the macro_rules! produced by the query proc macro must be imported from a different path. My plan is to move just the query definitions and the DepKind enum. The rest depends to much on rustc_middle.
That might enable it that a bunch of generic bounds on the DepKind trait could be removed, but I've tried around quite a bit and it seems to not work out thanks to DepKind::debug_node(). So I'll probably have to give up on that.

The original goal was to prevent the query system from being monomorphized for every single query kind. Maybe I can find a different way. Any ideas?

@jyn514
Copy link
Member

jyn514 commented Feb 1, 2021

I tried that once, but it doesn't seem to work with GitHub.

It doesn't work with github, but it works fine locally with git log --follow (which I think is more useful than the online view, which can't do regex searches or things like that).

@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Feb 1, 2021

📌 Commit 988d93c has been approved by davidtwco

@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 1, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Feb 2, 2021
…ories, r=davidtwco

Remove the remains of query categories

Back in October 2020 in rust-lang#77830 `@cjgillot` removed the query categories information from the profiler, but the actual definitions which query was in which category remained, although unused.
Here I clean that up, to simplify the query definitions even further.

It's unfortunate that this loses all the context for `git blame`, ~~but I'm working on moving those query definitions into `rustc_query_system`, which will lose that context anyway.~~ EDIT: Might not work out.

The functional changes are in the first commit. The second one only changes the indentation.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 2, 2021
…as-schievink

Rollup of 11 pull requests

Successful merges:

 - rust-lang#80629 (Add lint for 2229 migrations)
 - rust-lang#81022 (Add Frames Iterator for Backtrace)
 - rust-lang#81481 (move some tests)
 - rust-lang#81485 (Add some tests for associated-type-bounds issues)
 - rust-lang#81492 (rustdoc: Note why `rustdoc::html::markdown` is public)
 - rust-lang#81577 (const_evaluatable: consider sub-expressions to be evaluatable)
 - rust-lang#81599 (Implement `TrustedLen` for `Fuse<I: TrustedLen>`)
 - rust-lang#81608 (Improve handling of spans around macro result parse errors)
 - rust-lang#81609 (Remove the remains of query categories)
 - rust-lang#81630 (Fix overflowing text on mobile when sidebar is displayed)
 - rust-lang#81631 (Remove unneeded `mut` variable)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 285524f into rust-lang:master Feb 2, 2021
@rustbot rustbot added this to the 1.51.0 milestone Feb 2, 2021
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants