Skip to content

Stabilize RandomSource and DefaultRandomSource #157168

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.

Sign up for GitHub

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

Open
joshtriplett wants to merge 1 commit into
base: main
Choose a base branch
from
+7 −4
Open

Stabilize RandomSource and DefaultRandomSource #157168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion library/core/src/random.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
use crate::ops::RangeFull;

/// A source of randomness.
#[unstable(feature = "random", issue = "130703")]
#[stable(feature = "random_source", since = "CURRENT_RUSTC_VERSION")]
pub trait RandomSource {

@hanna-kruppe hanna-kruppe May 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Regarding next_u32/next_u64, while I really want DefaultRandomSource.fill_bytes (in some form) stabilized ASAP, I have reservations about leaving it at "it's not clear we need them for performance and we can add them later". Personally I'd rather err on the side of adding these methods, unless we're quite sure we will never need them, or it's clear that we can't resolve the question in a reasonable time frame.

Adding the methods after stabilization has a cost (even besides opportunity cost). As @dhardy pointed out in the past, adding provided method later means existing implementers that want to offer reproducibility (as in stability of produced values) can't override the provided methods without breaking reproducibility for users who started using those methods. And for libraries that use RNGs to sample some distribution and want to promise reproducibility of that sampling, the same problem applies if they're first written against fill_bytes and later want to use next_uN.

Another (smaller) reason to err on the side of including these is to ease the ecosystem's transition from rand traits (which have always had next_u32/u64) to the std trait. If std doesn't have the methods at first and adds them later, that's two unnecessarily transitions (rand::Rng::next_uN -> fill_bytes + uN::from_*e_bytes -> RandomSource::next_uN). Stabilizing some subset of distributions would avoid this, but the distributions are far from ready for stabilization.

Finally, while the benchmarks in #157193 and on Zulip don't have a smoking gun that the methods are necessary for performance, it's also not clear that we won't want them. Even those benchmarks show a benefit for dyn RandomSource (the only argument is whether you consider that compatible with "cares about performance"), and @dhardy previously mentioned that rand has benchmarks justifying the methods in rand's context. At minimum we should look at those benchmarks as well and see if the fill_bytes semantics (which I think matches rand's) actually works for those benchmarks as well.

View changes since the review

@conradludgate conradludgate May 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IIRC, it's possible to use inlining and https://doc.rust-lang.org/std/intrinsics/fn.is_val_statically_known.html to perform these optimisations without needing the API surface.

@hanna-kruppe hanna-kruppe May 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Inlining doesn't work for dyn RandomSource. And is_val_statically_known only helps when the two implementations that have the same behavior, but in this case, some potentially desirable optimizations change behavior. (Also, the intrinsic doesn't seem to have a clear path to being exposed on stable.)

/// Fills `bytes` with random bytes.
///
/// Note that calling `fill_bytes` multiple times is not equivalent to calling `fill_bytes` once
/// with a larger buffer. A `RandomSource` is allowed to return different bytes for those two
/// cases. For instance, this allows a `RandomSource` to generate a word at a time and throw
/// part of it away if not needed.
#[stable(feature = "random_source", since = "CURRENT_RUSTC_VERSION")]
fn fill_bytes(&mut self, bytes: &mut [u8]);
}

Expand Down
8 changes: 5 additions & 3 deletions library/std/src/random.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
//! Random value generation.

#[unstable(feature = "random", issue = "130703")]
pub use core::random::*;
pub use core::random::Distribution;
#[stable(feature = "random_source", since = "CURRENT_RUSTC_VERSION")]
pub use core::random::RandomSource;

use crate::sys::random as sys;

Expand Down Expand Up @@ -55,10 +57,10 @@ use crate::sys::random as sys;
/// [`getrandom`]: https://www.man7.org/linux/man-pages/man2/getrandom.2.html
/// [`/dev/urandom`]: https://www.man7.org/linux/man-pages/man4/random.4.html
#[derive(Default, Debug, Clone, Copy)]
#[unstable(feature = "random", issue = "130703")]
#[stable(feature = "random_source", since = "CURRENT_RUSTC_VERSION")]
pub struct DefaultRandomSource;

#[unstable(feature = "random", issue = "130703")]
#[stable(feature = "random_source", since = "CURRENT_RUSTC_VERSION")]
impl RandomSource for DefaultRandomSource {
fn fill_bytes(&mut self, bytes: &mut [u8]) {
sys::fill_bytes(bytes)
Expand Down
Loading