fix(query): Rotate Parallel Cycle Entries to Match Recovery Target

[TKanX]

Summary:

Mutual trait references as bare trait objects cause a fn_sig -> is_dyn_compatible cycle. In parallel mode, remove_cycle may rotate the cycle entries arbitrarily, causing from_cycle_error to pick an incorrect query as the entry point. This leads to an invalid recovery signature with wrong arity, eventually triggering a panic in virtual_call_violations_for_method.

Fix: in wait_for_query, rotate cycle entries so cycle[0] matches the query being recovered, restoring the invariant that find_cycle_in_stack naturally maintains in single-threaded mode.

Closes #153391

cc @matthiaskrgr @lqd

[rustbot]

r? @adwinwhite

rustbot has assigned @adwinwhite.
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
• compiler expanded to 69 candidates
• Random selection from 16 candidates

[rustbot]

⚠️ Warning ⚠️

• There are issue links (such as #123) in the commit messages of the following commits.
  Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.
  • e0716ad

[lqd]

cc @zetanumbers @Zoxc @SparrowLii for parallel query cycle changes

[zetanumbers]

Mutual trait references as bare trait objects cause a fn_sig -> is_dyn_compatible cycle. In parallel mode, remove_cycle may rotate the cycle entries arbitrarily, causing from_cycle_error to pick an incorrect query as the entry point. This leads to an invalid recovery signature with wrong arity, eventually triggering a panic in virtual_call_violations_for_method.

Good job at diagnosing this A-parallel-compiler issue! I expect your skills could benefit query system and parallel compiler in the long run.

However, I am afraid your fix is superseded by a new proposed query cycle handling design. In #153493 (comment) I've described a fix for #153391 that is included as a part of that proposal.

[zetanumbers]

Your fix also resembles #148936, but instead is forced to make a change to the query system. But this PR adds there an exhaustive check with regards to fn_sig query.

And Zoxc had another idea to abort on any query cycle error, so that fn_sig output's arity in cycle is not important.

[zetanumbers]

However, I am afraid your fix is superseded by a new proposed query cycle handling design. In #153493 (comment) I've described a fix for #153391 that is included as a part of that proposal.

Nonetheless you can help there. I invite you to write a PR that simply adds a query key to the from_cycle_error signature directly instead of extracting it from the query cycle. I will supervise you in this process.

[TKanX]

This is superseded by #153493 which removes FromCycleError entirely and fixes the root cause.

[zetanumbers]

This is superseded by #153493 which removes FromCycleError entirely and fixes the root cause.

No, this is not true. #153493 does not implement a fix for this bug as I've pointed out in #153493 (comment). As such I've asked if you want to implement a proper fix by adding a key: C::Key argument to the value_from_cycle_error function. I estimate that #153493 changes will be nearly orthogonal to your changes.

[TKanX]

This is superseded by #153493 which removes FromCycleError entirely and fixes the root cause.

No, this is not true. #153493 does not implement a fix for this bug as I've pointed out in #153493 (comment). As such I've asked if you want to implement a proper fix by adding a key: C::Key argument to the value_from_cycle_error function. I estimate that #153493 changes will be nearly orthogonal to your changes.

@zetanumbers Thanks for the correction. Should I update this PR or open a new one?

[zetanumbers]

You can open a new PR but just share its link here please.