Skip to content

Avoid duplicated query modifier comments.#153428

Merged
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
nnethercote:fix-query-comments
Mar 13, 2026
Merged

Avoid duplicated query modifier comments.#153428
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
nnethercote:fix-query-comments

Conversation

@nnethercote
Copy link
Contributor

@nnethercote nnethercote commented Mar 5, 2026 •
edited by rustbot
Loading

View all comments

There is currently some duplication.

@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 Mar 5, 2026
@rustbot
Copy link
Collaborator

rustbot commented Mar 5, 2026

r? @JohnTitor

rustbot has assigned @JohnTitor.
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 15 candidates

@nnethercote nnethercote marked this pull request as draft March 5, 2026 04:38
@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 5, 2026
@nnethercote
Copy link
Contributor Author

nnethercote commented Mar 5, 2026

r? @Zalathar

@rustbot rustbot assigned Zalathar and unassigned JohnTitor Mar 5, 2026
@rustbot
Copy link
Collaborator

rustbot commented Mar 5, 2026

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

@nnethercote nnethercote changed the title Fix query comments Avoid duplicated query modifier comments. Mar 5, 2026
@nnethercote nnethercote marked this pull request as ready for review March 5, 2026 05:05
@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 5, 2026
Zalathar
Zalathar reviewed Mar 5, 2026
View reviewed changes
@rust-bors

This comment has been minimized.

Sign in to view
@rustbot

This comment has been minimized.

Sign in to view
@nnethercote
Copy link
Contributor Author

nnethercote commented Mar 5, 2026

I rebased.

@Zalathar
Copy link
Member

Zalathar commented Mar 6, 2026

Regarding the $cache_on_disk_if change, I have a PR proposing to get rid of the explicit cache-on-disk condition and rename the contributor-facing modifier to just cache_on_disk:

@rust-bors

This comment has been minimized.

Sign in to view
@rustbot

This comment has been minimized.

Sign in to view
@nnethercote
Copy link
Contributor Author

nnethercote commented Mar 8, 2026

I rebased.

@nnethercote
Copy link
Contributor Author

nnethercote commented Mar 9, 2026

@Zalathar: any thoughts?

@Zalathar
Copy link
Member

Zalathar commented Mar 10, 2026 •
edited
Loading

I’ve spent hours trying to review this, but I keep getting discouraged by how much energy it would take to argue against the contentious parts.

@nnethercote
Copy link
Contributor Author

nnethercote commented Mar 10, 2026 •
edited
Loading

Huh... I thought this would be uncontroversial. Can you indicate which commits you think are contentious?

@rustbot

This comment has been minimized.

Sign in to view
@nnethercote
Copy link
Contributor Author

nnethercote commented Mar 11, 2026

@Zalathar: I have removed the first and third commits involving query modifier names, because cycle_delay_bug may be removed soon and cache_on_disk_if may be renamed soon.

That just leaves the query modifier comment de-duplication. If there is contention over the wording of the comments, I'm happy to use your suggestions. I'm also happy if you want to just take this over and do it yourself. I just want the duplication gone :)

@rust-bors

This comment has been minimized.

Sign in to view
@rustbot

This comment has been minimized.

Sign in to view
Zalathar
Zalathar reviewed Mar 12, 2026
View reviewed changes
Zalathar
Zalathar reviewed Mar 12, 2026
View reviewed changes
@nnethercote nnethercote force-pushed the fix-query-comments branch 2 times, most recently from 30553e7 to 8112cc8 Compare March 12, 2026 03:05
@rust-log-analyzer

This comment has been minimized.

Sign in to view
Zalathar
Zalathar reviewed Mar 13, 2026
View reviewed changes
compiler/rustc_middle/src/queries.rs Outdated
//! - `separate_provide_extern`: Use separate provider functions for local and external crates.
//! - `feedable`: Allow the query result to be set from another query ("fed" externally).
//! For the list of modifiers, see
//! [`rustc_middle/src/query/modifiers.rs`](https://github.com/rust-lang/rust/blob/HEAD/compiler/rustc_middle/src/query/modifiers.rs).
Copy link
Member

@Zalathar Zalathar Mar 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Nit: Is there a specific reason for using a hardcoded web link here? I would have expected the target to be a Rust module path that gets resolved by rustdoc, i.e. rustc_middle::query::modifiers.

Copy link
Contributor Author

@nnethercote nnethercote Mar 13, 2026

Choose a reason for hiding this comment

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

Not sure, it was pre-existing. I've switched it to a normal link and done a local docs build to test that it works. The rendered rustdocs are nicer to look at that the source code in GitHub :)

@nnethercote
Avoid duplicated query modifier comments.
16dcceb 
By removing the ones in `compiler/rustc_middle/src/queries.rs`, and
making `compiler/rustc_middle/src/query/modifiers.rs` the single
source of truth.
@rustbot
Copy link
Collaborator

rustbot commented Mar 13, 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
Copy link
Member

Zalathar commented Mar 13, 2026

Thanks for the adjustments.

@bors r+ rollup

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 13, 2026

📌 Commit 16dcceb has been approved by Zalathar

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 13, 2026
Zalathar added a commit to Zalathar/rust that referenced this pull request Mar 13, 2026
@Zalathar
Rollup merge of rust-lang#153428 - nnethercote:fix-query-comments, r=…
f2b0a3e 
…Zalathar

Avoid duplicated query modifier comments.

There is currently some duplication.
This was referenced Mar 13, 2026
Closed
Merged
rust-bors bot pushed a commit that referenced this pull request Mar 13, 2026
@bors
Auto merge of #153805 - Zalathar:rollup-rfXICNX, r=Zalathar
7ad5c60 
Rollup of 5 pull requests

Successful merges:

 - #152258 (fixed VecDeque::splice() not filling the buffer correctly when resizing the buffer on start = end range)
 - #153691 (Miscellaneous tweaks)
 - #153766 (`try_execute_query` tweaks)
 - #153188 (implementation of `-Z min-recursion-limit`)
 - #153428 (Avoid duplicated query modifier comments.)
@Zalathar Zalathar mentioned this pull request Mar 13, 2026
Merged
@rust-bors rust-bors bot merged commit 8669e0c into rust-lang:main Mar 13, 2026
11 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Mar 13, 2026
@nnethercote nnethercote deleted the fix-query-comments branch March 13, 2026 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Zalathar Zalathar Zalathar left review comments

Assignees

@Zalathar Zalathar

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

Milestone

1.96.0

Development

Successfully merging this pull request may close these issues.

5 participants

@nnethercote @rustbot @Zalathar @rust-log-analyzer @JohnTitor