Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace SeedableRng impl for SmallRng with inherent fns #1368

Merged
merged 6 commits into from
Jan 5, 2024

Conversation

dhardy
Copy link
Member

@dhardy dhardy commented Dec 30, 2023

Fixes #1285 by:

  • Remove SeedableRng impl for SmallRng. Use inherent fns instead.
  • Fix Seed type for StdRng (the only practical effect is to make a possible future breaking change to the API more obvious).

Loss of functionality:

  • SmallRng can no longer be used in generics requiring a SeedableRng bound. As such, it can also no longer be reseeded from generic code. Is this an issue?
  • There is no public SmallRng::from_seed function. This is deliberate since most useful uses require a portable RNG.

@dhardy
Copy link
Member Author

dhardy commented Dec 30, 2023

Some tests failed because SmallRng::from_entropy is missing. But do we want this anyway? Instead, I just modified the benchmarks to use <$R>::from_rng(rand::thread_rng()).unwrap()

Additional change: make fn SmallRng::from_thread_rng infallible, since it appears that the only possible failure is a panic when initializing thread_rng.

Aside: if we could bring SeedableRng into rand proper, we could add from_thread_rng as a standard addition..

@dhardy dhardy merged commit 1db3aa4 into rust-random:master Jan 5, 2024
12 checks passed
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.

CHANGE: Make <SmallRng as SeedableRng>::Seed the same type on 32 and 64 bit platforms
2 participants