5

[arithmetic_side_effects] Cache symbols by c410-f3r · Pull Request #10675 · rust...

 1 year ago
source link: https://github.com/rust-lang/rust-clippy/pull/10675
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

[arithmetic_side_effects] Cache symbols #10675

Conversation

Contributor

@c410-f3r c410-f3r

commented

Apr 20, 2023

edited by llogiq

An internal-only modification to speed up the processing of symbols because "intern" isn't very cheap, even more when you are doing the same thing for every method expression.

changelog: none

Collaborator

r? @llogiq

(rustbot has picked a reviewer for you, use r? to override)

rustbot

added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label

Apr 20, 2023

Contributor

Good catch!

@bors r+

Contributor

pushpin Commit 0b16f80 has been approved by llogiq

It is now in the queue for this repository.

Contributor

hourglass Testing commit 0b16f80 with merge bc9892d...

Contributor

broken_heart Test failed - checks-action_test

Contributor

Ah, we missed the changelog line. @bors retry

Contributor

hourglass Testing commit 0b16f80 with merge c976ad0...

1 similar comment

bors

merged commit c976ad0 into

rust-lang:master

Apr 20, 2023

5 checks passed

Contributor

Isn't it simpler to add that strings to symbol table, and then just filter on symbols, skipping str<->String comparison/conversions?

Contributor

Author

That is what I asked at #10615 (comment)

IIRC, it is not rare to see PRs in rust-last/rust trimming pre-registered symbols and Clippy already has a bunch of interns, therefore, caching seemed to be the natural choice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

llogiq

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

None yet

5 participants

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK