Skip to content

Conversation

@dhardy
Copy link
Member

@dhardy dhardy commented Dec 29, 2025

  • Revise traits
  • Update documentation
  • Added a CHANGELOG.md entry

Summary & motivation

It was noted in #13 that the impl relationship between RngCore and TryRngCore is unsatisfactory. @newpavlov proposed a revision where RngCore: TryRngCore. This PR goes a bit further, merging the two traits.

  • Delete RngCore
  • Rename TryRngCoreTryRng. (Opinionated and not required. I also considered renaming it to RngCore.)
  • Add trait InfallibleRng: TryRng<Error = Infallible>. This is effectively a trait alias with no methods of its own.
  • Merge CryptoRng and TryCryptoRng into CryptoRng: TryRng

Results (motivation):

  • One/two less traits (depending on whether you count InfallibleRng as a "trait" or "trait alias")
  • Solves the issues surrounding impls on RNGs; in particular &mut R: TryRng for R: TryRng
  • UnwrapMut is no longer needed

(And yes, I can see the hypocrisy of suggesting these breaking changes after complaining about others, but preparing for rand_core v1.0 means deciding whether to deal with existing issues or stabilise what we already have.)

Optimality

In theory there should be little impact on the generated code, but it's clear the result isn't identical. I ran all rand benchmarks against master; those with at least 5% time change:

random_u32/chacha20: -8%
random_u32/std: +5%
random_u32/thread: +7%
random_u64/thread: +7%
init_gen/pcg64: +5%
init_gen/std: +11%
init_from_u64/chacha12: -5%
init_from_u64/chacha20: -5%
init_from_u64/std: -7%
init_from_seed/pcg64mcg: -9%
seq_slice_choose_1_of_100: +101%
seq_slice_sample_weighted_1_of_1000: +8%
seq_iter_sample_10_of_100: +24%
seq_iter_sample_fill_10_of_100: +33%
choose_unhinted_from_1_ChaCha20: -5%
choose_windowed_from_1_ChaCha20: -5%
choose_size-hinted_from_2_ChaCha20: +7%
choose_size-hinted_from_3_ChaCha20: +7%
choose_size-hinted_from_10_ChaCha20: +6%
choose_size-hinted_from_100_ChaCha20: +7%
choose_size-hinted_from_1000_ChaCha20: +7%
choose_windowed_from_1000_ChaCha20: +8%
choose_windowed_from_1_Pcg32: +9%
choose_windowed_from_100_Pcg32: +8%
choose_windowed_from_1000_Pcg32: -8%
choose_size-hinted_from_2_Pcg64: -9%
choose_size-hinted_from_3_Pcg64: -9%
choose_size-hinted_from_10_Pcg64: -9%
choose_size-hinted_from_100_Pcg64: -9%
choose_size-hinted_from_1000_Pcg64: -9%
choose_windowed_from_1000_Pcg64: +7%
shuffle_2_ChaCha12: -6%
partial_shuffle_10000_ChaCha12: +5%
shuffle_2_Pcg32: +83%
sample_i8x1/Pcg64/single: -9%
sample_i16x1/SmallRng/single: +11%
sample_i16x1/Pcg32/single: +6%
sample_i16x1/Pcg64/single: +7%
sample_i32x1/Pcg32/single: -12%
sample_i32x1/Pcg64/single: -12%
sample_i64x1/ChaCha8Rng/distr: -7%
sample_i64x1/Pcg32/distr: -8%
uniform_single/f64/Pcg32: +5%
uniform_distribution/f32/SmallRng: +10%
uniform_distribution/f32/Pcg32: -7%
uniform_distribution/f64/Pcg32: +19%
weighted_index_modification: -15%
weighted_sample_indices_10_of_1k: -6%
weighted_sample_indices_100_of_1k: -10%
weighted_sample_indices_400_of_1M: -6%
weighted_sample_indices_600_of_1M: -6%
weighted_sample_indices_1000_of_1M: -10%

The majority of results have no significant change and the above have roughly as many wins as losses, though there are a few more significant losses. TODO: look into this more.

Alternatives

  1. No change. We commit to keeping UnwrapMut and cannot have &mut R: TryRngCore for R: TryRngCore.
  2. We revert rand_core: add blanket impl of TryRngCore for RngCore rand#1499. Infallible RNGs need to impl both RngCore and TryRngCore (possible via macros).
  3. @newpavlov's suggestion. Infallible RNGs need to impl TryRngCore<Error = Infallible> (same as this PR). dyn RngCore vtables have extra methods.
  4. A variant on @newpavlov's suggestion / this PR where RngCore (or InfallibleRng) derives TryRngCore<Error = Infallible> + Sized. This implies a need to use dyn TryRngCore<Error = Infallible> instead of dyn InfallibleRng, but also that the vtable will not feature both try_next_u32 and next_u32.
  5. With the (long time unstable) trait-alias feature we could actually make InfallibleRng a trait alias. (I don't know whether switching to this this once stable would be a breaking change.)

Details

See also:

Concerns

Would it be preferable to stabilise this code in rand_core v1.0 over what we have now? I think so but would very much like feedback on this. CC @tarcieri @baloo

How is usability of the result?

  • Implementing an InfallibleRng requires a tiny bit more boilerplate.
  • Testing / using InfallibleRng without rand::Rng is a slight pain without the infallible methods next_u32() etc.; one must either .unwrap() (good for tests) or use match to unpack the Result. In PRNG crates this is a small (IMO acceptable) burden.
  • Usability of rand::Rng is basically unaffected
  • Is this a big deal for rand_core users, e.g. RustCrypto?

How much work is involved in dealing with the changes?

  • It's a breaking change to core traits, temporarily requiring a compatibility-bridge like rand_core_compat for some applications.
  • Updating RngCore implementations to use TryRng with type Error = Infallible and different method prototypes is a minor PITA. This affects us more than users; updating the rust-random/rngs repo will be a pain.
  • Code using rand::Rng is basically unaffected, except that usages of RngCore must be replaced with Rng or InfallibleRng
  • Code using rand_core::RngCore infallible methods (next_u32 etc.) without rand::Rng has to find some alternative

Copy link
Member

@newpavlov newpavlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I don't think bigger vtables is an important factor, but I am fine with not having any additional methods on the infallible trait. It also would make it easier to migrate to the trait alias in future.

Also Rng would be a better and more consistent name than InfallibleRng in my opinion. It would follow the precedent introduced by std with its TryFrom/From and TryInto/Into traits. If you don't want to rename rand::Rng to RngExt, then we probably could have both rand::Rng and rand_core::Rng, the former being an extension trait for the latter. It could be somewhat confusing, but probably would work fine in practice.

///
/// This trait encapsulates the low-level functionality common to all
/// generators, and is the "back end", to be implemented by generators.
/// End users should normally use the [`rand::Rng`] trait
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's worth to keep this reference to rand::Rng in the trait docs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't even make a "low effort" on the docs yet (until I have a better idea of whether we're doing this); some stuff will need un-deleting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants