Skip to content

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Jun 4, 2024

When given

trait SomeTrait {
    type SomeType<'a>;
}

#[derive(Clone)]
struct Foo<T: SomeTrait> {
    x: for<'a> fn(T::SomeType<'a>)
}

expand to

impl<T: ::core::clone::Clone + SomeTrait> ::core::clone::Clone for Foo<T>
    where for<'a> T::SomeType<'a>: ::core::clone::Clone {
    #[inline]
    fn clone(&self) -> Foo<T> {
        Foo { x: ::core::clone::Clone::clone(&self.x) }
    }
}

instead of the previous invalid

impl<T: ::core::clone::Clone + SomeTrait> ::core::clone::Clone for Foo<T>
    where T::SomeType<'a>: ::core::clone::Clone {
    #[inline]
    fn clone(&self) -> Foo<T> {
        Foo { x: ::core::clone::Clone::clone(&self.x) }
    }
}

Fix #122622.

@rustbot
Copy link
Collaborator

rustbot commented Jun 4, 2024

r? @Nadrieril

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 4, 2024
@estebank estebank added T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 4, 2024
@rust-log-analyzer

This comment has been minimized.

When given

```rust
trait SomeTrait {
    type SomeType<'a>;
}

struct Foo<T: SomeTrait> {
    x: for<'a> fn(T::SomeType<'a>)
}
```

expand to

```rust
impl<T: ::core::clone::Clone + SomeTrait> ::core::clone::Clone for Foo<T>
    where for<'a> T::SomeType<'a>: ::core::clone::Clone {
    #[inline]
    fn clone(&self) -> Foo<T> {
        Foo { x: ::core::clone::Clone::clone(&self.x) }
    }
}
```

instead of the previous invalid

```
impl<T: ::core::clone::Clone + SomeTrait> ::core::clone::Clone for Foo<T>
    where T::SomeType<'a>: ::core::clone::Clone {
    #[inline]
    fn clone(&self) -> Foo<T> {
        Foo { x: ::core::clone::Clone::clone(&self.x) }
    }
}
```

Fix rust-lang#122622.
@Nadrieril
Copy link
Member

Looks sensible, ty!

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 5, 2024

📌 Commit e7ad2da has been approved by Nadrieril

It is now in the queue for this repository.

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 5, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 5, 2024
When `derive`ing, account for HRTB on `BareFn` fields

When given

```rust
trait SomeTrait {
    type SomeType<'a>;
}

#[derive(Clone)]
struct Foo<T: SomeTrait> {
    x: for<'a> fn(T::SomeType<'a>)
}
```

expand to

```rust
impl<T: ::core::clone::Clone + SomeTrait> ::core::clone::Clone for Foo<T>
    where for<'a> T::SomeType<'a>: ::core::clone::Clone {
    #[inline]
    fn clone(&self) -> Foo<T> {
        Foo { x: ::core::clone::Clone::clone(&self.x) }
    }
}
```

instead of the previous invalid

```
impl<T: ::core::clone::Clone + SomeTrait> ::core::clone::Clone for Foo<T>
    where T::SomeType<'a>: ::core::clone::Clone {
    #[inline]
    fn clone(&self) -> Foo<T> {
        Foo { x: ::core::clone::Clone::clone(&self.x) }
    }
}
```

Fix rust-lang#122622.

<!--
If this PR is related to an unstable feature or an otherwise tracked effort,
please link to the relevant tracking issue here. If you don't know of a related
tracking issue or there are none, feel free to ignore this.

This PR will get automatically assigned to a reviewer. In case you would like
a specific user to review your work, you can assign it to them by using

    r​? <reviewer name>
-->
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 6, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#124731 (Add translation support by mdbook-i18n-helpers to bootstrap)
 - rust-lang#125168 (Match ergonomics 2024: align implementation with RFC)
 - rust-lang#125925 (Don't trigger `unsafe_op_in_unsafe_fn` for deprecated safe fns)
 - rust-lang#125966 (Implement `os_string_pathbuf_leak`)
 - rust-lang#125987 (When `derive`ing, account for HRTB on `BareFn` fields)
 - rust-lang#126045 (check_expr_struct_fields: taint context with errors if struct definit…)
 - rust-lang#126048 (Fix typos in cargo-specifics.md)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 6, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#124731 (Add translation support by mdbook-i18n-helpers to bootstrap)
 - rust-lang#125168 (Match ergonomics 2024: align implementation with RFC)
 - rust-lang#125925 (Don't trigger `unsafe_op_in_unsafe_fn` for deprecated safe fns)
 - rust-lang#125987 (When `derive`ing, account for HRTB on `BareFn` fields)
 - rust-lang#126045 (check_expr_struct_fields: taint context with errors if struct definit…)
 - rust-lang#126048 (Fix typos in cargo-specifics.md)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 55b76f4 into rust-lang:master Jun 6, 2024
@rustbot rustbot added this to the 1.80.0 milestone Jun 6, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 6, 2024
Rollup merge of rust-lang#125987 - estebank:issue-122622, r=Nadrieril

When `derive`ing, account for HRTB on `BareFn` fields

When given

```rust
trait SomeTrait {
    type SomeType<'a>;
}

#[derive(Clone)]
struct Foo<T: SomeTrait> {
    x: for<'a> fn(T::SomeType<'a>)
}
```

expand to

```rust
impl<T: ::core::clone::Clone + SomeTrait> ::core::clone::Clone for Foo<T>
    where for<'a> T::SomeType<'a>: ::core::clone::Clone {
    #[inline]
    fn clone(&self) -> Foo<T> {
        Foo { x: ::core::clone::Clone::clone(&self.x) }
    }
}
```

instead of the previous invalid

```
impl<T: ::core::clone::Clone + SomeTrait> ::core::clone::Clone for Foo<T>
    where T::SomeType<'a>: ::core::clone::Clone {
    #[inline]
    fn clone(&self) -> Foo<T> {
        Foo { x: ::core::clone::Clone::clone(&self.x) }
    }
}
```

Fix rust-lang#122622.

<!--
If this PR is related to an unstable feature or an otherwise tracked effort,
please link to the relevant tracking issue here. If you don't know of a related
tracking issue or there are none, feel free to ignore this.

This PR will get automatically assigned to a reviewer. In case you would like
a specific user to review your work, you can assign it to them by using

    r​? <reviewer name>
-->
@jplatte
Copy link
Contributor

jplatte commented Jun 19, 2024

Why would any part of the field type result in extra bounds at all? I thought any sort of "perfect derive" was blocked on an opt-in syntax, so private implementation details like field types don't accidentally leak into public impl headers.

@Nadrieril
Copy link
Member

We already to a little bit of looking into fields for cases like:

trait Trait {
    type Item;
}

#[derive(Clone)]
struct Struct<T: Trait>(T::Item);

The generated Clone impl has a T::Item: Clone bound (play) (on top of an unnecessary T: Clone bound). This only extends that mechanism a little bit.

@jplatte
Copy link
Contributor

jplatte commented Jun 21, 2024

Interesting. I don't really get why it would ever be needed for fn types, but I suppose it's a backwards-compatible change loosen the generated bounds later anyways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

#[derive(Clone)] fails for HRTB function type taking an associated type
6 participants