Feedback requested on RFC 1028: potential breaking change

I would like to solicit community feedback on a possible breaking change. There has been an RFC (and implementation) to change deriving so that deriving subtraits, like Copy, Ord, and Eq, would imply deriving their respective supertraits.

In other words:

#[derive(Copy)] equivalent to #[derive(Copy, Clone)]
#[derive(Ord)] equivalent to #[derive(Ord, PartialOrd, Eq, PartialEq)]
#[derive(Eq)] equivalent to #[derive(Eq, PartialEq)]

(Actually, I believe the RFC only says that Ord implies PartialOrd, but I think it makes sense to go the whole way.) The goal here is primarily to reduce the verbosity that is currently required, though it can also allow derive to produce more efficient impls of Clone (since derive would know that the type is also Copy).

However, this is a breaking change. If you currently use #[derive] to derive (e.g.) Eq, but you manually implement PartialEq, then your code would now fail due to coherence errors. This is because #[derive(Eq)] would now also supply an impl for PartialEq. This is of course relatively easy to fix – either remove the manual impl of PartialEq, or else convert the derive(Eq) into a manual impl.

This has been a long-requested feature. Unfortunately, it didn’t make it into the beta. This means that if we are going to introduce this feature, we have to do it now – or at least we have to wait until we have specialization or some other means for derive to ‘conditionally’ generate these impls.

At this point, the RFC has relatively little feedback – what comments there are are primarily positive, but it would be helpful to get more feedback overall.

In particular: are the improved ergonomics here worth the potential breakage during beta?

6 Likes

(Oh, please put comments on the RFC, not here.)

I’m in for the change. Sacrifices will have to be made either with breaking changes or regret… I vote to break things, at least until 1.0 is out.

I think it’s a good idea. Is this forwards-compatible with a hypothetical extendable #[derive(…)] in the future? So, in general, when there are two traits A and B that are derivable, A: B could imply that #derive(A) means #derive(A, B). Are there any other autoderivable stdlib traits with such a relation that are not covered here, that might make this more difficult?

+1 I think it would make it derivations cleaner; it’s not a lot of code to refactor (for me, anyway)

+1 Well worth the short term pain.

We don’t need a breaking change, can’t we expand deriving in a non-breaking way? That way we have time to do it during a normal release cycle too and ship it earliest as 1.1.

For example what about #[derive(Ord="full")] to derive the whole shebang, or some similar backwards compatible expansion of the current feature.

I think we need to calmly look at nice generic solutions just like we used to. Always deriving multiple traits is less flexible, and useful in less situations, even though it is easier when it is applicable. Even just giving the trait group a new derive name is a simpler and more broadly useful solution: #[derive(TotalOrdering)]

1 Like

What about adding a #[deriveOnly(X)]? where this does not happen? That way current code can be ported 1:1 to deriveOnly, which is useful in itself in case one doesn’t want the “new” behavior of derive.

1 Like

Alternatively, #[derive(Eq)] could only derive PartialEq if a manual implementation of PartialEq doesn’t exist, or it could create a PartialEq implementation that can be overridden by a manual implementation in some way.

1 Like

I think this is a somewhat dangerous precedent to set. Provided it’s absolutely clear that changes of this nature are only going to be allowed prior to the full 1.0.0 release, I think it’s a good change to make. I think that, as a community, we have to be careful to not promulgate the idea that this whole “semantic versioning” thing is just hot air.

That said, as a way to potentially make this backward-compatible and uphold the semver promises: what if Rust had a concept of #[weak] impls. If you restricted them so that you can only ever have one for a given (type, trait) combo, and you can only write one in the same module as the type itself, you could switch #[deriving] to use them later on without violating coherence or backward compatibility.

+1. I think this is worth the breaking change. I would honestly be surprised if this broke more than a handful of crates.

I don’t have an opinion on the specific issue of derive, but this is turning into a referendum on to what degree the community is bothered by breaking changes. I think it would be interesting to quantify that, i.e. “we promise we won’t make any single change that breaks more than X% of crates.io, or YY% total during the beta period.” My understanding is that you guys are now generating reports of this sort, and this wouldn’t be nearly as far-fetched as it would be in other communities.

I’d believe that this will be breaking roughly 100% of the crates. It’s easy to fix though.

I like this change, but I would prefer that we try as hard as we can to make it non-breaking, like suggested in the quoted post.

Put another way, I would prefer not having this change then to introducing a non-bug-fix breaking change into a beta build.

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.