impl Clone/Send/Sync on the builder type if all member types support it

Question

impl Clone/Send/Sync on the builder type if all member types support it

Closed this issue 4 years ago · 5 comments

Not sure how possible is it, but I can try.

Answer 1 · 2021-01-16T13:15:47.000Z

I think you might get this one for free now if you just emit #[derive(::core::Clone)] on the builder struct, even if I don't really see the use for it with this particular crate. Send and Sync are already available if I'm not entirely mistaken.

Answer 2 · 2021-01-16T13:28:55.000Z

@Tamschi That won't work, because some fields may be of types that are not Clone and in that case the builder type itself - which may contain these types - cannot be Clone: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=3d10fc1ab0b265f0cb2f74ddf14664c4

To support this, I'll need to make the macro emit an impl Clone for with a constraint that all the generic types in it are Clone.

Answer 3 · 2021-01-16T15:08:42.000Z

All the data (PhantomData aside) is inside a tuple that's behind a generic parameter though, right?

Tuples are automatically Clone if all their items are Clone, PhantomData is Copy regardless of its type parameters and I think #[derive(Clone)] emits : Clone constraints for each generic type parameter.

Edit: I looked up the relevant documentation:
https://doc.rust-lang.org/std/primitive.tuple.html#trait-implementations-1
https://doc.rust-lang.org/std/clone/trait.Clone.html#derivable

Answer 4 · 2021-01-16T17:09:04.000Z

I thought about this a bit more and it would likely be better to implement it "manually", conditional only on the fields tuple.
(This would be to avoid generic type parameters interfering with the impl, so the builder would remain Clone until non-Clone fields are actually in use. It's quite a fortunate side effect of the generics scheme you came up with.)

Send and Sync could benefit from the same, since the phantom is only used to make sure the generics compile.

I might give this a shot once I'm more familiar with the code base. I'm working a feature suggestion for now.

Answer 5 · 2021-01-16T18:36:25.000Z

Good point. When I opened this ticket the builder had a separate field for each field of the struct, but since I've changed it to be a tuple it should be dead easy to implement a clone now. I'll give it a try.