Add rustc lint, warning when iterating over hashmaps #89558

lcnr · 2021-10-05T14:28:55Z

r? rust-lang/wg-incr-comp

rust-highfive · 2021-10-05T14:28:58Z

(rust-highfive has picked a reviewer for you, use r? to override)

src/test/ui-fulldeps/internal-lints/query_stability.rs

estebank · 2021-10-05T16:12:28Z

src/test/ui-fulldeps/internal-lints/query_stability.stderr

@@ -0,0 +1,39 @@
+error: using `drain` can result in unstable query results


The lint looks useful in general for end users (I'm sure it's already in clippy). It'd be nice if we uplifted it someday.

estebank · 2021-10-05T16:13:26Z

@bors r+

We don't need to mark the Fx versions?

bors · 2021-10-05T16:13:27Z

📌 Commit 82fea72 has been approved by estebank

lcnr · 2021-10-05T16:20:30Z

@bors r-

somehow CI doesn't run at all for this PR and this lint causes a lot of warnings which is why i've opened rust-lang/compiler-team#465 for this.

We don't need to mark the `Fx versions?

The Fx versions are only type aliases, which are pretty much ignored by rustc (as you probably also painfully aware)

petrochenkov · 2021-10-05T16:23:12Z

@lcnr
We have some lint naming conventions - https://github.com/rust-lang/rfcs/blob/master/text/0344-conventions-galore.md#lints.
As you can see allow(query_stability) doesn't make much sense as a sentence.

lqd · 2021-10-07T11:02:43Z

Should we wait for the FCP to complete on rust-lang/compiler-team#465, or at least for the MCP to be announced in the t-compiler meeting ? (not that they would be controversial, of course; just opportunities for discussion and possible improvements)

cjgillot · 2021-10-08T17:53:12Z

Bookkeeping: #84447

lcnr · 2021-10-13T12:55:23Z

@petrochenkov renamed the lint to potential_query_instability.

and explicitly allowed this lint in every crate that is affected. Fixing all warnings as part of this PR is a lot of work, especially as some of those warnings don't seems trivial to fix

lcnr · 2021-10-23T07:49:34Z

this should be ready for merge now 🤔 going to deal with the FIXME(rustdoc) in a followup PR

estebank · 2021-10-23T17:42:34Z

compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs

@@ -830,6 +830,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
            self.tcx.type_of(def_id)
        };
        let substs = self.infcx.fresh_substs_for_item(span, def_id);
+        self.write_substs(hir_id, substs);


This change seems unrelated to the lint, right? It seems correct to do, I'm just surprised it was either not needed until now or exposed as needed in this PR.

estebank · 2021-10-23T17:44:34Z

We are allowing these in a lot of modules. Do we have plans to fix them at a later point?

@bors r+

bors · 2021-10-23T17:44:35Z

📌 Commit 22e1798 has been approved by estebank

lcnr · 2021-10-23T18:57:51Z

We are allowing these in a lot of modules. Do we have plans to fix them at a later point?

I am allowing these for all compiler crates rn and intend to slowly remove these crate in the future.
Fixing all the cases (or figuring out whether they are correct) isn't always trivial, so I didn't do that in this PR

Add rustc lint, warning when iterating over hashmaps r? rust-lang/wg-incr-comp

…askrgr Rollup of 3 pull requests Successful merges: - rust-lang#85254 (Normalize MIR with RevealAll before optimizations.) - rust-lang#89558 (Add rustc lint, warning when iterating over hashmaps) - rust-lang#90205 (Repace use of `static_nobundle` with `native_link_modifiers` within Rust codebase) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Add rustc lint, warning when iterating over hashmaps r? rust-lang/wg-incr-comp

…askrgr Rollup of 5 pull requests Successful merges: - rust-lang#85254 (Normalize MIR with RevealAll before optimizations.) - rust-lang#89558 (Add rustc lint, warning when iterating over hashmaps) - rust-lang#90100 (Skip documentation for tier 2 targets on dist-x86_64-apple-darwin) - rust-lang#90155 (Fix alignment of method headings for scannability) - rust-lang#90205 (Repace use of `static_nobundle` with `native_link_modifiers` within Rust codebase) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Add rustc lint, warning when iterating over hashmaps r? rust-lang/wg-incr-comp

…askrgr Rollup of 6 pull requests Successful merges: - rust-lang#85254 (Normalize MIR with RevealAll before optimizations.) - rust-lang#89558 (Add rustc lint, warning when iterating over hashmaps) - rust-lang#90100 (Skip documentation for tier 2 targets on dist-x86_64-apple-darwin) - rust-lang#90155 (Fix alignment of method headings for scannability) - rust-lang#90162 (Mark `{array, slice}::{from_ref, from_mut}` as const fn) - rust-lang#90205 (Repace use of `static_nobundle` with `native_link_modifiers` within Rust codebase) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Add rustc lint, warning when iterating over hashmaps r? rust-lang/wg-incr-comp

…askrgr Rollup of 6 pull requests Successful merges: - rust-lang#89558 (Add rustc lint, warning when iterating over hashmaps) - rust-lang#90100 (Skip documentation for tier 2 targets on dist-x86_64-apple-darwin) - rust-lang#90155 (Fix alignment of method headings for scannability) - rust-lang#90162 (Mark `{array, slice}::{from_ref, from_mut}` as const fn) - rust-lang#90221 (Fix ICE when forgetting to `Box` a parameter to a `Self::func` call) - rust-lang#90234 (Temporarily turn overflow checks off for rustc-rayon-core) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned estebank Oct 5, 2021

rust-highfive added the S-waiting-on-review label Oct 5, 2021

lcnr changed the title ~~Add lint warning against iterating over hashmaps~~ Add rustc lint warning against iterating over hashmaps Oct 5, 2021

lcnr changed the title ~~Add rustc lint warning against iterating over hashmaps~~ Add rustc lint warning when iterating over hashmaps Oct 5, 2021

lcnr mentioned this pull request Oct 5, 2021

lint internal hashmap iteration rust-lang/compiler-team#465

Closed

3 tasks

estebank reviewed Oct 5, 2021

View changes

src/test/ui-fulldeps/internal-lints/query_stability.rs Outdated Show resolved Hide resolved

estebank reviewed Oct 5, 2021

View changes

bors added S-waiting-on-bors and removed S-waiting-on-review labels Oct 5, 2021

bors added S-waiting-on-author and removed S-waiting-on-bors labels Oct 5, 2021

lcnr force-pushed the query-stable-lint branch from 82fea72 to 2cad766 Oct 5, 2021

This comment has been hidden.

Sign in to view

cjgillot self-assigned this Oct 5, 2021

lcnr changed the title ~~Add rustc lint warning when iterating over hashmaps~~ Add rustc lint, warning when iterating over hashmaps Oct 5, 2021

lcnr force-pushed the query-stable-lint branch from 2cad766 to 0031ac8 Oct 13, 2021

This comment has been hidden.

Sign in to view

lcnr force-pushed the query-stable-lint branch from f1a64d7 to 729fa25 Oct 13, 2021

This comment has been hidden.

Sign in to view

lcnr force-pushed the query-stable-lint branch from 729fa25 to 1e13c75 Oct 13, 2021

This comment has been hidden.

Sign in to view

lcnr force-pushed the query-stable-lint branch from 613fd07 to 4724ee5 Oct 21, 2021

ignore `potential_query_instability` in rustdoc

22e1798

lcnr force-pushed the query-stable-lint branch from 4724ee5 to 22e1798 Oct 21, 2021

estebank reviewed Oct 23, 2021

View changes

estebank approved these changes Oct 23, 2021

View changes

cjgillot removed their assignment Oct 23, 2021

bors added S-waiting-on-bors and removed S-waiting-on-author labels Oct 23, 2021

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 23, 2021

Rollup merge of rust-lang#89558 - lcnr:query-stable-lint, r=estebank

65c1a2e

Add rustc lint, warning when iterating over hashmaps r? rust-lang/wg-incr-comp

matthiaskrgr mentioned this pull request Oct 23, 2021

Rollup of 3 pull requests #90217

Closed

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 24, 2021

Rollup merge of rust-lang#89558 - lcnr:query-stable-lint, r=estebank

9bb1c45

Add rustc lint, warning when iterating over hashmaps r? rust-lang/wg-incr-comp

matthiaskrgr mentioned this pull request Oct 24, 2021

Rollup of 5 pull requests #90222

Closed

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 24, 2021

Rollup merge of rust-lang#89558 - lcnr:query-stable-lint, r=estebank

d52a126

Add rustc lint, warning when iterating over hashmaps r? rust-lang/wg-incr-comp

matthiaskrgr mentioned this pull request Oct 24, 2021

Rollup of 5 pull requests #90224

Closed

matthiaskrgr mentioned this pull request Oct 24, 2021

Rollup of 6 pull requests #90226

Closed

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 24, 2021

Rollup merge of rust-lang#89558 - lcnr:query-stable-lint, r=estebank

a5ddd8b

Add rustc lint, warning when iterating over hashmaps r? rust-lang/wg-incr-comp

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 24, 2021

Rollup merge of rust-lang#89558 - lcnr:query-stable-lint, r=estebank

aade408

Add rustc lint, warning when iterating over hashmaps r? rust-lang/wg-incr-comp

matthiaskrgr mentioned this pull request Oct 24, 2021

Rollup of 4 pull requests #90229

Closed

matthiaskrgr mentioned this pull request Oct 24, 2021

Rollup of 6 pull requests #90235

Merged

bors merged commit 87822b2 into rust-lang:master Oct 24, 2021
10 checks passed

rustbot added this to the 1.58.0 milestone Oct 24, 2021

Sep	OCT	Nov
	28
2020	2021	2022

rust-lang / rust Public

Add rustc lint, warning when iterating over hashmaps #89558

Add rustc lint, warning when iterating over hashmaps #89558

lcnr commented Oct 5, 2021

rust-highfive commented Oct 5, 2021

estebank Oct 5, 2021

estebank commented Oct 5, 2021

bors commented Oct 5, 2021

lcnr commented Oct 5, 2021

petrochenkov commented Oct 5, 2021

This comment has been hidden.

lqd commented Oct 7, 2021 •

edited

cjgillot commented Oct 8, 2021

This comment has been hidden.

lcnr commented Oct 13, 2021

This comment has been hidden.

This comment has been hidden.

This comment has been hidden.

This comment has been hidden.

lcnr commented Oct 23, 2021

estebank Oct 23, 2021

estebank commented Oct 23, 2021

bors commented Oct 23, 2021

lcnr commented Oct 23, 2021

		@@ -0,0 +1,39 @@
		error: using `drain` can result in unstable query results

rust-lang / rust Public

Add rustc lint, warning when iterating over hashmaps #89558

Add rustc lint, warning when iterating over hashmaps #89558

Conversation

lcnr commented Oct 5, 2021

rust-highfive commented Oct 5, 2021

estebank Oct 5, 2021

estebank commented Oct 5, 2021

bors commented Oct 5, 2021

lcnr commented Oct 5, 2021

petrochenkov commented Oct 5, 2021

This comment has been hidden.

lqd commented Oct 7, 2021 • edited

cjgillot commented Oct 8, 2021

This comment has been hidden.

lcnr commented Oct 13, 2021

This comment has been hidden.

This comment has been hidden.

This comment has been hidden.

This comment has been hidden.

lcnr commented Oct 23, 2021

estebank Oct 23, 2021

estebank commented Oct 23, 2021

bors commented Oct 23, 2021

lcnr commented Oct 23, 2021

lqd commented Oct 7, 2021 •

edited