Skip to content

Avoid unnecessary String allocations in MinifyingSugg arithmetic ops#17057

Merged
samueltardieu merged 1 commit into
rust-lang:masterfrom
shulaoda:05-23-avoid_unnecessary_allocations_in_arithmetic_ops
May 22, 2026
Merged

Avoid unnecessary String allocations in MinifyingSugg arithmetic ops#17057
samueltardieu merged 1 commit into
rust-lang:masterfrom
shulaoda:05-23-avoid_unnecessary_allocations_in_arithmetic_ops

Conversation

@shulaoda

Copy link
Copy Markdown
Contributor

Summary

In clippy_lints/src/loops/manual_memcpy.rs, the four Add/Sub trait impls on MinifyingSugg used the pattern:

match (self.to_string().as_str(), rhs.to_string().as_str()) {
    ("0", _) => rhs.clone(),
    (_, "0") => self.clone(),
    ...
}

Every call therefore allocated two Strings purely so they could be matched against the literal "0". The allocations were discarded immediately afterwards.

This PR:

  • Adds an inline is_zero() helper that pattern-matches Sugg::NonParen("0") without allocating. In this lint's data flow Sugg::NonParen is the only variant that can carry the textual value "0" (see sugg::ZERO and the literal path in get_offset), so the check is equivalent to the original to_string() == "0".
  • Rewrites the four Add/Sub impls to use is_zero() for the zero-side fast paths.
  • Keeps self.to_string() == rhs.to_string() for the a - a = 0 collapse inside Sub, but only as the final branch — it now only runs when both operands are confirmed non-zero, so the common case is fully allocation-free.

changelog: none

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 22, 2026
@rustbot

rustbot commented May 22, 2026

Copy link
Copy Markdown
Collaborator

r? @llogiq

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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 7 candidates
  • 7 candidates expanded to 7 candidates
  • Random selection from Jarcho, llogiq, samueltardieu

@samueltardieu samueltardieu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

r? samueltardieu
@rustbot author

View changes since this review

Comment thread clippy_lints/src/loops/manual_memcpy.rs Outdated
self.0
}

#[inline]

@samueltardieu samueltardieu May 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: this should not be needed, as a mod private function it will be inlined if the compiler thinks this is a good idea.

@rustbot rustbot assigned samueltardieu and unassigned llogiq May 22, 2026
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels May 22, 2026
@rustbot

rustbot commented May 22, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@shulaoda

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels May 22, 2026
@samueltardieu

Copy link
Copy Markdown
Member

Please squash the commits together (and "u" is hardly an acceptable commit message anyway, typo?).

@shulaoda shulaoda force-pushed the 05-23-avoid_unnecessary_allocations_in_arithmetic_ops branch from 11c156d to 7c1c1b0 Compare May 22, 2026 20:47
@shulaoda

Copy link
Copy Markdown
Contributor Author

Please squash the commits together (and "u" is hardly an acceptable commit message anyway, typo?).

Sorry about that. I usually use a simple "u" as shorthand for "update", since in previous contributions these commits were always squashed into a single commit named after the PR title, without keeping the individual commit messages.

@samueltardieu samueltardieu added this pull request to the merge queue May 22, 2026
Merged via the queue into rust-lang:master with commit 7019104 May 22, 2026
11 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants