Skip to content

Conversation

ada4a
Copy link
Contributor

@ada4a ada4a commented Sep 23, 2025

Based on #15520 (comment), with the following changes/additions:

  • No longer use the Trait enum for quickly filtering out irrelevant impls -- instead, check the trait_of basically right away:
    1. pre-fetch DefIds of the relevant traits into the lint pass
    2. reuse cx.tcx.impl_trait_ref's output for the the DefId of the trait being implemented
    3. compare those DefIds, which should be very cheap
  • Next, check whether self_ty implements the other relevant trait.
  • Pre-filter impl items in the same (lazy) iterator, but delay the proc-macro check as much as possible

changelog: none

Not auto-assigning since @blyxyas and/or @Jarcho will be the ones reviewing it:
r? ghost

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 23, 2025
@ada4a
Copy link
Contributor Author

ada4a commented Sep 23, 2025

Depends on #15520, so @rustbot label S-blocked

@rustbot rustbot added the S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work label Sep 23, 2025
Copy link

github-actions bot commented Sep 23, 2025

Lintcheck changes for 2168603

Lint Added Removed Changed
clippy::non_canonical_partial_ord_impl 0 1 0

This comment will be updated if you push new changes

@ada4a ada4a force-pushed the non_canonical_impls-2 branch 3 times, most recently from 7b7a795 to 9718c47 Compare September 23, 2025 22:32
@ada4a
Copy link
Contributor Author

ada4a commented Sep 24, 2025

The PR was merged, so @rustbot label -S-blocked

@rustbot rustbot removed the S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work label Sep 24, 2025
@ada4a ada4a force-pushed the non_canonical_impls-2 branch from 9718c47 to 948e23d Compare September 24, 2025 14:23
@rustbot
Copy link
Collaborator

rustbot commented Sep 24, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

The missing external macro test for `non_canonical_clone_impl` was
caught thanks to lintcheck -- I went ahead and added all the other
combinations of the test while at it
@ada4a ada4a force-pushed the non_canonical_impls-2 branch from 948e23d to 2168603 Compare September 24, 2025 15:18
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.

4 participants