Skip to content

Conversation

flip1995
Copy link
Member

Alexendoo and others added 30 commits April 3, 2025 12:18
Turns out that `doc_markdown` uses a non-cheap rustdoc function to convert from markdown ranges into source spans. And it was using it a lot (about once every 18 lines of documentation on `tokio`, which ends up being about 1800 times). This ended up being about 18% of the total Clippy runtime as discovered by lintcheck --perf in docs-heavy crates. This PR optimizes one of the cases in which Clippy calls the function, and a future PR once pulldown-cmark/pulldown-cmark issue number 1034 is merged will be open. Note that not all crates were affected by this crate equally, those with more docs are affected far more than those light ones.
This is shorter, and also avoids overloading the `peel_blocks()` from `clippy_utils` with different semantics.
Placeholders are still given for the content of the whole block. However, if the result of the original `if let` or `match` expression was assigned, the assignment is reflected in the suggestion. No-op assignments (`let x = x;`) are skipped though, unless they contain an explicit type which might help the compiler (`let x: u32 = x;` is kept).
…inter (rust-lang#13319) Fixes https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Ambiguous.20default.20value.20for.20.60trivial-copy-size-limit.60 The current situation is | Target width | `trivial-copy-size-limit`| |--------|--------| | 8-bit | 2 | | 16-bit | 4 | | 32-bit | 8 | | 64-bit | 8 | ~~Since practically speaking it's almost always 8, let's go with that as the unconditional default to make it easier to understand~~ Now defaults to `target_pointer_width` changelog: [`trivial-copy-size-limit`] now also defaults to the size of a target pointer (unchanged for 64-bit targets)
This stops using `cargo fmt` and instead calls rustfmt directly with the list of all files. All `cargo fmt` does is find the crate roots and passes the edition from `cargo.toml`. Since the edition is set in `rustfmt.toml` for the test files and we're already iterating through all the files this is not needed. `--skip-children` is used since we already pass all the files, so the automatic detection isn't buying us anything other than running slower. ~Second commit~ (part of the first commit now) is a change to only use the `ignore` option in `rustfmt.toml` rather than having a way in `cargo dev fmt` to ignore files. r? @samueltardieu changelog: none
…t-lang#14693) Turns out that `doc_markdown` uses a non-cheap rustdoc function to convert from markdown ranges into source spans. And it was using it a lot (about once every 17 lines of documentation on `tokio`, which ends up being about 2000 times). This ended up being about 18% of the total Clippy runtime as discovered by lintcheck --perf in docs-heavy crates. This PR optimizes one of the cases in which Clippy calls the function, and a future PR once pulldown-cmark/pulldown-cmark#1034 is merged will be opened. This PR lands the use of the function into the single-digit zone. Note that not all crates were affected by this crate equally, those with more docs are affected far more than those light ones. changelog:[`clippy::doc_markdown`] has been optimized by 50%
Because the empty string is not a keyword.
rustc_on_unimplemented cleanups Addresses some of the fixmes from rust-lang#139091 and rust-lang#140307. - switch from `_Self` to `Self` in library - properly validate that arguments in the `on` filter and the format strings are actually valid See rust-lang/rustc-dev-guide#2357 for the relevant documentation.
…ust-lang#14735) Closes rust-lang/rust-clippy#14734 changelog: [`needless_for_each`] wrong suggestions when closure has no braces
Rust 1.88 introduces the `dangerous_implicit_autorefs` lint which warns about using implicit autorefs on a place obtained from a raw pointer, as this may create aliasing issues. Prevent `clippy::needless_borrow` from triggering in this case, by disabling the lint when taking a reference on a raw pointer dereference. There might be a better way for doing this in the long run with a finer way of distinguish the problematic cases, but this will prevent Clippy from contradicting the compiler in the meantime.
`if` expressions don't necessarily contain a block in the `else` part in the presence of an `else if`. The `else` part, if present, must be handled as a regular expression, not necessarily as a block expression.
…rochenkov Rename `kw::Empty` as `sym::empty`. Because the empty string is not a keyword. r? `@petrochenkov`
…ust-lang#14810) Rust 1.88 introduces the `dangerous_implicit_autorefs` lint which warns about using implicit autorefs on a place obtained from a raw pointer, as this may create aliasing issues. Prevent `clippy::needless_borrow` from triggering in this case, by disabling the lint when taking a reference on a raw pointer dereference. There might be a better way for doing this in the long run with a finer way of distinguish the problematic cases, but this will prevent Clippy from contradicting the compiler in the meantime. Fixes rust-lang/rust-clippy#14743 changelog: [`needless_borrow`]: do not contradict the compiler's `dangerous_implicit_autorefs` lint even though the refererences are not mandatory @rustbot label +beta-nominated <!-- TRIAGEBOT_START --> <!-- TRIAGEBOT_SUMMARY_START --> ### Summary Notes - [Beta nomination for 1.88](rust-lang/rust-clippy#14810 (comment)) by [samueltardieu](https://github.com/samueltardieu) Generated by triagebot, see [help](https://forge.rust-lang.org/triagebot/note.html) for how to add more <!-- TRIAGEBOT_SUMMARY_DATA_START$${"entries_by_url":{"https://github.com/rust-lang/rust-clippy/pull/14810#issuecomment-2883753957":{"title":"Beta nomination for 1.88","comment_url":"https://github.com/rust-lang/rust-clippy/pull/14810#issuecomment-2883753957","author":"samueltardieu"}}}$$TRIAGEBOT_SUMMARY_DATA_END --> <!-- TRIAGEBOT_SUMMARY_END --> <!-- TRIAGEBOT_END -->
So, after rust-lang/rust-clippy#14693 was merged, this is the continuation. It performs some optimizations on `Fragments::span` , makes it so we don't call it so much, and makes a 85.75% decrease (7.51% -> 10.07%) in execution samples of `source_span_for_markdown_range` and a 6.39% -> 0.88% for `core::StrSearcher::new`. Overall a 13.11% icount decrase on docs-heavy crates. Benchmarked mainly on `regex-1.10.5`. This means that currently our heaviest function is `rustc_middle::Interners::intern_ty`, even for documentation-heavy crates Co-authored-by: Roope Salmi <rpsalmi@gmail.com>
…14870) So, after rust-lang/rust-clippy#14693 was merged, this is the continuation. It performs some optimizations on `Fragments::span` , makes it so we don't call it so much, and makes a 85.75% decrease (7.51% -> 1.07%) in execution samples of `source_span_for_markdown_range` and a 6.39% -> 0.88% for `core::StrSearcher::new`. Overall a 13.11% icount decrase on docs-heavy crates. Benchmarked mainly on `regex-1.10.5`. @rustbot label +performance-project This means that currently our heaviest function is `rustc_middle::Interners::intern_ty`, even for documentation-heavy crates Along with rust-lang/rust-clippy#14693, this makes the lint a 7% of what it was before and makes it so that even in the most doc-heavy of crates it's not an issue. changelog:Optimize documentation lints by a further 85% r? @Jarcho
`ui_test` detects old style headers so it's redundant changelog: none
…#14859) Make lintcheck support different `CARGO_TARGET_DIR`, do not hardcode `target` (useful for perf) changelog:none
samueltardieu and others added 12 commits May 30, 2025 08:28
`is_trait_method` is not even used in this codeblock, whereas `implements_trait` is used but not imported (not sure if this is _actually_ a "changelog: none", since the documentation is at least contributor-facing) changelog: none
Correct comments. changelog: none
…4921) changelog: [`explicit_deref_methods`]: do not lint on method chains fixes rust-lang/rust-clippy#14593
Closes rust-lang/rust-clippy#14914 ---- changelog: [`dbg_macro`]: fix mishandling of async coroutine desugar
r? @ghost changelog: none
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 31, 2025
@rustbot
Copy link
Collaborator

rustbot commented May 31, 2025

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@rustbot
Copy link
Collaborator

rustbot commented May 31, 2025

⚠️ Warning ⚠️

@Manishearth
Copy link
Member

@bors r+ p=1

@bors
Copy link
Collaborator

bors commented Jun 2, 2025

📌 Commit 8ac5f7f has been approved by Manishearth

It is now in the queue for this repository.

@bors 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 Jun 2, 2025
@Manishearth
Copy link
Member

@bors rollup=never

@bors
Copy link
Collaborator

bors commented Jun 2, 2025

⌛ Testing commit 8ac5f7f with merge 2398bd6...

@bors
Copy link
Collaborator

bors commented Jun 2, 2025

☀️ Test successful - checks-actions
Approved by: Manishearth
Pushing 2398bd6 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 2, 2025
@bors bors merged commit 2398bd6 into rust-lang:master Jun 2, 2025
10 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 2, 2025
Copy link
Contributor

github-actions bot commented Jun 2, 2025

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 52882f6 (parent) -> 2398bd6 (this PR)

Test differences

No test diffs found

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \ test-dashboard 2398bd60ef526e686a38a299cc2fa991d8b3c33e --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. dist-aarch64-linux: 8499.6s -> 5291.7s (-37.7%)
  2. x86_64-apple-1: 7123.7s -> 9426.5s (32.3%)
  3. dist-apple-various: 6918.6s -> 7420.4s (7.3%)
  4. x86_64-apple-2: 4265.1s -> 4565.1s (7.0%)
  5. x86_64-msvc-2: 6541.9s -> 6951.5s (6.3%)
  6. aarch64-apple: 5447.6s -> 5717.3s (5.0%)
  7. dist-x86_64-netbsd: 5021.9s -> 5260.5s (4.8%)
  8. dist-armhf-linux: 5093.5s -> 5320.1s (4.4%)
  9. dist-loongarch64-linux: 6229.1s -> 6456.4s (3.6%)
  10. dist-powerpc64-linux: 5412.0s -> 5604.7s (3.6%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (2398bd6): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (secondary 1.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
1.6% [0.4%, 9.7%] 10
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.7% [-0.8%, -0.6%] 2
All ❌✅ (primary) - - 0

Cycles

Results (secondary -0.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.8% [0.4%, 1.5%] 5
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.7% [-1.0%, -0.5%] 7
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 743.462s -> 743.192s (-0.04%)
Artifact size: 372.27 MiB -> 372.27 MiB (0.00%)

flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 13, 2025
…nishearth Clippy subtree update r? `@Manishearth`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.