orion
orion copied to clipboard
Consider adding `Eq` where possible
When #[warn(clippy::derive_partial_eq_without_eq)]
is enabled, clippy will suggest to add Eq
to some types. We should make sure this makes sense before we add this, because removing them later will be a breaking change.
The additional semantic requirements for Eq
are laid out in the trait's docs. I think this is okay to implement at least for any of our types that are just "bags of bytes". The three relevant properties are:
- reflexivity:
a == a
- symmetry:
a == b
impliesb == a
- transitivity:
a == b
andb == c
impliesa == c
The type system will enforce that the bag-of-bytes types can only be compared with the same type, which guarantees that the underlying bytes semantically mean the same thing and that we're not, for example, comparing a private key and a hash. That's the only thing I can think of that might break one of the above properties, and it's not an issue.
Just for reference, clippy suggests the additional Eq
on the following types:
- X25519
PublicKey
+PrivateKey
+SessionKeys
(high-level API) - BLAKE2b
Hasher
enum
Good points @vlmutolo. Seems like this shouldn't be too much of an issue then.
In the const generics refactor, the byte containers will all look like Public<C, D>
. I think we can derive Eq
on PublicData
and the various D
storage types (e.g. ArrayData
). This means that Public
will implement Eq
if and only if the context parameter C
implements Eq
. So we can control which types implement Eq
just using the context parameter, which is in line with the spirit of the refactor: pushing logic (such as min, max sizes) into the context parameters and letting everything else be the same.
I imagine we'll want to implement Eq
for all the current byte container types, but it's good that we'll have the option to not implement it selectively.
Do you think it'd be better to postpone this to post const-generics, that you've been working on @vlmutolo?
I think we can go ahead and add this in the current implementation. I'll also make sure to add it in the const-generics branch.