RustCrypto / hybrid-array

Hybrid typenum/const generic arrays
Apache License 2.0
9 stars 8 forks source link

modify ArraySize to allow for arbitrary size support by third parties #81

Open conradludgate opened 3 months ago

conradludgate commented 3 months ago

I had this idea on discord to allow for third party crates to provide their own sizes. It means that ArrayN<T, N> no longer is guaranteed to always work, but it still unblocks users with odd sized arrays such that they can unsafe impl ArraySize themselves.

This also provides a best-effort invariant check that can catch wrong impls at compile time.

I appreciate if this design is undesirable, but I feel like #66 & #79 will be a never ending issue otherwise.

conradludgate commented 3 months ago

The invariant checker produces errors like

error[E0080]: evaluation of `<Size54321 as hybrid_array::ArraySize>::__CHECK_INVARIANT` failed
  --> /Users/conrad/Documents/code/rustcrypto/hybrid-array/src/traits.rs:23:9
   |
23 |         assert!(a == b, "ArraySize invariant violated");
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'ArraySize invariant violated', /Users/conrad/Documents/code/rustcrypto/hybrid-array/src/traits.rs:23:9
   |
   = note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info)

note: erroneous constant encountered
  --> /Users/conrad/Documents/code/rustcrypto/hybrid-array/src/from_fn.rs:27:33
   |
27 |         core::convert::identity(U::__CHECK_INVARIANT);
   |                                 ^^^^^^^^^^^^^^^^^^^^

note: the above error was encountered while instantiating `fn hybrid_array::from_fn::<impl hybrid_array::Array<u8, Size54321>>::try_from_fn::<std::convert::Infallible, {closure@hybrid_array::from_fn::<impl hybrid_array::Array<u8, Size54321>>::from_fn<{closure@tests/custom_size.rs:21:49: 21:52}>::{closure#0}}>`
  --> /Users/conrad/Documents/code/rustcrypto/hybrid-array/src/from_fn.rs:17:9
   |
17 |         Self::try_from_fn::<Infallible>(|n| Ok(f(n))).expect("should never fail")
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Which is a bit of soup - but the text "ArraySize invariant violated" does appear pretty early, also listing <Size54321 as hybrid_array::ArraySize> so you know which ArraySize impl is incorrectly implemented.

conradludgate commented 3 months ago

For convenience, it might make sense to introduce type USIZE: usize = <Self::Size as Unsigned>::USIZE;. Doing this locally, I managed to get hashes to build with only minimal changes needed to the traits/utils crates.

tarcieri commented 2 months ago

So, I think something like this is interesting and something we should probably explore, but I worry about including it in the initial generic-array -> hybrid-array migration, specifically because it removes the supertrait bound on Unsigned for ArraySize which makes a complicated migration that much harder.

I'd like to keep this PR open, but I think we should skip it for hybrid-array v0.2