Add borrow impl for Dname<Vec<u8>>

[iximeow]

This impl supports users of domain that may use Dname as keys in standard library collections, who may want to query those collections with non-owned Dname.

Include a doc comment testing that a collection of Dname<Vec<u8>> can be queried with a Dname<[u8]>.

this comes from talking with @edmonds today, who was trying to do the seemingly-reasonable operation of building a map of Dname and associated attributes, then looking entries in that map up from a Dname built from a &[u8]. unfortunately, i think the best we can do is best-effort implementations of Borrow for T: AsRef<[u8]> that seem "useful enough"; there would need to be another impl for someone who has made a collection keyed on Dname<Bytes>, for example.

ideally we'd be able to write impl<Octs: AsRef<[u8]>> Borrow<Dname<[u8]>> for Dname<Octs>, but rustc (unfortunately, correctly), realizes that that permits impl Borrow<Dname<[u8]>> for Dname<[u8]>, which conflicts with the blanket impl of Borrow<T> for T in stdlib. i figured noting that wrinkle is helpful in the doc comment for any future confused souls, but i don't know your appetite for other Borrow impls (or this one!) in domain itself - users could add a newtype in their code and write an appropriate Borrow impl for any owned octet collections they're using in their containers.

edit: turns out the above issue was trying to write an impl<Octs: AsRef<[u8]> + ?Sized>, but impl<Octs: AsRef<[u8]> alone is accepted by rustc and looks to do the right things.

[partim]

Thank you for the PR. Those missing Borrow<_> impls certainly are an oversight.

Curiously, I added a blanket impl along the lines you mentioned as a starting point to try out a few work-arounds and the compiler just accepted it. The impl in question was

impl<Octs: AsRef<[u8]>> core::borrow::Borrow<Dname<[u8]>> for Dname<Octs> {
    fn borrow(&self) -> &Dname<[u8]> {
        self.for_slice()
    }
}

What error did you get?

[iximeow]

oh! i took poor notes and confused myself just the same. the issue actually appears with slightly more permissive bounds:

impl<Octs: AsRef<[u8]> + ?Sized> borrow::Borrow<Dname<[u8]>> for Dname<Octs> {
    fn borrow(&self) -> &Dname<[u8]> {
        self.for_slice()
    }    
}

which rustc rejects with:

error[E0119]: conflicting implementations of trait `Borrow<name::dname::Dname<[u8]>>` for type `name::dname::Dname<[u8]>`
   --> src/base/name/dname.rs:858:1
    |
858 | impl<Octs: AsRef<[u8]> + ?Sized> borrow::Borrow<Dname<[u8]>> for Dname<Octs> {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: conflicting implementation in crate `core`:
            - impl<T> Borrow<T> for T
              where T: ?Sized;

For more information about this error, try `rustc --explain E0119`.
error: could not compile `domain` (lib) due to previous error

thinking about it more, i'm not sure where someone would have a container keyed on Dname<Octs: !Sized> so the extra + ?Sized bound is probably excessive?

thanks for double-checking there, i'm certainly glad to see using just AsRef<[u8]> works! if you want to go want to go with that i'm happy to update the PR accordingly.

[partim]

Ah yes, that explains it. In fact, I’ve had this case before where the missing ?Sized trait bound allowed for a blanket impl. Rather quite convenient. Given that [u8] realistically is the only unsized octets sequence and it is covered by core’s impl<T> Borrow<T> for T, this should all be perfect.

[iximeow]

(updated the PR to a more generic Octs: AsRef<[u8]> impl of Borrow)

[iximeow]

@partim was there anything else to do with this on my side? i'm not sure if we've accidentally deadlocked waiting for the other to say something 😁

[partim]

Oh, my apologies. I completely forgot about the PR.

I’m now wondering if we should perhaps also have AsRef<_> impls, but that can happen in a separate PR.

[partim]

@iximeow Could you merge main into the branch so we get the CI to go green?

[iximeow]

aha, i hadn't quite realized that CI runs rely on your approval since this is an external PR. and then it's using main's ci.yml, running clippy, and my old branch still had lints to deny. cargo clippy --all-features -- -D warnings is clean (recent nightly though) - i expect CI ought to pass now.

there probably should be an AsRef<_> impl, yeah. i can do that as a follow-up.

[partim]

Looking good now – thank you for the PR!

I’ll add the AsRef<_> impl quickly before releasing 0.8.1 – it should really be in there, too.