Skip to content

Conversation

Alexendoo
Copy link
Member

On rust-lang/cargo@8ddf367 it's a slight improvement

$ hyperfine -w 2 -r 100 -p 'touch src/cargo/lib.rs' 'cargo +master clippy' 'cargo +preintern clippy'
Benchmark 1: cargo +master clippy
  Time (mean ± σ):      6.908 s ±  0.063 s    [User: 6.134 s, System: 0.690 s]
  Range (min … max):    6.757 s …  7.045 s    100 runs
 
Benchmark 2: cargo +preintern clippy
  Time (mean ± σ):      6.815 s ±  0.065 s    [User: 6.039 s, System: 0.692 s]
  Range (min … max):    6.653 s …  6.968 s    100 runs
 
Summary
  cargo +preintern clippy ran
    1.01 ± 0.01 times faster than cargo +master clippy

Part 1 of removing Symbol <-> str conversions

changelog: none

@rustbot
Copy link
Collaborator

rustbot commented Apr 18, 2025

r? @y21

rustbot has assigned @y21.
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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 18, 2025
@Alexendoo Alexendoo force-pushed the replace-interning-literals branch from 113528c to 4a9bbbe Compare April 18, 2025 21:27
@Alexendoo Alexendoo force-pushed the replace-interning-literals branch from 4a9bbbe to b52bd96 Compare April 18, 2025 22:44
Copy link
Member

@y21 y21 left a comment

Choose a reason for hiding this comment

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

This is a really nice change, so good that we can preintern our own symbols now 👍

@y21 y21 added this pull request to the merge queue Apr 21, 2025
Merged via the queue into rust-lang:master with commit cc00c77 Apr 21, 2025
11 checks passed
@Alexendoo Alexendoo deleted the replace-interning-literals branch April 21, 2025 15:08
@Alexendoo Alexendoo added the G-performance-project Goal: For issues and PRs related to the Clippy Performance Project label Apr 26, 2025
github-merge-queue bot pushed a commit that referenced this pull request Apr 27, 2025
Follow up to #14650

Replaces uses in the form `s.as_str() == "literal"`

r? @y21

changelog: none
github-merge-queue bot pushed a commit that referenced this pull request May 5, 2025
The `&[&str]` path based `clippy_utils` have been removed and replace
with a new type `PathLookup`:

-
[`match_trait_method`](https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/fn.match_trait_method.html)
-
[`match_qpath`](https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/fn.match_qpath.html)
-
[`match_path`](https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/fn.match_path.html)
-
[`match_any_def_paths`](https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/fn.match_any_def_paths.html)
-
[`match_def_path`](https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/fn.match_def_path.html)
-
[`match_type`](https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/ty/fn.match_type.html)
-
[`get_trait_def_id`](https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/fn.get_trait_def_id.html)

Internally `PathLookup` is a lazy call to `lookup_path` (the new name
for
[`def_path_res`](https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/fn.def_path_res.html)
to distinguish it from
[`path_res`](https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/fn.path_res.html))

The `invalid_paths` internal lint is removed, it could be reimplemented
but it feels redundant since every path should be covered by a test
anyway

### User facing changes

- `manual_saturating_arithmetic` now checks for `u32::MAX/MIN` instead
of only detecting the legacy numeric consts (`std::u32::MAX/MIN`),
`clippy::legacy_numeric_constants` will redirect usages of the legacy
versions to the new one

- `allow-invalid = true` now suppresses all invalid path warnings,
currently you can run into a warning that can't be ignored in some
situations, e.g. with `serde` without the `derive` feature

  ```
  warning: expected a macro, found a trait
   --> /home/gh-Alexendoo/temp/clippy.toml:2:5
    |
  2 |     { path = "serde::Serialize", allow-invalid = true },
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  ```

- Re-exports of primitives types like `std::primitive::*` no longer work
in `disallowed-types`, this seems acceptable since it would be unusual
to deny a primitive this way rather than writing e.g. `usize`. Type
aliases such as `c_char` are unaffected

- A similar slight performance improvement to
#14650
  ```bash
$ hyperfine -w 2 -p 'touch src/cargo/lib.rs' 'cargo +master clippy'
'cargo +lazy-paths clippy'
  ```
  ```
  Benchmark 1: cargo +master clippy
Time (mean ± σ): 6.829 s ± 0.064 s [User: 6.079 s, System: 0.673 s]
    Range (min … max):    6.705 s …  6.907 s    10 runs

  Benchmark 2: cargo +lazy-paths clippy
Time (mean ± σ): 6.765 s ± 0.064 s [User: 5.984 s, System: 0.698 s]
    Range (min … max):    6.636 s …  6.834 s    10 runs

  Summary
    cargo +lazy-paths clippy ran
      1.01 ± 0.01 times faster than cargo +master clippy
  ```

changelog: none
BD103 added a commit to TheBevyFlock/bevy_cli that referenced this pull request Jun 24, 2025
It was removed in rust-lang/rust-clippy#14650. We can either call `Symbol::intern()` manually (which is what this commit does), or use the new preinterned symbol system (https://doc.rust-lang.org/nightly/nightly-rustc/rustc_interface/interface/struct.Config.html#structfield.extra_symbols).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
G-performance-project Goal: For issues and PRs related to the Clippy Performance Project S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants