hash2curve: make hash_to_field output an array instead of taking an out parameter.

[carloskiki]

Discussed as part of #1295.

[daxpedda]

This was attempted before in RustCrypto/traits#872 (comment).

AFAICS we decided against it for two reason:

• Not all lengths can be expressed with ArraySize.
• Monomorphization causing extreme compilation and binary bloat.

[carloskiki]

• Not all lengths can be expressed with ArraySize.

I used hybrid_array::Array because that's what the crate uses elsewhere but in hindsight we could use primitive ([T; N]) arrays instead.

Monomorphization causing extreme compilation and binary bloat.

I don't think this is really that much of an issue, we are only using the function to generate arrays of 1 or 2 elements, and we may not even want to expose this function publicly. I think out parameters are generally harder to optimize than returning a value: https://www.youtube.com/watch?v=FnGCDLhaxKU&t=5458s, but that may not be true at all & is probably not worth changing a function for it.

I don't think this is a big deal at all, feel free to close this PR. I don't want to bikeshed over useless details, this was just something silly I came across when trying to grasp the codebase. I can also refactor to use [T; N] if you want the change.

[daxpedda]

Oh no, this is totally my bad. I totally jumped to conclusions without taking a proper look.
I was thinking of the length parameter in hash_from_bytes(), encode_from_bytes() and hash_to_scalar().

This is a great change! And yes, if you could change it to const N that would be nice.
FYI: I'm not a maintainer here, I'm just dropping my unasked 2¢ left and right :)

[daxpedda]

Actually, if we leave this to ArraySize instead of const C, we could do the following:

Suggested change

	T: FromOkm + Default,
	C: ArraySize,
	T: FromOkm + Default,
	C: ArraySize,
	T::Length: Mul<C, Output: IsLess<U65536, Output = True>>,

Eliminating one more run-time error.

However propagating this requirement up for users might be quite ugly. So I'm in favor of actually going ahead with the const generic, but thought I'd drop the thought at least.

[carloskiki]

Yes this is a good idea, but I don't like the complex trait bounds either. With inline consts we could do something like:

const { if N * T::Length > u16::MAX {
    compile_error!("...");
} };

Although this causes non-local errors and so I did not add it for now.