Closed dhardy closed 1 year ago
R: Rng + Sized
is exactly equivalent to R: RngCore + Sized
(prior to this rename).
This is not the case when using virtual dispatch: dyn RngCore
implies a vtable over methods next_u32
, next_u64
, fill_bytes
, try_fill_bytes
(intended usage). But dyn Rng
implies a vtable over gen_bool
, gen_ratio` (all other methods are generic and hence excluded).
So if we do not use this rename:
RngCore
Rng
dyn RngCore
If we do use this rename:
RngCore
R: Rng
and dyn Rng
, but...RngExt
(from prelude or explicitly)Does anyone have further thoughts on this, or shall we just reject it? (Primary rationale: it's a big breaking change for little gain.)
@vks @newpavlov @tarcieri and anyone else who wishes to comment (open discussion)
I think I'm happy with just #1273 which will allow us to migrate from CryptoRngCore
-> CryptoRng
everywhere
For review, as mentioned in #1273. Effects:
Rng, CryptoRng, BlockRngCore, CryptoBlockRng
. Eh, this still isn't quite right. But we havestruct BlockRng
, thus must keeptrait BlockRngCore
.Rng
and (at least sometimes)RngExt
.My opinion is now against this change:
Rng
. (In part this is because it's rarely necessary to callRngCore
methods directly; in part it's because anyR: Rng
also satisfiesR: RngCore
and vice-versa.) Now, user-code tends to use both (generics overR: Rng
and you needRngExt::gen
orRngExt::gen_range
or ..).Still, I'll leave it here in case anyone wishes to review the changes. (Note that a couple of simplifications could be made without the trait rename; e.g. replacing
R: RngCore
withR: Rng
in some places.)