Implement `Distribution` for `hybrid_array::Array`

[daxpedda]

Because rand depends on chacha20, which in turn depends on hybrid-array, its isn't possible to implement Distribution for hybrid_array::Array in hybrid-array itself.

Therefor I propose adding the implementation in rand itself, behind a crate feature of course.

I understand that this is a big ask, rand isn't in the business of implementing traits for other types, its supposed to be the other way around. But AFAIK there isn't any other way to do this for hybrid_array::Array.

Happy to do the PR upon approval.

[dhardy]

@daxpedda I presume you mean Distribution<hybrid_array::Array<T, U>> for StandardUniform?

This appears to be due to the cipher dependency of chacha20 which isn't even used by rand. I think there are two ways to resolve this:

1. As suggested above (by making rand optionally dependent on hybrid_array)
2. By splitting chacha20, essentially into three parts: backend, cipher, rng (could be merged into rand)

Option 1 could be accepted, but option 2 is worth at least a brief consideration.

CC @newpavlov @tarcieri @nstilt1

[tarcieri]

I don’t think adding this impl is worth splitting up the chacha20 crate over

[dhardy]

By itself, no. I can accept an implementation within rand.

@daxpedda what's the motivation — a use case or just wanting to support Distribution?

[daxpedda]

No particularly motivating use-case.

I just keep stumbling on it during tests where I have to regularly write:

let mut array = Array::default();
rand::rng().fill_bytes(&mut array);

and I would like to instead write:

let array = rng::random();

It definitely helps that Array now implements proptest::Arbitrary, but not every test uses proptest.

I can accept an implementation within rand.

I will go ahead and make a PR then. But let me know if you change your mind in light of a lacking use-case.

[tarcieri]

@daxpedda actually that's the arbitrary::Arbitrary trait, which serves a similar purpose to the proptest::Arbitrary trait, but is actually related to rust-fuzz.

I don't know why proptest defines their own Arbitrary trait instead of using arbitrary.

[daxpedda]

@daxpedda actually that's the arbitrary::Arbitrary trait, which serves a similar purpose to the proptest::Arbitrary trait, but is actually related to rust-fuzz.

I don't know why proptest defines their own Arbitrary trait instead of using arbitrary.

I see, yeah that's unfortunate. I might be making a PR to add proptest support to hybrid-array in the future then.

[newpavlov]

I don't think that rand should add such implementation. What about other numerous types defined in third-party crates which may want a similar impl? If you work with a fixed size, then you should be able to simply wrap core array into Array, i.e. let array = Array(rng::random());. And it's not hard to write a one-line helper function for this.

[daxpedda]

What about other numerous types defined in third-party crates which may want a similar impl?

Just for context again: this is an exception because hybrid-array just can't implement it itself, because rand has it in its dependency tree.

If you work with a fixed size, then you should be able to simply wrap core array into Array, i.e. let array = Array(rng::random());. And it's not hard to write a one-line helper function for this.

Don't want to undermine my own case here, but just for full transparency, even without a fixed size its still a one-liner:

let array = Array::from_fn(|_| rng.random());

I don't really consider this a motivating use-case anyway. I think the bigger picture here is users who want to implement Distribution on their own types, with e.g. a derive macro, which would rely on underlying types implementing this trait. I also hardly have to explain here how traits can be used in more complex scenarios where generalized functions expect this trait and users might have to do some weird workarounds, like wrapping Array in a newtype to implement Distribution.

All in theory of course, we still don't have an actual use-case at hand.

[dhardy]

No particularly motivating use-case.

We do require motivation for additions.

Pertinent to this question is whether hybrid-array has a large user-base (it appears not) and whether there is an easy workaround (it appears so).

[daxpedda]

That's totally understandable! I don't have much else to add here.

Please feel free to close the issue and PR if there isn't anything else to add from your side here.

[tarcieri]

As a sidebar, I'll say I've been looking forward to consolidating dependencies around ChaCha20 in projects where I currently have both rand_chacha and chacha20.

As things stand, chacha20 is completely self-contained for the RNG use case, where rand_chacha has a ppv-lite86 dependency which has a transitive zerocopy dependency.

I think a big selling point of rand_chacha => chacha20 is going from rand_chacha+ppv-lite86+zerocopy => chacha20 for the RNG case, and if you were already using chacha20 you now have 1 dependency instead of 4.

Splitting chacha20 into 3 crates kind of defeats that. I'm not completely against splitting it up but if we did that I'd probably prefer to only have two crates: a "core" crate with the barebones Rust impl and one which impl's the cipher API, and in such a case I guess we could put RNG support in the "core" crate?

[dhardy]

@tarcieri I agree that the smaller dependencies of chacha20 is an improvement over rand_chacha, but think it's more significant to measure lines of code than crates. I don't think this issue alone is sufficient motivation for a split.

@daxpedda I just tested, and it is possible to have two packages with optional dependencies on the other, though of course both features can't be enabled simultaneously, so probably not useful to us here.

I will close this now. If we have stronger motivation then #1663 can be reconsidered.