Document potential_query_instability allow sites#157367
Conversation
|
The Clippy subtree was changed cc @rust-lang/clippy Some changes occurred in compiler/rustc_attr_parsing cc @jdonszelmann, @JonathanBrouwer Some changes occurred in match checking cc @Nadrieril Some changes occurred in check-cfg diagnostics cc @Urgau |
rustbot
added
A-attributes
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @folkertdev (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
Why was this reviewer chosen?The reviewer was selected based on:
|
| #[allow(rustc::potential_query_instability)] | ||
| let possibilities: Vec<Symbol> = sess.check_config.expecteds.keys().copied().collect(); | ||
| let mut possibilities: Vec<Symbol> = sess.check_config.expecteds.keys().copied().collect(); | ||
| possibilities.sort_by(|a, b| a.as_str().cmp(b.as_str())); |
There was a problem hiding this comment.
AFAICT all uses below either don't care about order or already sort it.
There was a problem hiding this comment.
Done. I removed the extra sort here and moved the justification into the allow reason.
| let mut repeatables = Vec::new(); | ||
| let mut non_repeatables = Vec::new(); | ||
|
|
||
| // The buckets are sorted by symbol before diagnostic matching below. |
There was a problem hiding this comment.
I think sorting is not strictly necessary given that find_best_match_for_name will already sort when there are multiple options with the same priority.
There was a problem hiding this comment.
Done. I removed the additional sorting and kept this as an allow(..., reason = ...) justification.
|
r? bjorn3 |
| (name, name_span): (Symbol, Span), | ||
| value: Option<(Symbol, Span)>, | ||
| ) -> errors::UnexpectedCfgName { | ||
| // Sort before diagnostic matching so equally good suggestions are chosen deterministically. |
There was a problem hiding this comment.
Maybe put this and other justifications into #[allow(rustc::potential_query_instability, reason = "<here>")]?
There was a problem hiding this comment.
Done. I moved the justifications into #[allow(rustc::potential_query_instability, reason = ...)] attributes across the touched sites.
- 为 potential_query_instability allow 补充 reason - 按 review 移除不必要的诊断候选排序
JinRudy
force-pushed
the
fix/potential-query-instability-comments
branch
from
0b7064a to
7767520
Compare
|
Addressed the review feedback:
Verification:
@rustbot review |
|
Hi, For the last few months we have been receiving an increased amount of LLM generated contributions. We have no way to distinguish your PR from other generated ones and thus unfortunately need to put some additional barriers for contributions in you way. We are a community of contributors, not just a code repository. We focus on contributors who desire to stay around and put in the work to produce high quality contributions or learn to do so. We are thus warning you as per our policies (1) and contribution standards (2) to stop generating PRs and comments. To make sure you can learn how to contribute, we invite you to join https://rust-lang.zulipchat.com/join/rlfvpemsaacs3pfi6kwqnqjb/ and start a thread asking for a mentor. Thanks for understanding Oli in the name of the mod team |
rustbot
removed
the
S-waiting-on-review
6 participants
Closes #84447.
This audits the remaining
#[allow(rustc::potential_query_instability)]sites that did not have a local justification.Changes:
unexpected_cfg_namecandidates before diagnostic matching so equally good suggestions are selected deterministically.Verification:
git diff --checkallow(rustc::potential_query_instability)sites without adjacent comments: no output./x fmt --check./x test tidy./x check compiler/rustc_attr_parsing compiler/rustc_expand compiler/rustc_interface compiler/rustc_codegen_ssa compiler/rustc_mir_build compiler/rustc_mir_dataflow