`AsRef<Array<_, _>>` for `[T; N]` is poisonous

[kayabaNerve]

I recently attempted to update to digest 0.11.0-rc.0 with some of my crates. It appears having hybrid-array anywhere in the dependency tree of a crate, not even in the immediate view, is sufficient for every existing [T; N]::as_ref call which prior worked to stop working unless the caller explicitly specifies what they wanted a reference of as now there are multiple candidates for which AsRef is to be invoked.

I wouldn't raise this as an issue, except since having hybrid-array anywhere as a dependency seems to trigger this, it causes any inclusion of hybrid-array in the dependency tree to require updating all parts of a codebase. I have several crates, some decently distanced from the underlying hashes, which would require updating.

I wanted to flag this before the digest 0.11 release was actually cut as hopefully, if this impl AsRef is removed, it won't require waiting for digest 0.12. Unfortunately, it would require a hybrid-array 0.4.0. With how pervasive this single impl is though, hence why my issue title calls it 'poisonous' as it poisons the entire tree, I'd hope to see this impl either removed or truly to be so beneficial, to be worth keeping. I don't have sufficient context to comment on the latter and concede that.

[newpavlov]

This is indeed unfortunate. But, arguably, the root problem is relying on the fragile inference (the language/Clippy should do a better job with warning against it, but alas). A proper solution would be either to replace .as_ref() with slicing (&array[..]), rely on Deref, or add an explicit type annotation let slice: &[T] = array.as_ref();. As the time debacle has shown, addition of such impls is not considered a breaking change and the problem should be fixed in downstream crates.

On the other hand, I understand the potential headache of dealing with this issue in downstream crates, especially when breakage is caused by a semver-compatible update of an upstream dependency, so I am on the fence about this, but lean slightly towards keeping the impl. I will leave the final decision to @tarcieri.

[kayabaNerve]

I agree it isn't the fault of hybrid-array and is something the users are at risk of.

I just also believe that crates which are annoying to users won't be used. I think this specific feature potentially causes enough friction it isn't worth the benefit offered, even if hybrid-array is completely correct :)

[tarcieri]

@kayabaNerve can you outline specific details of the exact the scenarios where you were having problems? The main one I've seen in the past is unnecessary calls to as_ref, where e.g. deref coercion would've sufficed instead, stop working.

These conversions are widely used in the RustCrypto codebase now. At a time when we're trying to wrap things up, it will entail releasing a hybrid-array v0.4 and updating everything to use it.

One of the biggest problems with generic-array has been ergonomics. This conversion provides a simple way to go from one reference type to another. There is the other approach supported by generic-arrray, a From impl from one reference type to another, but using it is much more awkward and a bit counterintuitive.

[kayabaNerve]

I have a codebase which used <[N; T] as AsRef<[N]>>::as_ref extensively. Was this my 'fault' for using as_ref instead of as_slice? Yes. If hybrid-array now appears anywhere in my dependency tree, do I need all following code, even that which doesn't import hybrid-array, to be updated (tens of mentions across multiple repositories at this point)? Yes.

I'm in full support of the transition from generic-array to hybrid-array. I tried the digest 0.11 pre-release because it offered functionality I needed due to the usage of hybrid-array. I don't love this annoyance however, and am unsure if the benefit offered exceeds the pain caused.

If you believe it's justified, either due to being brought up too late in the process or due to how it's the caller's fault, I won't push though. I wanted to raise it, not demand it.

Separately: we can table this for now and discuss if this should be a breaking change made in the future. We don't have to say, if this isn't reasonable to resolve by digest 0.11, it must make sense as a design decision. The fact digest 0.11 will force everyone to update their conflicting code makes this a bit of a moot point though. It'd only benefit users who didn't have digest >= 0.11 and adopted digest 0.12, or similar.

Also: my tree happened to compile without issue, with this removal included (k256, p256, sha2, blake2). I agree this is a simple way to convert references, but I'm unsure which dependencies actually use this exact trait implementation.

[tarcieri]

I have a codebase which used <[N; T] as AsRef<[N]>>::as_ref extensively.

I'll assume you mean <[T; N] as AsRef<[T]>>::as_ref.

I'm still curious what lead to an ambiguity here. I use as_ref on core arrays frequently in projects containing hybrid-array without issue.

Do you have a more concrete code example where the inference was ambiguous? A complete, self-contained, minimum failing example which reproduces the issue?

[kayabaNerve]

Right, sorry, regarding the type signature (though technically mine was correct, just against standard nomenclature 😅 I was still waking up).

For an immediate example, I'll refer to the monero-bulletproofs crate within https://github.com/monero-oxide/monero-oxide. Adding hybrid-array = "0.3.1" to monero-oxide/ringct/bulletproofs/Cargo.toml's dependencies and use hybrid_array; to the start of lib.rs (not importing anything from it, solely compiling it in) immediately causes 15 compilation errors. Some of these are because monero-bulletproofs was forked from a generic Bulletproofs library I wrote, which was generic over ff::PrimeField whose ::Repr only implements AsRef<[u8]> (justifying why this codebase uses AsRef::<[u8]>::as_ref instead of as_slice), yet now the codebase has been literally instantiated entirely around Ed25519 via curve25519-dalek. This lets Rust see through the Repr as [u8; 32] and detect the new candidates available (AsRef<[u8]> and AsRef<Array<u8, U32>>).

The entire monero-oxide repository has similar commentary available, along with my Serai repository (hosting code for DLEq proofs, threshold multisignatures via FROST, and some DKGs). I can produce a more minimal example if still requested, but hopefully that repository demonstrates the impact and the many places this can force updates to be required.

[tarcieri]

which was generic over ff::PrimeField whose ::Repr only implements AsRef<[u8]> (justifying why this codebase uses AsRef::<[u8]>::as_ref instead of as_slice)

If you were going through the Repr bounds exclusively, there wouldn't be any ambiguity.

Something is happening in your code where you're using AsRef in a situation where the correct impl to use can't be determined using the return type. So the compiler is seeing a concrete array type somewhere, and you're possibly invoking a needless AsRef in a situation where deref coercion would suffice, which is what I suggested might be going wrong earlier.

(Sidebar: I would've thought clippy would catch such cases via useless_asref though)

[kayabaNerve]

Right. Due to the history of the repository, I'm using as_ref when other options such as as_slice would be accepted, and now I have this conflict if I were to update to hybrid-array. This isn't hybrid-array's fault. I'm solely opening this issue as I'm unsure hybrid-array is better for introducing this impl and causing this discussion.

Also, I prefer as_ref to & in these circumstances as & itself is ambiguous between &[u8] and &[u8; 32]. That itself has caused me errors in the past and caused me to adopt as_ref as general practice. Apparently, to be free of all possible ambiguity, I should've started using as_slice ~3 years ago.

If I am the only person who eagerly used as_ref and would be affected by this, feel free to close. I raised this as I'm concerned other people will face this same annoyance.

clippy::useless_asref is only raised when as_ref transforms from T -> T. This from [u8; N] to &[u8], which are different types, hence why the lint doesn't apply.

[tarcieri]

I'm using as_ref when other options such as as_slice would be accepted

It sounds like you don't need either. Core array references coerce to slices automatically. But I will have to actually look over your code to check.

Edit: also TIL core arrays don't actually impl Deref/DerefMut, but use some more fundamental coercion magic to go from a sized to unsized reference

[kayabaNerve]

I don't believe these are originally references, at least, not always. I believe these (at least sometimes) are owned arrays which I'm converting to a slice. That's why I'd look to use &, as_ref, or as_slice. The issue with & is that it's ambiguous, hence as_ref, although that's now also ambiguous with hybrid-array in-tree 😅

as_slice or &a[..] (as @newpavlov noted) would be unambiguous. I concede the issue is my code is ambiguous. I'm just noting that when hybrid-array is an indirect dependency via all the new crypto releases (which I much look forward to), anyone else who unknowingly had potentially ambiguous code is going to have update all of it, and I'm asking if hybrid-array wants to force that or not. In some regards, hybrid-array is helping people out. In others, hybrid-array is annoying and why bother updating to that drama?

[tarcieri]

The issue with & is that it's ambiguous

I'm not sure in what way & would be ambiguous that would cause AsRef to resolve the ambiguity. I would continue to expect that the situation is in fact the opposite and a needless AsRef is what's introducing the ambiguity.

But I still need to look at your code, unless you can come up with a minimal self-contained example.

[kayabaNerve]

[b"abc", b"abcdef"].concat() fails to compile, as the types are different.
[&b"abc", b"abcdef"].concat() fails to compile, as &b"abc" produces a &&[u8; 3] which is a different type than &[u8; 6].
[b"abc".as_ref(), b"abcdef"].concat() compiles as the as_ref causes a type of &[u8] which &[u8; 6] automatically satisfies.

That's one reason I adopted as_ref > & throughout my codebase years ago. A distinct reason is I'm used to working with generics which only bound AsRef, so even when I'm no longer working with generics yet literal code, I'm used to simply writing as_ref.

[tarcieri]

Those aren't examples of "ambiguity" though, where the compiler can't resolve which concrete type to use. Those are simple type mismatches: you are trying to build heterogenously typed arrays (the concat is superfluous).

Again, I guess I'll have to look at your code and see if the as_refs can be replaced with automatic coercion. A self-contained reproduction of the specific problem you opened this issue about would continue to be very helpful until I do.