Skip to content

Conversation

@nnethercote
Copy link
Contributor

@nnethercote nnethercote commented Jan 16, 2026

PR #138672 fixed a query cycle OOM reported in #124901. The fix involved delaying computation of some query stack frame elements and was very complex. It involved the addition of two new types, the addition of a generic I parameter to eleven(!) other types, a PhantomData field, and even required an unsafe transmute of a closure. This comment had suggested a much simpler fix, but it was ignored. The PR also failed to add a test case.

This PR adds a test case, reverts the complex fix, applies the simpler fix, and does a few other minor cleanups.

r? @oli-obk

To be consistent with the closely related `cycle_stash` and
`cycle_delay_bug`.
Issue rust-lang#124901 was an OOM caused by a query cycle. It was fixed by
a complex change in PR rust-lang#138672, but that PR did not add the test case.
Let's add it now, because it's the only known reproducer of the OOM.
PR rust-lang#138672 introduced a complex and invasive split of `QueryStackFrame`
to avoid a query cycle. This commit reverts that change because there is
a much simpler change that fixes the problem, which will be in the next
commit.
This changes the error message of `query-cycle-issue-124901.rs`, which
doesn't matter much.
@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 Jan 16, 2026
@rustbot
Copy link
Collaborator

rustbot commented Jan 16, 2026

oli-obk is not on the review rotation at the moment.
They may take a while to respond.

@rustbot
Copy link
Collaborator

rustbot commented Jan 16, 2026

⚠️ 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.

@oli-obk
Copy link
Contributor

oli-obk commented Jan 16, 2026

makes sense. I'd wanna debug this case specifically to see why we even have that cycle, but avoiding the oom with the simpler fix is good for now

@bors r+ rollup

@rust-bors
Copy link
Contributor

rust-bors bot commented Jan 16, 2026

📌 Commit 90e4a42 has been approved by oli-obk

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 Jan 16, 2026
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 16, 2026
…i-obk

Revert `QueryStackFrame` split

PR rust-lang#138672 fixed a query cycle OOM reported in rust-lang#124901. The fix involved delaying computation of some query stack frame elements and was very complex. It involved the addition of two new types, the addition of a generic `I` parameter to eleven(!) other types, a `PhantomData` field, and even required an unsafe transmute of a closure. [This comment](rust-lang#124901 (comment)) had suggested a much simpler fix, but it was ignored. The PR also failed to add a test case.

This PR adds a test case, reverts the complex fix, applies the simpler fix, and does a few other minor cleanups.

r? @oli-obk
rust-bors bot pushed a commit that referenced this pull request Jan 16, 2026
Rollup of 6 pull requests

Successful merges:

 - #145354 (Cache derive proc macro expansion with incremental query)
 - #151123 (Support primitives in type info reflection)
 - #151178 (simplify words initialization using Rc::new_zeroed)
 - #151187 (Use `default_field_values` more in `Resolver`)
 - #151197 (remove lcnr from compiler review rotation)
 - #151203 (Revert `QueryStackFrame` split)

r? @ghost
@Zoxc
Copy link
Contributor

Zoxc commented Jan 16, 2026

This does not justify why moving query computation back into the query cycle handling is safe. We generally should avoid using queries in the implementation of the query system itself. #138672 eliminates a category of bugs, it's not specific to #124901.

@rust-bors rust-bors bot merged commit 57e950b into rust-lang:main Jan 16, 2026
11 checks passed
rust-timer added a commit that referenced this pull request Jan 16, 2026
Rollup merge of #151203 - revert-QueryStackFrame-split, r=oli-obk

Revert `QueryStackFrame` split

PR #138672 fixed a query cycle OOM reported in #124901. The fix involved delaying computation of some query stack frame elements and was very complex. It involved the addition of two new types, the addition of a generic `I` parameter to eleven(!) other types, a `PhantomData` field, and even required an unsafe transmute of a closure. [This comment](#124901 (comment)) had suggested a much simpler fix, but it was ignored. The PR also failed to add a test case.

This PR adds a test case, reverts the complex fix, applies the simpler fix, and does a few other minor cleanups.

r? @oli-obk
@rustbot rustbot added this to the 1.94.0 milestone Jan 16, 2026
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.

4 participants