Skip to content

fix from_over_into false positive with conflicting blanket From impl#16881

Merged
Alexendoo merged 3 commits intorust-lang:masterfrom
pocopepe:fix-16823
Apr 26, 2026
Merged

fix from_over_into false positive with conflicting blanket From impl#16881
Alexendoo merged 3 commits intorust-lang:masterfrom
pocopepe:fix-16823

Conversation

@pocopepe
Copy link
Copy Markdown
Contributor

@pocopepe pocopepe commented Apr 18, 2026

fixes #16823

This is my first contribution to Clippy, happy to make changes if the approach isn't quite right

changelog: [from_over_into]: don't lint when a blanket From impl would cause a coherence conflict

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 18, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 18, 2026

r? @dswij

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

Comment thread clippy_lints/src/from_over_into.rs Outdated
let Some(from_def_id) = cx.tcx.get_diagnostic_item(sym::From) else {
return false;
};
cx.tcx.all_impls(from_def_id).any(|impl_id| {
Copy link
Copy Markdown
Member

@Alexendoo Alexendoo Apr 25, 2026

Choose a reason for hiding this comment

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

This can use cx.tcx.non_blanket_impls_for_ty to avoid iterating over all From impls

Despite the name impl From<T> for Foo is stored in TraitImpls::non_blanket_impls, this disagrees with the glossary but I'm not sure which one is correct

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks for pointing it out, also dropped the self_ty == check since non_blanket_impls_for_ty already handles that

Copy link
Copy Markdown
Member

@Alexendoo Alexendoo left a comment

Choose a reason for hiding this comment

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

@Alexendoo Alexendoo added this pull request to the merge queue Apr 26, 2026
Merged via the queue into rust-lang:master with commit 57fdfaa Apr 26, 2026
11 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 26, 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.

from_over_into suggests turning Into into conflicting From

4 participants