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

Generics #319

Merged
merged 5 commits into from
Mar 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions benches/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ fn misc_gen_bool_var(b: &mut Bencher) {

#[bench]
fn misc_shuffle_100(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(&mut thread_rng()).unwrap();
let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
let x : &mut [usize] = &mut [1; 100];
b.iter(|| {
rng.shuffle(x);
Expand All @@ -48,7 +48,7 @@ fn misc_shuffle_100(b: &mut Bencher) {

#[bench]
fn misc_sample_iter_10_of_100(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(&mut thread_rng()).unwrap();
let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
let x : &[usize] = &[1; 100];
b.iter(|| {
black_box(sample_iter(&mut rng, x, 10).unwrap_or_else(|e| e));
Expand All @@ -57,7 +57,7 @@ fn misc_sample_iter_10_of_100(b: &mut Bencher) {

#[bench]
fn misc_sample_slice_10_of_100(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(&mut thread_rng()).unwrap();
let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
let x : &[usize] = &[1; 100];
b.iter(|| {
black_box(sample_slice(&mut rng, x, 10));
Expand All @@ -66,7 +66,7 @@ fn misc_sample_slice_10_of_100(b: &mut Bencher) {

#[bench]
fn misc_sample_slice_ref_10_of_100(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(&mut thread_rng()).unwrap();
let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
let x : &[usize] = &[1; 100];
b.iter(|| {
black_box(sample_slice_ref(&mut rng, x, 10));
Expand All @@ -77,7 +77,7 @@ macro_rules! sample_indices {
($name:ident, $amount:expr, $length:expr) => {
#[bench]
fn $name(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(&mut thread_rng()).unwrap();
let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
b.iter(|| {
black_box(sample_indices(&mut rng, $length, $amount));
})
Expand Down
2 changes: 1 addition & 1 deletion rand-core/src/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ impl<R: BlockRngCore + SeedableRng> SeedableRng for BlockRng<R> {
}
}

fn from_rng<RNG: RngCore>(rng: &mut RNG) -> Result<Self, Error> {
fn from_rng<S: RngCore>(rng: S) -> Result<Self, Error> {
let results_empty = R::Results::default();
Ok(Self {
core: R::from_rng(rng)?,
Expand Down
2 changes: 1 addition & 1 deletion rand-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ pub trait SeedableRng: Sized {
/// [`NewRng`]: https://docs.rs/rand/0.5/rand/trait.NewRng.html
/// [`OsRng`]: https://docs.rs/rand/0.5/rand/os/struct.OsRng.html
/// [`XorShiftRng`]: https://docs.rs/rand/0.5/rand/prng/xorshift/struct.XorShiftRng.html
fn from_rng<R: RngCore>(rng: &mut R) -> Result<Self, Error> {
fn from_rng<R: RngCore>(mut rng: R) -> Result<Self, Error> {
let mut seed = Self::Seed::default();
rng.try_fill_bytes(seed.as_mut())?;
Ok(Self::from_seed(seed))
Expand Down
2 changes: 1 addition & 1 deletion src/distributions/other.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ mod tests {

#[test]
fn test_misc() {
let mut rng: &mut RngCore = &mut ::test::rng(820);
let rng: &mut RngCore = &mut ::test::rng(820);

rng.sample::<char, _>(Uniform);
rng.sample::<bool, _>(Uniform);
Expand Down
56 changes: 42 additions & 14 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,16 +332,40 @@ pub trait Rand : Sized {
/// An automatically-implemented extension trait on [`RngCore`] providing high-level
/// generic methods for sampling values and other convenience methods.
///
/// This is the primary trait to use when generating random values. Example:
/// This is the primary trait to use when generating random values.
///
/// # Generic usage
///
/// The basic pattern is `fn foo<R: Rng + ?Sized>(rng: &mut R)`. Some
/// things are worth noting here:
///
/// - Since `Rng: RngCore` and every `RngCore` implements `Rng`, it makes no
/// difference whether we use `R: Rng` or `R: RngCore`.
/// - The `+ ?Sized` un-bounding allows functions to be called directly on
/// type-erased references; i.e. `foo(r)` where `r: &mut RngCore`. Without
/// this it would be necessary to write `foo(&mut r)`.
///
/// An alternative pattern is possible: `fn foo<R: Rng>(rng: R)`. This has some
/// trade-offs. It allows the argument to be consumed directly without a `&mut`
/// (which is how `from_rng(thread_rng())` works); also it still works directly
/// on references (including type-erased references). Unfortunately within the
/// function `foo` it is not known whether `rng` is a reference type or not,
/// hence many uses of `rng` require an extra reference, either explicitly
/// (`distr.sample(&mut rng)`) or implicitly (`rng.gen()`); one may hope the
/// optimiser can remove redundant references later.
///
/// Example:
///
/// ```rust
/// use rand::Rng;
///
/// fn use_rng<R: Rng + ?Sized>(rng: &mut R) -> f32 {
/// fn foo<R: Rng + ?Sized>(rng: &mut R) -> f32 {
/// rng.gen()
/// }
/// ```
///
/// # Iteration
///
/// Iteration over an `Rng` can be achieved using `iter::repeat` as follows:
///
/// ```rust
Expand Down Expand Up @@ -635,7 +659,7 @@ pub trait Rng: RngCore {
}
}

impl<R: RngCore> Rng for R {}
impl<R: RngCore + ?Sized> Rng for R {}

/// Trait for casting types to byte slices
///
Expand Down Expand Up @@ -800,7 +824,7 @@ pub trait NewRng: SeedableRng {
///
/// fn foo() -> Result<(), Error> {
/// // This uses StdRng, but is valid for any R: SeedableRng
/// let mut rng = StdRng::from_rng(&mut EntropyRng::new())?;
/// let mut rng = StdRng::from_rng(EntropyRng::new())?;
///
Copy link
Contributor

@burdges burdges Mar 22, 2018

Choose a reason for hiding this comment

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

I think the examples and documentation should do &mut much more frequently than not, as inexperienced rust developers may find the error message from a missing &mut unintuitive. I don't see any problem with doing this one without the &mut of course.

Copy link
Member Author

Choose a reason for hiding this comment

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

Using a different convention for from_rng than the other RNG consumers bugs me a bit, but seems the best option. It sounds like the RFC I opened about reborrowing is being postponed, so we're not going to get a nice uniform solution any time soon; I guess then this is the best we can do.

Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't checked the error message myself, but really that sounds like the only problematic part. If it's really confusion then we could suggest a rewording as a rustc bug, which get addressed quickly.

/// println!("random number: {}", rng.gen_range(1, 10));
/// Ok(())
Expand All @@ -812,7 +836,7 @@ pub trait NewRng: SeedableRng {
#[cfg(feature="std")]
impl<R: SeedableRng> NewRng for R {
fn new() -> R {
R::from_rng(&mut EntropyRng::new()).unwrap_or_else(|err|
R::from_rng(EntropyRng::new()).unwrap_or_else(|err|
panic!("NewRng::new() failed: {}", err))
}
}
Expand Down Expand Up @@ -860,8 +884,8 @@ impl SeedableRng for StdRng {
StdRng(Hc128Rng::from_seed(seed))
}

fn from_rng<R: RngCore>(rng: &mut R) -> Result<Self, Error> {
Hc128Rng::from_rng(rng).map(|rng| StdRng(rng))
fn from_rng<R: RngCore>(rng: R) -> Result<Self, Error> {
Hc128Rng::from_rng(rng).map(|result| StdRng(result))
}
}

Expand Down Expand Up @@ -896,14 +920,18 @@ impl CryptoRng for StdRng {}
/// efficient:
///
/// ```
/// use std::iter;
/// use rand::{SeedableRng, SmallRng, thread_rng};
///
/// // Create a big, expensive to initialize and slower, but unpredictable RNG.
/// // This is cached and done only once per thread.
/// let mut thread_rng = thread_rng();
/// // Create small, cheap to initialize and fast RNG with a random seed.
/// // This is very unlikely to fail.
/// let mut small_rng = SmallRng::from_rng(&mut thread_rng).unwrap();
/// // Create small, cheap to initialize and fast RNGs with random seeds.
/// // One can generally assume this won't fail.
/// let rngs: Vec<SmallRng> = iter::repeat(())
/// .map(|()| SmallRng::from_rng(&mut thread_rng).unwrap())
/// .take(10)
/// .collect();
/// ```
///
/// [Xorshift]: struct.XorShiftRng.html
Expand Down Expand Up @@ -937,8 +965,8 @@ impl SeedableRng for SmallRng {
SmallRng(XorShiftRng::from_seed(seed))
}

fn from_rng<R: RngCore>(rng: &mut R) -> Result<Self, Error> {
XorShiftRng::from_rng(rng).map(|rng| SmallRng(rng))
fn from_rng<R: RngCore>(rng: R) -> Result<Self, Error> {
XorShiftRng::from_rng(rng).map(|result| SmallRng(result))
}
}

Expand All @@ -955,7 +983,7 @@ impl SeedableRng for SmallRng {
#[deprecated(since="0.5.0", note="removed in favor of SmallRng")]
#[cfg(feature="std")]
pub fn weak_rng() -> XorShiftRng {
XorShiftRng::from_rng(&mut thread_rng()).unwrap_or_else(|err|
XorShiftRng::from_rng(thread_rng()).unwrap_or_else(|err|
panic!("weak_rng failed: {:?}", err))
}

Expand Down Expand Up @@ -1182,7 +1210,7 @@ mod test {
let mut rng1 = StdRng::from_seed(seed);
assert_eq!(rng1.next_u64(), 15759097995037006553);

let mut rng2 = StdRng::from_rng(&mut rng1).unwrap();
let mut rng2 = StdRng::from_rng(rng1).unwrap();
assert_eq!(rng2.next_u64(), 6766915756997287454);
}
}
7 changes: 4 additions & 3 deletions src/prng/chacha.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ impl SeedableRng for ChaChaRng {
ChaChaRng(BlockRng::<ChaChaCore>::from_seed(seed))
}

fn from_rng<R: RngCore>(rng: &mut R) -> Result<Self, Error> {
BlockRng::<ChaChaCore>::from_rng(rng).map(|rng| ChaChaRng(rng))
fn from_rng<R: RngCore>(rng: R) -> Result<Self, Error> {
BlockRng::<ChaChaCore>::from_rng(rng).map(|result| ChaChaRng(result))
}
}

Expand Down Expand Up @@ -273,6 +273,7 @@ impl ChaChaCore {

impl SeedableRng for ChaChaCore {
type Seed = [u8; SEED_WORDS*4];

fn from_seed(seed: Self::Seed) -> Self {
let mut seed_le = [0u32; SEED_WORDS];
le::read_u32_into(&seed, &mut seed_le);
Expand Down Expand Up @@ -302,7 +303,7 @@ mod test {
let mut rng1 = ChaChaRng::from_seed(seed);
assert_eq!(rng1.next_u32(), 137206642);

let mut rng2 = ChaChaRng::from_rng(&mut rng1).unwrap();
let mut rng2 = ChaChaRng::from_rng(rng1).unwrap();
assert_eq!(rng2.next_u32(), 1325750369);
}

Expand Down
4 changes: 2 additions & 2 deletions src/prng/hc128.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ impl SeedableRng for Hc128Rng {
Hc128Rng(BlockRng::<Hc128Core>::from_seed(seed))
}

fn from_rng<R: RngCore>(rng: &mut R) -> Result<Self, Error> {
BlockRng::<Hc128Core>::from_rng(rng).map(|rng| Hc128Rng(rng))
fn from_rng<R: RngCore>(rng: R) -> Result<Self, Error> {
BlockRng::<Hc128Core>::from_rng(rng).map(|result| Hc128Rng(result))
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/prng/isaac.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ impl SeedableRng for IsaacRng {
init(seed_extended, 2)
}

fn from_rng<R: RngCore>(rng: &mut R) -> Result<Self, Error> {
fn from_rng<R: RngCore>(mut rng: R) -> Result<Self, Error> {
// Custom `from_rng` implementation that fills a seed with the same size
// as the entire state.
let mut seed = [w(0u32); RAND_SIZE];
Expand Down Expand Up @@ -380,7 +380,7 @@ mod test {
let mut rng1 = IsaacRng::from_seed(seed);
assert_eq!(rng1.next_u32(), 2869442790);

let mut rng2 = IsaacRng::from_rng(&mut rng1).unwrap();
let mut rng2 = IsaacRng::from_rng(rng1).unwrap();
assert_eq!(rng2.next_u32(), 3094074039);
}

Expand Down
4 changes: 2 additions & 2 deletions src/prng/isaac64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ impl SeedableRng for Isaac64Rng {
init(seed_extended, 2)
}

fn from_rng<R: RngCore>(rng: &mut R) -> Result<Self, Error> {
fn from_rng<R: RngCore>(mut rng: R) -> Result<Self, Error> {
// Custom `from_rng` implementation that fills a seed with the same size
// as the entire state.
let mut seed = [w(0u64); RAND_SIZE];
Expand Down Expand Up @@ -356,7 +356,7 @@ mod test {
let mut rng1 = Isaac64Rng::from_seed(seed);
assert_eq!(rng1.next_u64(), 14964555543728284049);

let mut rng2 = Isaac64Rng::from_rng(&mut rng1).unwrap();
let mut rng2 = Isaac64Rng::from_rng(rng1).unwrap();
assert_eq!(rng2.next_u64(), 919595328260451758);
}

Expand Down
4 changes: 2 additions & 2 deletions src/prng/xorshift.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl SeedableRng for XorShiftRng {
}
}

fn from_rng<R: RngCore>(rng: &mut R) -> Result<Self, Error> {
fn from_rng<R: RngCore>(mut rng: R) -> Result<Self, Error> {
let mut seed_u32 = [0u32; 4];
loop {
unsafe {
Expand Down Expand Up @@ -138,7 +138,7 @@ mod tests {
let mut rng1 = XorShiftRng::from_seed(seed);
assert_eq!(rng1.next_u64(), 4325440999699518727);

let _rng2 = XorShiftRng::from_rng(&mut rng1).unwrap();
let _rng2 = XorShiftRng::from_rng(rng1).unwrap();
// Note: we cannot test the state of _rng2 because from_rng does not
// fix Endianness. This is allowed in the trait specification.
}
Expand Down
12 changes: 6 additions & 6 deletions src/seq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use super::Rng;
/// ```
pub fn sample_iter<T, I, R>(rng: &mut R, iterable: I, amount: usize) -> Result<Vec<T>, Vec<T>>
where I: IntoIterator<Item=T>,
R: Rng,
R: Rng + ?Sized,
{
let mut iter = iterable.into_iter();
let mut reservoir = Vec::with_capacity(amount);
Expand Down Expand Up @@ -85,7 +85,7 @@ pub fn sample_iter<T, I, R>(rng: &mut R, iterable: I, amount: usize) -> Result<V
/// println!("{:?}", seq::sample_slice(&mut rng, &values, 3));
/// ```
pub fn sample_slice<R, T>(rng: &mut R, slice: &[T], amount: usize) -> Vec<T>
where R: Rng,
where R: Rng + ?Sized,
T: Clone
{
let indices = sample_indices(rng, slice.len(), amount);
Expand Down Expand Up @@ -113,7 +113,7 @@ pub fn sample_slice<R, T>(rng: &mut R, slice: &[T], amount: usize) -> Vec<T>
/// println!("{:?}", seq::sample_slice_ref(&mut rng, &values, 3));
/// ```
pub fn sample_slice_ref<'a, R, T>(rng: &mut R, slice: &'a [T], amount: usize) -> Vec<&'a T>
where R: Rng
where R: Rng + ?Sized
{
let indices = sample_indices(rng, slice.len(), amount);

Expand All @@ -133,7 +133,7 @@ pub fn sample_slice_ref<'a, R, T>(rng: &mut R, slice: &'a [T], amount: usize) ->
///
/// Panics if `amount > length`
pub fn sample_indices<R>(rng: &mut R, length: usize, amount: usize) -> Vec<usize>
where R: Rng,
where R: Rng + ?Sized,
{
if amount > length {
panic!("`amount` must be less than or equal to `slice.len()`");
Expand Down Expand Up @@ -166,7 +166,7 @@ pub fn sample_indices<R>(rng: &mut R, length: usize, amount: usize) -> Vec<usize
/// This is better than using a HashMap "cache" when `amount >= length / 2` since it does not
/// require allocating an extra cache and is much faster.
fn sample_indices_inplace<R>(rng: &mut R, length: usize, amount: usize) -> Vec<usize>
where R: Rng,
where R: Rng + ?Sized,
{
debug_assert!(amount <= length);
let mut indices: Vec<usize> = Vec::with_capacity(length);
Expand All @@ -193,7 +193,7 @@ fn sample_indices_cache<R>(
length: usize,
amount: usize,
) -> Vec<usize>
where R: Rng,
where R: Rng + ?Sized,
{
debug_assert!(amount <= length);
#[cfg(feature="std")] let mut cache = HashMap::with_capacity(amount);
Expand Down