rand_core: add compatibility shim around 0.5

[dhardy]

Time to let CI do its thing...

[dhardy]

That didn't work: implementations are using the wrong Error type. Time for try 2...

[dhardy]

So, I believe it is impossible to build a compatibility shim:

rand_core::Error must be the new error type (and not merely a wrapper) since it appears in the prototype of RngCore::try_fill_bytes both here and in libs implementing RngCore.

However, rand_core::Error must also support the old API so as to allow it to be constructed as it used to be.

Hence: I think we must yank rand_core 0.5 (and all the newly published crates depending on it), then revise rand_core::Error such that it has a compatible API.

The only alternative would be a clean break (i.e. users may not use old and new Rand versions simultaneously).

[dhardy]

It is in fact possible to build a (one-way) compatibility shim: the goal is of course that types using one version of the traits are compatible with types using the other version, so we can e.g. implement all 0.4 traits for any type supporting the 0.5 traits. (Note: the RngCore traits are incompatible because they reference different Error types, hence we can't re-export this trait as above.)

I think this is the better direction to provide compatibility, since it allows any library providing an RNG to be updated immediately, and consumers of RNGs to update once all their required RNGs have updated. (The other way would be problematic: libs providing RNGs would never be able to update without potential compatibility issues.)

[dhardy]

No, a blanket impl like this seems to be impossible:

impl<T: rand_core5::RngCore> RngCore for T { ... }

E.g. for T: rand_core5::RngCore, then &mut T: rand_core5::RngCore via the blanket impl for &mut T where T: rand_core5::RngCore, and thus &mut T: RngCore via the new impl. At the same time, T: RngCore via the new impl, and then &mut T: RngCore via the blanket impl for &mut T where T: RngCore. These implementations overlap, which is not allowed.

@burdges any ideas? I can't see a solution without removing one of the three blanket impls (crippling rand_core5 or breaking any direct impls for RngCore in 0.4 or disabling the compatibility shim).

[dhardy]

I pushed my implementation (complete other than the above issue).

[dhardy]

Cross-reference: https://internals.rust-lang.org/t/semver-trick-and-traits-overlapping-trait-implementations/10377

[burdges]

I'm not sure I understand the problem but yes impl<T: Foo> Bar for T might easily conflict with similar blanket impls. You might need a wrapper type like pub struct Rand5<R8>(pub R8); and lots of pub type Foo = Rand5<rqand8::Foo>; for various Foo along with macros to forward things.

[dhardy]

I don't think we can do that in a compatibility shim. This is a patch release over 0.4.0 and thus cannot have any API changes.

[dhardy]

My understanding is that a true compatibility shim is not an option, in which case we have only very few options:

1. Go ahead and release rand_core 0.5 without a compatibility shim. RNGs and errors from previous Rand versions will forever be incompatible with future versions (and vice-versa). This may make it difficult for some users to upgrade, but shouldn't be impossible.
2. Revisit the Error type and ensure all changes are API-compatible (such that we can just make a patch release over 0.4.0).

Thoughts? (I'm thinking go with 1.)

[newpavlov]

I also prefer option 1.

[dhardy]

I'll make a PR soon with the new crate versions. I already unyanked crates which did bump the minor version.

[mati865]

@dhardy is there any chance to coordinate it with Rust?
Last time it happened it made all PRs fail.

[dhardy]

@mati865 coordinate in what way? Those PRs failed because I released breaking changes in patch releases.

[mati865]

@dhardy sorry for misunderstanding, I was under impression the breakage is going to happen again.

[burdges]

If anyone needs it then I wrote out a one-way wrapper type adapter/shims as RngCore5As4 in #865 (comment) and one could write a RngCore4As5 for going the other direction too.

I agree impl<T: rand_core5::RngCore> rand_core4::RngCore for T { ... } sounds impossible due to &mut T: RngCore. Any normal type requires BorrowMut "wrapper traits" everywhere when being passed into functions, but RngCore "spends" its generic impls making this more ergonomic, so "wrapper types" appear here.

I suppose you might add specific impls for OsRng and ThreadRnd and handle the block ones like:

impl<R: rand_core5::BlockRngCore> rand_core4::BlockRngCore for r {
    type Item = <R as rand_core5::BlockRngCore>::Item;
    type Results = <R as rand_core5::BlockRngCore>::Item;
    fn generate(&mut self, results: &mut Self::Results) { 
        rand_core5::BlockRngCore(self,results); 
    }
}

Yet, anything that defines their own RngCore breaks so really adding manual RngCore5As4 and RngCore4As5 sounds better.

[dhardy]

Agreed. Lacking a good place to put these wrapper types, I think the best approach is if anyone needing them simply copies the code / adds their own?

There are From impls to convert the Error type in this PR.

[burdges]

I've a branch that does this correctly now. I doubt cargo feature work on stable, so I might factor RngCore5As4 out into a separate crate, but anyways the diff is:

w3f/schnorrkel@8150ef6

I never managed to invoke with_cause correctly, but whatever.