Use interned symbols instead of strings in more places #14840
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
the
S-waiting-on-review
samueltardieu
force-pushed
the
more-symbols-less-strings
branch
2 times, most recently
from
124cab7 to
e422ed7
Compare
Jarcho
requested changes
May 19, 2025
clippy_dev/src/fmt.rs
Outdated
| } | ||
|
|
||
| /// Format the symbols list | ||
| fn fmt_syms(check: bool) -> Result<(), Error> { |
There was a problem hiding this comment.
Can you switch this to use uitils::FileUpdater. Would be something like
FileUpdater::default().update_file_checked(
"clippy dev fmt",
mode,
path,
update_text_region_fn("start", "end", |dst| write_dst),
)There was a problem hiding this comment.
I didn't know FileUpdater, nice!
clippy_dev/src/fmt.rs
Outdated
| line.strip_suffix(',').unwrap_or(line).trim_end() | ||
| }) | ||
| .collect::<Vec<_>>(); | ||
| lines.sort_by_cached_key(|a| a.to_uppercase()); |
There was a problem hiding this comment.
Given that the order doesn't really matter, only that there is one, I would just sort normally. x.py tidy also sorts via a normal comparison.
samueltardieu
force-pushed
the
more-symbols-less-strings
branch
from
e422ed7 to
16087ca
Compare
This comment has been minimized.
This comment has been minimized.
samueltardieu
force-pushed
the
more-symbols-less-strings
branch
from
16087ca to
e16801e
Compare
|
Rebased |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch series makes heavy use of interned symbols when matching against known method names:
clippy_utils::symclippy dev fmt/clippy dev fmt --checkThe symbols are kept as-is (not rendered as strings) as much as possible to avoid needing locking the interner as much as possible. Static strings have been kept when they are only used when emitting a diagnostic, as there is no benefit in using interned strings for de-interning them right after.
changelog: none
r? @Alexendoo