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

Simplify query proc-macros #77830

Merged
merged 5 commits into from
Oct 25, 2020
Merged

Simplify query proc-macros #77830

merged 5 commits into from
Oct 25, 2020

Conversation

cjgillot
Copy link
Contributor

The query code generation is split between proc-macros and regular macros in rustc_middle::ty::query.

This PR removes unused capabilities of the proc-macros, and tend to use regular macros for the logic.

@rust-highfive
Copy link
Collaborator

r? @varkor

(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 Oct 11, 2020
@jyn514 jyn514 added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 11, 2020
$(DepKind::$name => {
debug_assert!(<$K as DepNodeParams<TyCtxt<'_>>>::can_reconstruct_query_key());

if let Some(key) = <$K as DepNodeParams<TyCtxt<'_>>>::recover(tcx, dep_node) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the None case happen here? Or does recover only return None when can_reconstruct_query_key() would return false?
I don't understand the code here, but it seems like a fragile interaction of different parts. I'd add an else { bug!() } branch instead of the debug assert.
It's probably fine though, because the previous code did the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

recover is implemented in terms of extract_def_id, which uses def_path_hash_to_def_id.get(...), which can return None.

@varkor
Copy link
Member

varkor commented Oct 23, 2020

Though this looks fine to me, I don't feel familiar enough with the query system to approve this. Sorry for taking so long to get to this.

r? @oli-obk

@rust-highfive rust-highfive assigned oli-obk and unassigned varkor Oct 23, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Oct 23, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Oct 23, 2020

📌 Commit 57ba8ed has been approved by oli-obk

@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 Oct 23, 2020
@jyn514 jyn514 added the A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) label Oct 24, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 24, 2020
…as-schievink

Rollup of 12 pull requests

Successful merges:

 - rust-lang#75115 (`#[deny(unsafe_op_in_unsafe_fn)]` in sys/cloudabi)
 - rust-lang#76614 (change the order of type arguments on ControlFlow)
 - rust-lang#77610 (revise Hermit's mutex interface to support the behaviour of StaticMutex)
 - rust-lang#77830 (Simplify query proc-macros)
 - rust-lang#77930 (Do not ICE with TraitPredicates containing [type error])
 - rust-lang#78069 (Fix const core::panic!(non_literal_str).)
 - rust-lang#78072 (Cleanup constant matching in exhaustiveness checking)
 - rust-lang#78119 (Throw core::panic!("message") as &str instead of String.)
 - rust-lang#78191 (Introduce a temporary for discriminant value in MatchBranchSimplification)
 - rust-lang#78272 (const_evaluatable_checked: deal with unused nodes + div)
 - rust-lang#78318 (TyCtxt: generate single impl block with `slice_interners` macro)
 - rust-lang#78327 (resolve: Relax macro resolution consistency check to account for any errors)

Failed merges:

r? `@ghost`
@bors bors merged commit 4d72939 into rust-lang:master Oct 25, 2020
@rustbot rustbot added this to the 1.49.0 milestone Oct 25, 2020
@cjgillot cjgillot deleted the remacro branch October 25, 2020 01:30
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) C-cleanup Category: PRs that clean code up or issues documenting cleanup. 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
Development

Successfully merging this pull request may close these issues.

8 participants