Replace `visit_waiters` with `abstracted_waiters_of` by Zoxc · Pull Request #153376 · rust-lang/rust

Zoxc · 2026-03-04T00:36:50Z

This replaces visit_waiters which uses closures to visit waiters with abstracted_waiters_of which returns a list of waiters. This makes the control flow of their users a bit clearer.

Additionally abstracted_waiters_of includes non-query waiters unlike visit_waiters. This means that connected_to_root now considers resumable waiters from the compiler root as being connected to root, while previously only non-resumable waiters counted. This can result in some additional entry points being found. Similarly in the loop detecting entry points we now consider queries in the cycle with direct resumable waiters from the compiler root as being entry points.

When looking for entry points we now look at waiters until we found a query to populate the EntryPoint.waiter field instead of stopping when we determined it to be an entry point.

cc @petrochenkov @nnethercote

rustbot · 2026-03-04T00:36:55Z

r? @nnethercote

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

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler, query-system
compiler, query-system expanded to 69 candidates
Random selection from 16 candidates

nnethercote · 2026-03-04T10:02:38Z

This is a smallish change to code that #153185 is proposing to completely rewrite. If that PR is accepted and merged then this one will be moot. So we need to reach a conclusion on that PR before considering this one.

@rustbot blocked

Zoxc · 2026-03-04T10:07:43Z

That's not how that works.

nnethercote

I've been looking at enough of this code to have some idea of what's going on now. This new code is definitely easier to follow. E.g. the old comments at the top of visit_waiters were really hard to understand. The remaining code is still very complicated!

There are enough changes that I'm having trouble seeing the exact 1-to-1 mapping from the old control flow to the new control flow. Especially given that it seems (from the PR description) there are some deliberate changes to the control flow.

This file uses "query" in a lot of places where "job" would probably be better. (Working on the assumption that "query" the conceptual thing, like type_of, and "job" is an actively running query with a particular key.) That's beyond the scope of this PR, though.

View changes since this review

nnethercote · 2026-03-09T23:15:21Z

compiler/rustc_query_impl/src/job.rs

+struct Waiter {
+    span: Span,
+    /// The query which we are waiting from, if none the waiter is from a compiler root.
+    query: Option<QueryJobId>,


Should this be called parent? (I have been wondering the same thing about QueryWaiter::query.

I do associate parent a bit more with trees than graphs, but this could have a name other than query.

nnethercote · 2026-03-09T23:16:42Z

compiler/rustc_query_impl/src/job.rs

+    /// The query which we are waiting from, if none the waiter is from a compiler root.
+    query: Option<QueryJobId>,
+    resumable: Option<ResumableWaiterLocation>,
+}


We now have QueryWaiter and Waiter. And also QueryJobInfo and QueryInfo. These structs with similar names and contents are hard to remember.

Also, the variable name waiter now sometimes means a QueryWaiter, sometimes a Waiter, and sometimes a QueryJobId. This gets quite confusing. It would be good to have separate names. E.g. sometimes waiter_query is used for a QueryJobId that comes from waiter.query, which is better.

Waiter represents graph edges, so we could use graph terminology for contrast here.

nnethercote · 2026-03-09T23:18:15Z

compiler/rustc_query_impl/src/job.rs

+            result.push(Waiter {
+                span: waiter.span,
+                query: waiter.query,
+                resumable: Some((query, i)),


You're pushing the same query multiple times. I think it's possible to change ResumableWaiterLocation to just be the usize index, because the query is always available at the waiters_of call site.

Actually, I'm not sure if that works. It's still odd to see query pushed multiple times.

nnethercote · 2026-03-09T23:37:00Z

compiler/rustc_query_impl/src/job.rs

 /// the function return true.
 /// If a cycle was not found, the starting query is removed from `jobs` and
 /// the function returns false.
 fn remove_cycle<'tcx>(


This function is a good example of confusing names: waiter appears multiple times, with a different type and meaning each time.

nnethercote

Ok, I think this is good enough. Squash the commits and I will r+, thanks.

View changes since this review

nnethercote · 2026-03-13T06:57:27Z

r=me once the tests are passing.

@bors delegate=Zoxc

rust-bors · 2026-03-13T06:57:30Z

Invalid command: Invalid delegation type Zoxc. Possible values are try/review. Run @bors help to see available commands.

rustbot · 2026-03-13T11:58:56Z

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.

nnethercote · 2026-03-13T22:13:00Z

I don't know why delegation is no longer working.

@bors r+

rust-bors · 2026-03-13T22:13:03Z

📌 Commit 1f44a11 has been approved by nnethercote

It is now in the queue for this repository.

@petrochenkov

…cote Replace `visit_waiters` with `abstracted_waiters_of` This replaces `visit_waiters` which uses closures to visit waiters with `abstracted_waiters_of` which returns a list of waiters. This makes the control flow of their users a bit clearer. Additionally `abstracted_waiters_of` includes non-query waiters unlike `visit_waiters`. This means that `connected_to_root` now considers resumable waiters from the compiler root as being connected to root, while previously only non-resumable waiters counted. This can result in some additional entry points being found. Similarly in the loop detecting entry points we now consider queries in the cycle with direct resumable waiters from the compiler root as being entry points. When looking for entry points we now look at waiters until we found a query to populate the `EntryPoint.waiter` field instead of stopping when we determined it to be an entry point. cc @petrochenkov @nnethercote

Rollup of 4 pull requests Successful merges: - #153376 (Replace `visit_waiters` with `abstracted_waiters_of`) - #153818 (Reimplement const closures) - #153774 (Fix std doctest build for SGX target.) - #153786 (Remove `value_from_cycle_error` specializations)

Rollup of 7 pull requests Successful merges: - #152621 (LinkedGraph: support adding nodes and edges in arbitrary order) - #153376 (Replace `visit_waiters` with `abstracted_waiters_of`) - #153760 (Move and expand the big `rustc_query_impl` macro into a physical `query_impl.rs`) - #153818 (Reimplement const closures) - #153774 (Fix std doctest build for SGX target.) - #153786 (Remove `value_from_cycle_error` specializations) - #153819 (delete some duplicated tests)

rustbot assigned nnethercote Mar 4, 2026

nnethercote added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 4, 2026

nnethercote reviewed Mar 9, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

Zoxc force-pushed the cycle-waiters-iter branch from 2049f02 to c6bc581 Compare March 12, 2026 18:51

This comment has been minimized.

Sign in to view

nnethercote approved these changes Mar 13, 2026

View reviewed changes

nnethercote added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Mar 13, 2026

Zoxc force-pushed the cycle-waiters-iter branch from c6bc581 to ed07376 Compare March 13, 2026 06:44

This comment has been minimized.

Sign in to view

Zoxc changed the title ~~Replace visit_waiters with waiters_of~~ Replace visit_waiters with abstracted_waiters_of Mar 13, 2026

This comment has been minimized.

Sign in to view

Replace visit_waiters with abstracted_waiters_of

1f44a11

Zoxc force-pushed the cycle-waiters-iter branch from ed07376 to 1f44a11 Compare March 13, 2026 11:58

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 Mar 13, 2026

matthiaskrgr mentioned this pull request Mar 13, 2026

Rollup of 4 pull requests #153841

Closed

Zalathar mentioned this pull request Mar 14, 2026

Rollup of 7 pull requests #153846

Merged

rust-bors bot merged commit 1d49de5 into rust-lang:main Mar 14, 2026
11 checks passed

rustbot added this to the 1.96.0 milestone Mar 14, 2026

Zoxc deleted the cycle-waiters-iter branch March 14, 2026 11:38

Uh oh!

Conversation

Zoxc commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Mar 4, 2026

Uh oh!

nnethercote commented Mar 4, 2026

Uh oh!

Zoxc commented Mar 4, 2026

Uh oh!

nnethercote left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

nnethercote left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

nnethercote commented Mar 13, 2026

Uh oh!

rust-bors bot commented Mar 13, 2026

Uh oh!

rustbot commented Mar 13, 2026

Uh oh!

nnethercote commented Mar 13, 2026

Uh oh!

rust-bors bot commented Mar 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Zoxc commented Mar 4, 2026 •

edited

Loading

nnethercote left a comment •

edited by rustbot

Loading

nnethercote left a comment •

edited by rustbot

Loading