Skip to content

refactor: arithmetic_side_effects: replace string-based type comparison with DefId/PrimTy lookup - #16730

Open
Tiauma wants to merge 4 commits into
rust-lang:masterfrom
Tiauma:my-arith-fix
Open

refactor: arithmetic_side_effects: replace string-based type comparison with DefId/PrimTy lookup#16730
Tiauma wants to merge 4 commits into
rust-lang:masterfrom
Tiauma:my-arith-fix

Conversation

@Tiauma

@Tiauma Tiauma commented Mar 19, 2026

Copy link
Copy Markdown

changelog: [arithmetic_side_effects]: replace string-based type comparison with DefId/PrimTy lookup

Previously, the lint compared types using their string representations,
which was inefficient and failed to recognize type aliases (e.g.,
core→std aliases). This PR:

  • Introduces GeneralTy enum to represent types as either PrimTy or DefId
  • Builds lookup maps at initialization using paths::lookup_path_str and local crate search
  • Properly resolves type aliases and paths across crates
  • Removes FxHashMap in favor of UnordMap/UnordSet for deterministic output
  • Preserves all existing functionality and passes tests

Fixes rust-lang/rust-clippy#16359

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

rustbot commented Mar 19, 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, dswij, llogiq, samueltardieu

@Tiauma

Tiauma commented Apr 1, 2026

Copy link
Copy Markdown
Author

r? samueltardieu

@rustbot rustbot assigned samueltardieu and unassigned llogiq Apr 1, 2026
@samueltardieu

Copy link
Copy Markdown
Member

@rustbot reroll

@rustbot rustbot assigned Jarcho and unassigned samueltardieu Jun 17, 2026
@llogiq

llogiq commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Sorry for the delay. I'll review right away.

r? @llogiq

@rustbot rustbot assigned llogiq and unassigned Jarcho Jun 28, 2026

@llogiq llogiq left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me. Thank you!

View changes since this review

@llogiq
llogiq added this pull request to the merge queue Jun 28, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 28, 2026
@samueltardieu

Copy link
Copy Markdown
Member

While rebasing the commits, you might also want to squash them.

@Tiauma

Tiauma commented Jun 28, 2026

Copy link
Copy Markdown
Author

Ok, thank you.

@llogiq
llogiq added this pull request to the merge queue Jul 2, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 2, 2026
@rustbot

rustbot commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (possibly #17369) made this pull request unmergeable. Please resolve the merge conflicts.

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

Labels

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.

Rewrite arithmetic_side_effects to not use matching on string representation of types

5 participants