Skip to content

Don't pass a separate DepKind to query_feed#153811

Merged
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
Zalathar:query-feed
Mar 14, 2026
Merged

Don't pass a separate DepKind to query_feed#153811
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
Zalathar:query-feed

Conversation

@Zalathar
Copy link
Member

@Zalathar Zalathar commented Mar 13, 2026

This PR makes two small tweaks to the query_feed function, which is called by macro-generated methods on TyCtxtFeed:

  • Don't pass DepKind as a separate argument, because it's already in the QueryVTable
  • Rename query_vtable to query, to match other functions that take QueryVTable

r? nnethercote (or compiler)

@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 Mar 13, 2026
@nnethercote
Copy link
Contributor

First commit is great, third commit is fine, I'm less keen on the second commit. I'm generally not a fan of moving code like this into its own function unless there's a really good reason, like it's a huge amount of code. Because a separate function is just a level of indirection, and a chance to wonder "how many callers does this function have?" The control flow here isn't complicated (just a Some arm and a None arm) and now the two arms are asymmetrical in the sense that one delegates everything to another function while the other doesn't do that.

@Zalathar
Copy link
Member Author

I don’t mind dropping the second commit, though I was slightly surprised to hear an objection since I consider it to be doing basically the same thing as check_feedable_consistency in #153766.

@nnethercote
Copy link
Contributor

Fair enough. IMO, the key difference is that check_feedable_consistency is an auxiliary path.

@Zalathar
Copy link
Member Author

@rustbot author

@rustbot rustbot 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 Mar 13, 2026
The query's dep kind can be obtained from its vtable instead.

This commit also renames the `query_vtable` parameter to `query`, to be more
consistent with other functions that take a QueryVTable.
@rustbot
Copy link
Collaborator

rustbot commented Mar 14, 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 Zalathar changed the title Mostly-cosmetic improvements to query_feed Don't pass a separate DepKind to query_feed Mar 14, 2026
@Zalathar
Copy link
Member Author

Based on feedback, I've reduced the scope of this PR to just the first commit, since the extraction was contentious, and I'm less excited by the third commit if we aren't doing the extraction.

@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 Mar 14, 2026
@rust-log-analyzer
Copy link
Collaborator

The job aarch64-gnu-llvm-21-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Executing "/scripts/stage_2_test_set1.sh"
+ /scripts/stage_2_test_set1.sh
PR_CI_JOB set; skipping tidy
+ '[' 1 == 1 ']'
+ echo 'PR_CI_JOB set; skipping tidy'
+ SKIP_TIDY='--skip tidy'
+ ../x.py --stage 2 test --skip tidy --skip compiler --skip src
##[group]Building bootstrap
    Finished `dev` profile [unoptimized] target(s) in 0.04s
##[endgroup]
downloading https://static.rust-lang.org/dist/2026-03-05/rustfmt-nightly-aarch64-unknown-linux-gnu.tar.xz
---
   Compiling rustc_codegen_llvm v0.0.0 (/checkout/compiler/rustc_codegen_llvm)
[RUSTC-TIMING] rustc_const_eval test:false 43.729
   Compiling rustc_mir_transform v0.0.0 (/checkout/compiler/rustc_mir_transform)

Session terminated, killing shell...::group::Clock drift check
##[error]The runner has received a shutdown signal. This can happen when the runner service is stopped, or a manually started runner is canceled.
  local time: Sat Mar 14 05:07:31 UTC 2026
  network time: Sat, 14 Mar 2026 05:07:31 GMT
##[endgroup]
 ...killed.
##[error]The operation was canceled.

@nnethercote
Copy link
Contributor

r=me once the tests are working.

@Zalathar
Copy link
Member Author

PR CI is green.

@bors r=nnethercote rollup

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 14, 2026

📌 Commit 7d4ee74 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 Mar 14, 2026
rust-bors bot pushed a commit that referenced this pull request Mar 14, 2026
Rollup of 5 pull requests

Successful merges:

 - #153769 (target specs: stricter checks for LLVM ABI values, and correlate that with cfg(target_abi))
 - #153811 (Don't pass a separate `DepKind` to `query_feed`)
 - #153817 (relocate several ui tests)
 - #153840 (tests/debuginfo/basic-stepping.rs: Add cdb test)
 - #153858 (Use named fields in ChunkedBitSet's `Chunk::Mixed`)
@rust-bors rust-bors bot merged commit ec9bc94 into rust-lang:main Mar 14, 2026
14 of 22 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Mar 14, 2026
@Zalathar Zalathar deleted the query-feed branch March 14, 2026 23:02
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. 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.

4 participants