Derived Clone implementation is sometimes not usable

[dimbleby]

At work we recently ran into a case that's essentially the same as #19034, in which #[deriving(Clone)] produces an implementation that's not usable when you'd hope - whereas the 'natural' implementation that any human would likely write is just fine.

So this is both less useful than it could be, and confusing. (It took us non-trivial time to discover #19034 and understand the problem).

#19034 was rejected on the grounds that it was "working as intended". I ask that this be revisited:

• is it really intended to produce a hobbled impl?
• even if yes, can't we have an issue requesting that this be improved?
• if the derived impl is for some reason unimprovable, would it be possible to give better explanation in the error?

(Meta: I feel kinda bad opening an issue that I know full well has been rejected once already. Please feel free to re-educate me if I should have gone about this some other way!)

[abonander]

I think the "working as intended" statement was a little bit misleading when the message was more like "yes, #[derive(Clone)] is naive, and we're pretty much fine with that".

At this point, changing the behavior of #[derive] is probably a breaking change, but it might not necessarily be in this case, given that it would only make more code work that didn't work before.

I think the major reason why naivety was preferred is that making this work robustly would have necessitated making the derive machinery even more complicated, and even then wouldn't have completely solved the problem.

At the point in compilation that deriving occurs, there's no way to ask the compiler if a certain type implements Clone and over what bounds, so you'd have to hardcode knowledge of container types that don't require Clone. However, this would only work for stdlib types and any other types that you could include without coming off as "blessing" particular crates, so then you'd still have people having the exact same problems, except with external types instead of Arc et al.

Still, I don't see much reason why not to try something like what Serde does, where you supply a second attribute which overrides the inferred bounds for the derived trait impls. Either way, a change like this would need to go through an RFC, I think.

In fact, since custom derives are stable now, I don't see any reason why not to attempt this in an external crate as a proof-of-concept, either the whitelisting idea or the override-bounds attribute idea. It may have been tried already, but I can't find anything relevant on crates.io.

Addendum: I do believe that custom derives can't override built-in derives, so you'd have to do this with procedural macro attributes instead, which are still unstable and don't come with the validation that custom derive does (you'd still have to assert that your attribute is being applied to a struct or enum), but it's doable.

[abonander]

cc @jseyfried

[durka]

I think the major reason why naivety was preferred is that making this work robustly would have necessitated making the derive machinery even more complicated, and even then wouldn't have completely solved the problem.

That's not the only reason. Currently #[derive] adds the necessary bounds directly to the type parameters. That's not the most sound approach, because as in #19034 the type parameters may be used in a way where T: Clone is either weaker or stronger than TypeOfEachField<T>: Clone (which is what the derived impl actually needs to guarantee).

You could generate a where clause that directly bounds the type of each field, but this is not possible for structs because some of those types might be private and that would be private types in a public interface. (N.B. It is possible for enums because you already can't put a private type in an enum.) There are some ways to fix this,

1. Allowing derived impls to cheat on that rule, or
2. "Elaborating" where clauses so that the compiler sees through where PrivateType<T>: Clone to see that it really means, let's say, where i32: Clone, Arc<T>: Clone and allows it because those aren't private.

@eddyb has advocated for (2) and I hope I haven't misstated it.

[petrochenkov]

@durka
It turns out that relaxing privacy checks is not enough :(
https://internals.rust-lang.org/t/lang-team-minutes-private-in-public-rules/4504/20

[abonander]

So really, my statement still holds. I guess you could use where clauses for structs with all public fields, or only apply them to the types of public fields, but it's still only a partial measure.

Letting the user override the bounds seems like a pretty good compromise to me, although I think I would prefer allowing a selective override instead of requiring them to specify all bounds.

[durka]

Agreed re compromise.

…

On Thu, Mar 23, 2017 at 4:18 PM, Austin Bonander ***@***.***> wrote: So really, my statement still holds. I guess you could use where clauses for structs with all public fields, or only apply them to the types of public fields, but it's still only a partial measure. Letting the user override the bounds seems like a pretty good compromise to me, although I think I would prefer allowing a selective override instead of requiring them to specify all bounds. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#40754 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAC3nyWUGOqOGuwgnxojD73eu7g11Rbvks5rotOogaJpZM4MmeON> .

[eddyb]

@petrochenkov We have to wait for the trait system overhaul (aka "chalk") but I still think there's some hope - Haskell can handle this case AFAIK, and there's at least some "dumb" way of handling #29859 where given #29859 (comment) you treat T: Magic as T: Magic + Copy which does give the right error.

[arielb1]

@eddyb

The current feeling is that we do not want arbitrary traits to be coinductive, even with chalk.

[Mark-Simulacrum]

Closing in favor of #26925. This specific issue doesn't really add anything by being standalone and that issue tracks the general problem of derive perhaps being not as intelligent as we may want.