subtle icon indicating copy to clipboard operation
subtle copied to clipboard

Add ConstantTimeEq implementation for [T; N].

Open branlwyd opened this issue 2 years ago • 2 comments
trafficstars

Only when the const-generics feature is enabled.

I had to modify a few tests: they were relying on [u8; N] being automatically dereferenced into &[u8] to get the blanket ConstantTimeEq implementation for [T]. But once a blanket implementation is also available for [T; N], Rust attempts to use it instead & then complains that it can't compare arrays of different lengths. I suppose this technically counts as a backwards-compatibility break.

branlwyd avatar Sep 07 '23 23:09 branlwyd

A few notes:

  • The implementation is borrowed from the blanket implementation of ConstantTimeEq for [T]. I have not tested that the generated code is constant-time, and I'm not sure of the best way to test this.

  • I could switch the [T; N] ConstantTimeEq implementation to call into the [T] implementation instead, possibly at the risk of introducing a pointless length comparison (if the compiler is not smart enough to optimize it out in this case). I'd be happy to make this change if the maintainers like that approach better.

branlwyd avatar Sep 07 '23 23:09 branlwyd

It's kind of crazy how it's able to use a different impl for [T; N] via deref coercion(?) currently.

I'm not sure where I stand on this being a breaking change or not. It definitely seems a lot more correct/type-safe for the impl for [T; N] to be present, but there is a possibility of it breaking working code when the feature is enabled (which could happen via spooky-action-at-a-distance via feature unification). Since it's breaking the tests, that isn't a great sign.

tarcieri avatar Sep 07 '23 23:09 tarcieri