rand_core: add blanket impl of TryRngCore for RngCore

[newpavlov]

This PR removes the hacky impl_try_rng_from_rng_core and impl_try_crypto_rng_from_crypto_rng macros and replaces them with blanket impls of TryRngCore for RngCore and TryCryptoRng for CryptoRng.

This change means that TryRngCore/TryCryptoRng no longer can have blanket impls for &mut R and Box<R>. But I think it should be tolerable since most users will be using RngCore/CryptoRng, which have blanket impl for DerefMut (it covers both &mut R and Box<R>).

[newpavlov]

Strictly speaking, this change is not needed, but I made this signature symmetric with try_from_rng, which has to explicitly accept &mut impl TryRngCore. Plus, in our RustCrypto experience, it's easier for new users to understand &mut impl RngCore than to find that impl RngCore can accept &mut R.

[dhardy]

The rationale behind the current design was:

• The requirement to use a macro affects only RNG implementions, which are far fewer than RNG users
• The design should be forwards compatible: we could in the future add that blanket impl while replacing the macro with a deprecated version which does nothing (this is not strictly non-breaking since RNG implementations could behave unexpectedly, but it should be good enough for a new minor version).

[dhardy]

@josephlr or @TheIronBorn or @vks: it would be good to get a third opinion on this. Admittedly the impl_... macros are hacky; they are intended however only to be a stand-in until Rust has better support for overlapping blanket trait impls.

This change means that TryRngCore/TryCryptoRng no longer can have blanket impls for &mut R and Box<R>. But I think it should be tolerable since most users will be using RngCore/CryptoRng, which have blanket impl for DerefMut (it covers both &mut R and Box<R>).

Tolerable, yes, but it's something I'd like to be able to fix later without serious risk of breakage (assessing the impact of adding these impls later would be hard).

Of course, there is the question of whether Rust will ever get specialization or negative-trait-impls or some other mechanism allowing potentially-overlapping-trait-impls, but I would hope so (it's a significant limitation of the language currently).

[newpavlov]

Tolerable, yes, but it's something I'd like to be able to fix later without serious risk of breakage (assessing the impact of adding these impls later would be hard).

Assuming that in future we will get specialization or negative impls, I think we should be able to add a blanket DerefMut impl for TryRngCore/TryCryptoRng traits in a backwards compatible way (modulo MSRV bump). My other hope is that Rust eventually will be able to automatically use &mut T in place of impl Trait if Trait has only &mut self methods.

[dhardy]

impl<T: DerefMut> RngCore for T
where
    T::Target: RngCore,

My experience with this type of impl is that it sometimes works, and sometimes results in false conflicts.

A problematic case is here: the given impl covers the first three macro-impls (containers deref'ing to [W]) but not the next three (containers deref'ing to [(GridCellInfo, W)]); despite GridCellInfo being a local type, rust (6f4ae0f34 2024-10-08) complains of a potential conflict because "upstream crates may add a new impl of trait std::ops::Deref for type [(layout::grid_solver::GridCellInfo, _); _] in future versions" (technically possible if a std added a blanket impl for Deref over (A, B) tuples which completely ignores the first element).

A use of this that I haven't (yet) found a problem with can be found here (see autoimpl docs).

[dhardy]

Assuming that in future we will get specialization or negative impls, I think we should be able to add a blanket DerefMut impl for TryRngCore/TryCryptoRng traits in a backwards compatible way (modulo MSRV bump).

With negative trait impls, adding the blanket impl would be a breaking change since some downstream crate could make a local MyTryRng: TryRngCore and impl TryRngCore for &mut MyTryRng. With specialization it might not be.

My other hope is that Rust eventually will be able to automatically use &mut T in place of impl Trait if Trait has only &mut self methods.

That sounds kinda like a lang-wide automatic impl of DerefMut on traits, but with some hackery to avoid duplicate/conflicting impls. Or a new, hidden trait (also hacky). Either way, it would be adding hidden complexity to the language, so I wouldn't bet on this ever happening.

[dhardy]

I think, on the whole, I'm willing to accept this despite my arguments against, since in the (somewhat likely) case that Rust does not get specialization/negative-trait-bounds "reasonably soon" it is a little better than the status quo, and without causing significant issues.

I don't think the "hacky macros" being removed are a big deal though (due to being limited to RNG implementors).

[newpavlov]

My experience with this type of impl is that it sometimes works, and sometimes results in false conflicts.

In this case the main goal of the blanket impl is to make RNG implementations generic over &mut T and Box<T>, so I think it does this job well enough. Additional generality is just a nice bonus.