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

fix: only generate trait bound for associated types in field types #15000

Merged
merged 1 commit into from Jun 7, 2023

Conversation

lowr
Copy link
Contributor

@lowr lowr commented Jun 7, 2023

Given the following definitions:

trait Trait {
    type A;
    type B;
    type C;
}

#[derive(Clone)]
struct S<T: Trait>
where
    T::A: Send,
{
    qualified: <T as Trait>::B,
    shorthand: T::C,
}

we currently expand the derive macro to:

impl<T> Clone for S<T>
where
    T: Trait + Clone,
    T::A: Clone,
    T::B: Clone,
    T::C: Clone,
{ /* ... */ }

This does not match how rustc expands it. Specifically, Clone bounds for T::A and T::B should not be generated.

The criteria for associated types to get bound seem to be 1) the associated type appears as part of field types AND 2) it's written in the shorthand form. I have no idea why rustc doesn't consider qualified associated types (there's even a comment that suggests they should be considered; see rust-lang/rust#50730), but it's important to follow rustc.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 7, 2023
@lowr lowr force-pushed the fix/builtin-derive-bound-for-assoc branch from e005bcf to 4f0c6fa Compare June 7, 2023 11:52
@lowr
Copy link
Contributor Author

lowr commented Jun 7, 2023

This should fix #14954, but I'd rather have the reporter confirm it (hence no issue linking) since I couldn't reproduce the ! part due to the lack of knowledge of other types involved.

@HKalbasi
Copy link
Member

HKalbasi commented Jun 7, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 7, 2023

📌 Commit 4f0c6fa has been approved by HKalbasi

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jun 7, 2023

⌛ Testing commit 4f0c6fa with merge 085a311...

@bors
Copy link
Collaborator

bors commented Jun 7, 2023

☀️ Test successful - checks-actions
Approved by: HKalbasi
Pushing 085a311 to master...

@bors bors merged commit 085a311 into rust-lang:master Jun 7, 2023
10 checks passed
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.

None yet

4 participants