Skip to content

Extract DepKindVTable constructors to their own module#152515

Open
Zalathar wants to merge 1 commit intorust-lang:mainfrom
Zalathar:dep-kind-vtables
Open

Extract DepKindVTable constructors to their own module#152515
Zalathar wants to merge 1 commit intorust-lang:mainfrom
Zalathar:dep-kind-vtables

Conversation

@Zalathar
Copy link
Member

This is a fairly substantial chunk of code in rustc_query_impl::plumbing that mostly doesn't need to be inside a macro at all.

The part that does need to be in a macro is the declaration of a named vtable constructor function for each query. I have extracted that into its own separate call to rustc_middle::rustc_with_all_queries! in the new module, so that dep-kind vtable construction is almost entirely confined to rustc_query_impl::dep_kind_vtables.

There should be no change to compiler behaviour.

r? nnethercote (or compiler)

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) 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 Feb 12, 2026
Comment on lines +16 to +17
#[expect(unused_imports, reason = "used by doc comments")]
use rustc_middle::dep_graph::DepKindVTable;
Copy link
Member Author

Choose a reason for hiding this comment

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

At first I tried using #[cfg(doc)] here, but that seems to break the ability to follow the link by command-clicking, at least in rust-analyzer.

data: PhantomData<&'tcx ()>
}

pub(crate) const IS_EVAL_ALWAYS: bool = is_eval_always!([$($modifiers)*]);
Copy link
Member Author

Choose a reason for hiding this comment

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

In #152435 I experimented with making this a field in QueryFlags instead.

The perf results were inconclusive, so for this PR I decided not to bundle that change.

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't feel like a win. It introduces a new thing that has a single use. The name is similar enough to the old name that it's not any clearer.

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought "maybe it gets used in a couple of additional places in the next commit", but no.

Copy link
Member Author

Choose a reason for hiding this comment

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

It made sense as part of pulling out the separate macro in another module, but if we aren't pulling out the macro then it isn't doing much anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. r=me with the first commit removed.

Copy link
Contributor

@nnethercote nnethercote left a comment

Choose a reason for hiding this comment

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

I like that the functions Null, Red, etc., are pulled out of the macro.

I'm less keen on having another call to rustc_with_all_queries!. This new macro feels like it belongs with define_dep_nodes!, because that also has the same pattern of "all the queries + the special dep kinds (Null/Red/...)".

Do you think there's a way this DepKindVTable stuff could be combined with the dep_kind stuff? Because we currently list the special dep kinds in two locations, which allows for them to get out of sync.

View changes since this review

@Zalathar
Copy link
Member Author

I'm less keen on having another call to rustc_with_all_queries!. This new macro feels like it belongs with define_dep_nodes!, because that also has the same pattern of "all the queries + the special dep kinds (Null/Red/...)".

I was prepared for possible pushback on that, so I'm perfectly fine with putting it back in the central macro.

Note that we only use macro-generated code for the query dep kinds; the non-query ones all have hand-written dep-node-vtable constructors. And the macro that combines them all into one array (make_dep_kind_array!) is defined by define_dep_nodes!.

Do you think there's a way this DepKindVTable stuff could be combined with the dep_kind stuff? Because we currently list the special dep kinds in two locations, which allows for them to get out of sync.

Note that make_dep_kind_array! will enforce that the argument module contains an item for every dep kind (non-query and query), and we'll get a warning if any constructor is unused, so I don't think there can be undetected desync in either direction.

@rustbot

This comment has been minimized.

Copy link
Contributor

@nnethercote nnethercote left a comment

Choose a reason for hiding this comment

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

Second commit looks good. I don't like the first commit.

View changes since this review

@nnethercote
Copy link
Contributor

And I still hate the "macro that produces a macro" pattern, but that's pre-existing.

@rustbot

This comment has been minimized.

@Zalathar
Copy link
Member Author

First commit introducing IS_EVAL_ALWAYS has been removed.

@bors r=nnethercote

@rust-bors
Copy link
Contributor

rust-bors bot commented Feb 13, 2026

📌 Commit de04914 has been approved by nnethercote

It is now in the queue for this repository.

@rust-bors rust-bors bot 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 13, 2026
rust-bors bot pushed a commit that referenced this pull request Feb 13, 2026
Rollup of 17 pull requests

Successful merges:

 - #150551 (Compute localized outlives constraints lazily)
 - #150988 (Improve code suggestion for incorrect macro_rules! usage)
 - #152422 (Change query proc macro to be more rust-analyzer friendly)
 - #152496 (Fix multi-cgu+debug builds using autodiff by delaying autodiff till lto)
 - #152514 (Collect active query jobs into struct `QueryJobMap`)
 - #152520 (Don't use `DepContext` in `rustc_middle::traits::cache`)
 - #152528 (Support serializing CodegenContext)
 - #152082 (Move tests)
 - #152232 (Add must_use for FileTimes)
 - #152329 (Simplify parallel! macro)
 - #152444 (`-Znext-solver` Prevent committing unfulfilled unsized coercion)
 - #152486 (remove redundant backchain attribute in codegen)
 - #152519 (Fix feature gating for new `try bikeshed` expressions)
 - #152529 (sparc64: enable abi compatibility test)
 - #152548 (reject inline const patterns pre-expansion)
 - #152550 (Port #[prelude_import] to the attribute parser)
 - #152552 (Add 2048-bit HvxVectorPair support to Hexagon SIMD ABI checks)

Failed merges:

 - #152515 (Extract `DepKindVTable` constructors to their own module)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 13, 2026
…ercote

Extract `DepKindVTable` constructors to their own module

This is a fairly substantial chunk of code in `rustc_query_impl::plumbing` that mostly doesn't need to be inside a macro at all.

The part that does need to be in a macro is the declaration of a named vtable constructor function for each query. I have extracted that into its own separate call to `rustc_middle::rustc_with_all_queries!` in the new module, so that dep-kind vtable construction is almost entirely confined to `rustc_query_impl::dep_kind_vtables`.

There should be no change to compiler behaviour.

r? nnethercote (or compiler)
@rust-bors rust-bors bot 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 13, 2026
@rust-bors

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Feb 13, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@Zalathar
Copy link
Member Author

Rebased over trivial import conflict.

@bors r=nnethercote

@rust-bors
Copy link
Contributor

rust-bors bot commented Feb 13, 2026

📌 Commit 1d6ef95 has been approved by nnethercote

It is now in the queue for this repository.

@rust-bors rust-bors bot 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 13, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 13, 2026
…ercote

Extract `DepKindVTable` constructors to their own module

This is a fairly substantial chunk of code in `rustc_query_impl::plumbing` that mostly doesn't need to be inside a macro at all.

The part that does need to be in a macro is the declaration of a named vtable constructor function for each query. I have extracted that into its own separate call to `rustc_middle::rustc_with_all_queries!` in the new module, so that dep-kind vtable construction is almost entirely confined to `rustc_query_impl::dep_kind_vtables`.

There should be no change to compiler behaviour.

r? nnethercote (or compiler)
rust-bors bot pushed a commit that referenced this pull request Feb 13, 2026
…uwer

Rollup of 14 pull requests

Successful merges:

 - #152323 (Fix ICE in borrowck when recovering `fn_sig` for `-> _`)
 - #152469 (Remove unused features)
 - #152515 (Extract `DepKindVTable` constructors to their own module)
 - #152555 (Port `#[rustc_diagnostic_item]` to the new attribute parsers)
 - #152218 (Report unconstrained region in hidden types lazily)
 - #152356 (Improve the `inline_fluent!` macro)
 - #152392 (Fix ICE in supertrait_vtable_slot when supertrait has missing generics)
 - #152407 (Add regression test for type_const with unit struct ctor under mGCA)
 - #152440 (Fix typos and grammar in compiler and build documentation)
 - #152536 (bootstrap: add explicit UTF-8 encoding to text-mode open() calls)
 - #152554 (Remove `deprecated_safe` and its corresponding feature gate)
 - #152556 (doc: move riscv64a23-unknown-linux-gnu to tier 2)
 - #152563 (Replace "bug" with "issue" in triagebot ping messages)
 - #152565 (fix missleading error for tuple ctor)

Failed merges:

 - #152512 (core: Implement feature `float_exact_integer_constants`)
 - #152296 (Port `rust_nonnull_optimization_guaranteed` and `rustc_do_not_const_check` to the new attribute parser)
rust-bors bot pushed a commit that referenced this pull request Feb 13, 2026
…uwer

Rollup of 14 pull requests

Successful merges:

 - #152323 (Fix ICE in borrowck when recovering `fn_sig` for `-> _`)
 - #152469 (Remove unused features)
 - #152515 (Extract `DepKindVTable` constructors to their own module)
 - #152555 (Port `#[rustc_diagnostic_item]` to the new attribute parsers)
 - #152218 (Report unconstrained region in hidden types lazily)
 - #152356 (Improve the `inline_fluent!` macro)
 - #152392 (Fix ICE in supertrait_vtable_slot when supertrait has missing generics)
 - #152407 (Add regression test for type_const with unit struct ctor under mGCA)
 - #152440 (Fix typos and grammar in compiler and build documentation)
 - #152536 (bootstrap: add explicit UTF-8 encoding to text-mode open() calls)
 - #152554 (Remove `deprecated_safe` and its corresponding feature gate)
 - #152556 (doc: move riscv64a23-unknown-linux-gnu to tier 2)
 - #152563 (Replace "bug" with "issue" in triagebot ping messages)
 - #152565 (fix missleading error for tuple ctor)

Failed merges:

 - #152512 (core: Implement feature `float_exact_integer_constants`)
 - #152296 (Port `rust_nonnull_optimization_guaranteed` and `rustc_do_not_const_check` to the new attribute parser)
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) 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.

3 participants