Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass correct substs to implements_trait in incorrect_impls #11122

Merged
merged 1 commit into from
Jul 7, 2023

Conversation

Noratrieb
Copy link
Member

Copy<T> does in fact not exist. The substs on the trait_ref contain the Self type of the impl as the first parameter, so passing that to implements_trait, which then nicely prepends the Self type for us does not end will.

fixes #11121

The assertions requires debug assertions inside rustc, which is probably why it didn't fire here. I tested the change locally in rust-lang/rust and it did not ICE anymore.

cc @xFrednet @Centri3

changelog: [incorrect_impls]: fix confusion about generic parameters

`Copy<T>` does in fact not exist. The substs on the trait_ref contain
the `Self` type of the impl as the first parameter, so passing that
to `implements_trait`, which then nicely prepends the `Self` type
for us does not end will.
@rustbot
Copy link
Collaborator

rustbot commented Jul 7, 2023

r? @Manishearth

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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 7, 2023
@@ -80,9 +76,9 @@ impl LateLintPass<'_> for IncorrectImpls {
&& let Some(copy_def_id) = cx.tcx.get_diagnostic_item(sym::Copy)
&& implements_trait(
cx,
hir_ty_to_ty(cx.tcx, imp.self_ty),
trait_impl.self_ty(),
Copy link
Member Author

Choose a reason for hiding this comment

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

I also removed a needless hir_ty_to_ty here.

@flip1995
Copy link
Member

flip1995 commented Jul 7, 2023

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Jul 7, 2023

📌 Commit 5df1f66 has been approved by flip1995

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jul 7, 2023

⌛ Testing commit 5df1f66 with merge 2eff2f2...

@bors
Copy link
Collaborator

bors commented Jul 7, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 2eff2f2 to master...

@bors bors merged commit 2eff2f2 into rust-lang:master Jul 7, 2023
4 checks passed
@Noratrieb Noratrieb deleted the SUBSTITUTION-INITIATED branch July 7, 2023 19:27
@xFrednet xFrednet added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 7, 2023
@flip1995
Copy link
Member

Does not need a backport: The fix that was merged in the Rust repo directly already made it to beta. The only difference is, that the version on beta still uses the hir_ty_to_ty function. But this shouldn't be a problem.

@flip1995 flip1995 removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Aug 17, 2023
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.

wrong number of generic parameters for DefId(2:2695 ~ core[747e]::marker::Copy): [Bad, Bad]
6 participants