Skip to content

Simplify break_query_cycles #153185

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.

Sign up for GitHub

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

Closed
zetanumbers wants to merge 9 commits into from
Closed

Simplify break_query_cycles #153185

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d7a2352
Replace beack_query_cycles implementation with new
zetanumbers Feb 6, 2026
f7862e4
Write down theorems new cycle breaking depends on
zetanumbers Feb 27, 2026
9a3400d
Fix typo
zetanumbers Feb 27, 2026
253b3c1
Restore `QueryWaiter::span` field
zetanumbers Feb 28, 2026
6d5996a
Remove whitespace
zetanumbers Mar 2, 2026
2e0fbb9
Do not assume there's only one root query
zetanumbers Mar 2, 2026
6c56188
Use QueryWaiter's span
zetanumbers Mar 2, 2026
52c016b
Split cycle breaking into find_cycle_in_graph
zetanumbers Mar 2, 2026
11b79d6
Debug assert find_cycle_in_graph is consistent with find_cycle_in_stack
zetanumbers Mar 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions compiler/rustc_middle/src/query/job.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ impl<'tcx> QueryJob<'tcx> {
pub struct QueryWaiter<'tcx> {
pub query: Option<QueryJobId>,
pub condvar: Condvar,
pub span: Span,
Copy link
Copy Markdown
Contributor

@Zoxc Zoxc Feb 27, 2026

Choose a reason for hiding this comment

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

The removal of spans here doesn't seem great.

Copy link
Copy Markdown
Contributor Author

@zetanumbers zetanumbers Feb 27, 2026

Choose a reason for hiding this comment

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

Leaving unused struct fields doesn't seem like a good long-term approach either. I can split its removal into a separate commit for a fast revert if necessary.

Copy link
Copy Markdown
Contributor

@Zoxc Zoxc Feb 27, 2026

Choose a reason for hiding this comment

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

I mean it should still be used. Each query call has an associated span and that should be propagated to the cycle error.

Copy link
Copy Markdown
Contributor

@nnethercote nnethercote Mar 1, 2026

Choose a reason for hiding this comment

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

But the span is currently unused, correct?

Copy link
Copy Markdown
Contributor Author

@zetanumbers zetanumbers Mar 2, 2026

Choose a reason for hiding this comment

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

Done in 6c56188

pub cycle: Mutex<Option<CycleError<QueryStackDeferred<'tcx>>>>,
}

Expand All @@ -100,10 +99,9 @@ impl<'tcx> QueryLatch<'tcx> {
&self,
tcx: TyCtxt<'tcx>,
query: Option<QueryJobId>,
span: Span,
) -> Result<(), CycleError<QueryStackDeferred<'tcx>>> {
let waiter =
Arc::new(QueryWaiter { query, span, cycle: Mutex::new(None), condvar: Condvar::new() });
Arc::new(QueryWaiter { query, cycle: Mutex::new(None), condvar: Condvar::new() });
self.wait_on_inner(tcx, &waiter);
// FIXME: Get rid of this lock. We have ownership of the QueryWaiter
// although another thread may still have a Arc reference so we cannot
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_query_impl/src/execution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,6 @@ fn cycle_error<'tcx, C: QueryCache>(
fn wait_for_query<'tcx, C: QueryCache>(
query: &'tcx QueryVTable<'tcx, C>,
tcx: TyCtxt<'tcx>,
span: Span,
key: C::Key,
latch: QueryLatch<'tcx>,
current: Option<QueryJobId>,
Expand All @@ -240,7 +239,7 @@ fn wait_for_query<'tcx, C: QueryCache>(

// With parallel queries we might just have to wait on some other
// thread.
let result = latch.wait_on(tcx, current, span);
let result = latch.wait_on(tcx, current);

match result {
Ok(()) => {
Expand Down Expand Up @@ -320,7 +319,7 @@ fn try_execute_query<'tcx, C: QueryCache, const INCR: bool>(

// Only call `wait_for_query` if we're using a Rayon thread pool
// as it will attempt to mark the worker thread as blocked.
return wait_for_query(query, tcx, span, key, latch, current_job_id);
return wait_for_query(query, tcx, key, latch, current_job_id);
}

let id = job.id;
Expand Down
Loading
Loading