add rustc lint, warning when iterating over hashmaps 2 #92584

lcnr · 2022-01-05T11:56:49Z

first introduced in #89558 and reverted in #90380 due to its perf impact

r? @estebank

lcnr · 2022-01-05T11:57:06Z

@bors try @rust-timer queue

rust-timer · 2022-01-05T11:57:07Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-01-05T11:57:13Z

⌛ Trying commit 1460dfd1204a3be4c74848d2e7e3019b30de4f6f with merge 446098f1f5b461056edc9b8846ce94ce7f829652...

lcnr · 2022-01-06T08:33:51Z

@bors try @rust-timer queue

rust-timer · 2022-01-06T08:33:53Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-01-06T08:34:06Z

⌛ Trying commit 016df18fe6ec89d1201779c62f52e343cad3201d with merge ad47090cb5e41b11dcd8682dee0cacc33b2e9903...

bors · 2022-01-06T10:09:09Z

☀️ Try build successful - checks-actions
Build commit: ad47090cb5e41b11dcd8682dee0cacc33b2e9903 (ad47090cb5e41b11dcd8682dee0cacc33b2e9903)

rust-timer · 2022-01-06T10:09:11Z

Queued ad47090cb5e41b11dcd8682dee0cacc33b2e9903 with parent f1ce0e6, future comparison URL.

rust-timer · 2022-01-06T11:39:27Z

Finished benchmarking commit (ad47090cb5e41b11dcd8682dee0cacc33b2e9903): comparison url.

Summary: This change led to very large relevant regressions 😿 in compiler performance.

Very large regression in instruction counts (up to 10.4% on incr-full builds of deeply-nested-async)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

lcnr · 2022-01-06T11:47:49Z

@bors try @rust-timer queue

rust-timer · 2022-01-06T11:47:50Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-01-06T11:47:57Z

⌛ Trying commit 81cf15730bec1716a4792fa472ae1dc6bd0d6a3f with merge f595a08cba4963a2575a9e38727108c748031450...

bors · 2022-01-06T13:38:19Z

☀️ Try build successful - checks-actions
Build commit: f595a08cba4963a2575a9e38727108c748031450 (f595a08cba4963a2575a9e38727108c748031450)

rust-timer · 2022-01-06T13:38:20Z

Queued f595a08cba4963a2575a9e38727108c748031450 with parent f1ce0e6, future comparison URL.

rust-timer · 2022-01-06T15:08:21Z

Finished benchmarking commit (f595a08cba4963a2575a9e38727108c748031450): comparison url.

Summary: This change led to very large relevant regressions 😿 in compiler performance.

Very large regression in instruction counts (up to 10.4% on incr-full builds of deeply-nested-async)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

bors · 2022-01-19T10:23:18Z

⌛ Trying commit 1b6090b274d15811c6a0e791e03c62a0c6bc8e6b with merge 5d17beb959299ea0fbd2279075b7f0746ad7f886...

bors · 2022-01-19T12:00:51Z

☀️ Try build successful - checks-actions
Build commit: 5d17beb959299ea0fbd2279075b7f0746ad7f886 (5d17beb959299ea0fbd2279075b7f0746ad7f886)

rust-timer · 2022-01-19T12:00:52Z

Queued 5d17beb959299ea0fbd2279075b7f0746ad7f886 with parent 2f004d2, future comparison URL.

rust-timer · 2022-01-19T14:16:02Z

Finished benchmarking commit (5d17beb959299ea0fbd2279075b7f0746ad7f886): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

lcnr · 2022-01-20T09:00:08Z

should now be ready for review/merged again

estebank

r=me after addressing the blanket import.

compiler/rustc_lint/src/internal.rs

estebank · 2022-02-01T01:14:04Z

compiler/rustc_lint/src/internal.rs

+        // FIXME(rustdoc): This lint uses typecheck results, causing rustdoc to
+        // error if there are resolution failures.
+        //
+        // As internal lints are currently always run if there are `unstable_options`,
+        // they are added to the lint store of rustdoc. Internal lints are also
+        // not used via the `lint_mod` query. Crate lints run outside of a query
+        // so rustdoc currently doesn't disable them.
+        //
+        // Instead of relying on this, either change crate lints to a query disabled by
+        // rustdoc, only run internal lints if the user is explicitly opting in
+        // or figure out a different way to avoid running lints for rustdoc.
+        if cx.tcx.sess.opts.actually_rustdoc {
+            return;
+        }


Do any other lints have to deal with this?

not sure, rustdoc disables the late lint query and afaik pretty much runs no lints outside of the internal lints (which we run because we always add them to the lint store if -Zunstable-options is set)

and no other internal lint uses the typeck query which is what is causing rustdoc to error

lcnr · 2022-02-01T09:59:16Z

@bors r=estebank rollup

bors · 2022-02-01T09:59:18Z

📌 Commit 4bbe970 has been approved by estebank

add rustc lint, warning when iterating over hashmaps 2 first introduced in rust-lang#89558 and reverted in rust-lang#90380 due to its perf impact r? `@estebank`

…askrgr Rollup of 7 pull requests Successful merges: - rust-lang#86374 (Enable combining `+crt-static` and `relocation-model=pic` on `x86_64-unknown-linux-gnu`) - rust-lang#91828 (Implement `RawWaker` and `Waker` getters for underlying pointers) - rust-lang#92021 (Eliminate duplicate codes of is_single_fp_element) - rust-lang#92584 (add rustc lint, warning when iterating over hashmaps 2) - rust-lang#93267 (implement a lint for suspicious auto trait impls) - rust-lang#93290 (remove `TyS::same_type`) - rust-lang#93436 (Update compiler_builtins to fix duplicate symbols in `armv7-linux-androideabi` rlib) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned estebank Jan 5, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 5, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 5, 2022

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 5, 2022

This comment has been minimized.

Sign in to view

lcnr force-pushed the query-stable-lint branch from 0bcb3c3 to 016df18 Compare January 5, 2022 12:25

lcnr marked this pull request as draft January 5, 2022 13:10

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jan 6, 2022

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 6, 2022

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 6, 2022

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regression. labels Jan 19, 2022

lcnr marked this pull request as ready for review January 19, 2022 14:55

lcnr removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 20, 2022

estebank approved these changes Feb 1, 2022

View reviewed changes

lcnr added 3 commits February 1, 2022 10:15

add a rustc::query_stability lint

a1a30f7

rustfmt is broken, manually reduce line length

6970547

review + rebase

4bbe970

lcnr force-pushed the query-stable-lint branch from 1b6090b to 4bbe970 Compare February 1, 2022 09:29

bors 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 Feb 1, 2022

matthiaskrgr mentioned this pull request Feb 1, 2022

Rollup of 8 pull requests #93541

Closed

matthiaskrgr mentioned this pull request Feb 1, 2022

Rollup of 7 pull requests #93548

Merged

bors merged commit 741b62a into rust-lang:master Feb 1, 2022

rustbot added this to the 1.60.0 milestone Feb 1, 2022

flip1995 mentioned this pull request Feb 10, 2022

Update Clippy #93865

Merged

This was referenced Nov 10, 2023

Lint for iterating over the keys in an unordered collection such as HashSet or HashMap rust-lang/rust-clippy#11788

Closed

Add lint for HashMap iteration serai-dex/serai#424

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add rustc lint, warning when iterating over hashmaps 2 #92584

add rustc lint, warning when iterating over hashmaps 2 #92584

lcnr commented Jan 5, 2022

lcnr commented Jan 5, 2022

rust-timer commented Jan 5, 2022

bors commented Jan 5, 2022

This comment has been minimized.

lcnr commented Jan 6, 2022

rust-timer commented Jan 6, 2022

bors commented Jan 6, 2022

bors commented Jan 6, 2022

rust-timer commented Jan 6, 2022

rust-timer commented Jan 6, 2022

lcnr commented Jan 6, 2022

rust-timer commented Jan 6, 2022

bors commented Jan 6, 2022

This comment has been minimized.

bors commented Jan 6, 2022

rust-timer commented Jan 6, 2022

rust-timer commented Jan 6, 2022

bors commented Jan 19, 2022

bors commented Jan 19, 2022

rust-timer commented Jan 19, 2022

rust-timer commented Jan 19, 2022

lcnr commented Jan 20, 2022

estebank left a comment

estebank Feb 1, 2022

lcnr Feb 1, 2022 •

edited

Loading

lcnr commented Feb 1, 2022

bors commented Feb 1, 2022

add rustc lint, warning when iterating over hashmaps 2 #92584

add rustc lint, warning when iterating over hashmaps 2 #92584

Conversation

lcnr commented Jan 5, 2022

lcnr commented Jan 5, 2022

rust-timer commented Jan 5, 2022

bors commented Jan 5, 2022

This comment has been minimized.

lcnr commented Jan 6, 2022

rust-timer commented Jan 6, 2022

bors commented Jan 6, 2022

bors commented Jan 6, 2022

rust-timer commented Jan 6, 2022

rust-timer commented Jan 6, 2022

lcnr commented Jan 6, 2022

rust-timer commented Jan 6, 2022

bors commented Jan 6, 2022

This comment has been minimized.

bors commented Jan 6, 2022

rust-timer commented Jan 6, 2022

rust-timer commented Jan 6, 2022

bors commented Jan 19, 2022

bors commented Jan 19, 2022

rust-timer commented Jan 19, 2022

rust-timer commented Jan 19, 2022

lcnr commented Jan 20, 2022

estebank left a comment

Choose a reason for hiding this comment

estebank Feb 1, 2022

Choose a reason for hiding this comment

lcnr Feb 1, 2022 • edited Loading

Choose a reason for hiding this comment

lcnr commented Feb 1, 2022

bors commented Feb 1, 2022

lcnr Feb 1, 2022 •

edited

Loading