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

make arbitrary generate full range of integers #240

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 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
136 changes: 81 additions & 55 deletions src/arbitrary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH};
use rand::seq::SliceRandom;
use rand::{self, Rng, RngCore};

/// `Gen` wraps a `rand::RngCore` with parameters to control the distribution of
/// `Gen` wraps a `rand::RngCore` with parameters to control the range of
/// random values.
///
/// A value with type satisfying the `Gen` trait can be constructed with the
/// `gen` function in this crate.
pub trait Gen: RngCore {
/// Controls the range of values of certain types created with this Gen.
/// For an explaination of which types behave how, see Arbitrary
maxbla marked this conversation as resolved.
Show resolved Hide resolved
fn size(&self) -> usize;
}

Expand All @@ -41,9 +43,9 @@ impl<R: RngCore> StdGen<R> {
/// generator.
///
/// The `size` parameter controls the size of random values generated. For
/// example, it specifies the maximum length of a randomly generated vector
/// and also will specify the maximum magnitude of a randomly generated
/// number.
/// example, it specifies the maximum length of a randomly generated
/// vector, but not the maximum magnitude of a randomly generated number.
/// For further explaination, see Arbitrary.
pub fn new(rng: R, size: usize) -> StdGen<R> {
StdGen { rng: rng, size: size }
}
Expand Down Expand Up @@ -82,9 +84,9 @@ impl StdThreadGen {
/// Returns a new thread-local RNG.
///
/// The `size` parameter controls the size of random values generated. For
/// example, it specifies the maximum length of a randomly generated vector
/// and also will specify the maximum magnitude of a randomly generated
/// number.
/// example, it specifies the maximum length of a randomly generated
/// vector, but not the maximum magnitude of a randomly generated number.
/// For further explaination, see Arbitrary.
pub fn new(size: usize) -> StdThreadGen {
StdThreadGen(StdGen { rng: rand::thread_rng(), size: size })
}
Expand Down Expand Up @@ -128,7 +130,12 @@ pub fn single_shrinker<A: 'static>(value: A) -> Box<dyn Iterator<Item = A>> {
/// shrunk.
///
/// Aside from shrinking, `Arbitrary` is different from the `std::Rand` trait
/// in that it uses a `Gen` to control the distribution of random values.
/// in that it respects `Gen::size()` for certain types. For example,
/// `Vec::arbitrary()` respects `Gen::size()` to decide the maximum `len()`
/// of the vector. This behavior is necessary due to practical speed and size
/// limitations. Conversely, `i32::arbitrary()` ignores `size()`, as all `i32`
/// values require `O(1)` memory and operations between `i32`s require `O(1)`
/// time (with the exception of exponentiation).
///
/// As of now, all types that implement `Arbitrary` must also implement
/// `Clone`. (I'm not sure if this is a permanent restriction.)
Expand Down Expand Up @@ -745,15 +752,23 @@ macro_rules! unsigned_shrinker {
};
}

macro_rules! unsigned_problem_values {
($t:ty) => {
[<$t>::min_value(), 1, <$t>::max_value()]
};
}

macro_rules! unsigned_arbitrary {
($($ty:tt),*) => {
$(
impl Arbitrary for $ty {
fn arbitrary<G: Gen>(g: &mut G) -> $ty {
#![allow(trivial_numeric_casts)]
let s = g.size() as $ty;
use std::cmp::{min, max};
g.gen_range(0, max(1, min(s, $ty::max_value())))
match g.gen_range(0,10) {
0 => {
*unsigned_problem_values!($ty).choose(g).unwrap()
},
_ => g.gen()
}
}
fn shrink(&self) -> Box<dyn Iterator<Item=$ty>> {
unsigned_shrinker!($ty);
Expand All @@ -765,11 +780,7 @@ macro_rules! unsigned_arbitrary {
}

unsigned_arbitrary! {
usize, u8, u16, u32, u64
}

unsigned_arbitrary! {
u128
usize, u8, u16, u32, u64, u128
}

macro_rules! signed_shrinker {
Expand Down Expand Up @@ -811,19 +822,23 @@ macro_rules! signed_shrinker {
};
}

macro_rules! signed_problem_values {
($t:ty) => {
[<$t>::min_value(), 0, <$t>::max_value()]
};
}

macro_rules! signed_arbitrary {
($($ty:tt),*) => {
$(
impl Arbitrary for $ty {
fn arbitrary<G: Gen>(g: &mut G) -> $ty {
use std::cmp::{min,max};
let upper = min(g.size(), $ty::max_value() as usize);
let lower = if upper > $ty::max_value() as usize {
$ty::min_value()
} else {
-(upper as $ty)
};
g.gen_range(lower, max(1, upper as $ty))
match g.gen_range(0,10) {
0 => {
*signed_problem_values!($ty).choose(g).unwrap()
},
_ => g.gen()
}
}
fn shrink(&self) -> Box<dyn Iterator<Item=$ty>> {
signed_shrinker!($ty);
Expand All @@ -835,11 +850,7 @@ macro_rules! signed_arbitrary {
}

signed_arbitrary! {
isize, i8, i16, i32, i64
}

signed_arbitrary! {
i128
isize, i8, i16, i32, i64, i128
}

impl Arbitrary for f32 {
Expand Down Expand Up @@ -933,8 +944,8 @@ impl Arbitrary for RangeFull {

impl Arbitrary for Duration {
fn arbitrary<G: Gen>(gen: &mut G) -> Self {
let seconds = u64::arbitrary(gen);
let nanoseconds = u32::arbitrary(gen) % 1_000_000;
let seconds = gen.gen_range(0, gen.size() as u64);
Copy link
Owner

Choose a reason for hiding this comment

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

Out of curiosity, why did you change this to use gen.size()? It was implemented this way before, but it seems like we should probably open up the range of durations generated too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this was an oversight and based on previous discussion, I think the criterion for whether a type respects gen.size() should be whether there are practical (memory, speed or some other resource) limits which make ignoring gen.size() impractical. Based on this criterion, Duration should not respect gen.size(). If you agree with this criterion, I could add it to the documentation for Arbitrary.

let nanoseconds = gen.gen_range(0, 1_000_000);
Duration::new(seconds, nanoseconds)
}

Expand Down Expand Up @@ -1002,43 +1013,58 @@ mod test {
use std::hash::Hash;
use std::num::Wrapping;
use std::path::PathBuf;
use super::StdGen;

#[test]
fn arby_unit() {
assert_eq!(arby::<()>(), ());
}

macro_rules! arby_int {
( $signed:expr, $($t:ty),+) => {$(
let arbys: Vec<$t> = (0..10000).map(|_| arby::<$t>()).collect();
let mut problems = if $signed {
signed_problem_values!($t).iter()
} else {
unsigned_problem_values!($t).iter()
};
assert!(problems.all(|p| arbys.iter().any(|arby| arby == p)),
"Arbitrary does not generate all problematic values");
let max = <$t>::max_value();
let mid = (max + <$t>::min_value()) / 2;
// split full range of $t into chunks
// Arbitrary must return some value in each chunk
let double_chunks: $t = 9;
let chunks = double_chunks * 2; // chunks must be even
let lim: Box<dyn Iterator<Item=$t>> = if $signed {
Box::new((0..=chunks)
.map(|idx| idx - chunks / 2)
.map(|x| mid + max / (chunks / 2) * x))
} else {
Box::new((0..=chunks).map(|idx| max / chunks * idx))
};
let mut lim = lim.peekable();
while let (Some(low), Some(&high)) = (lim.next(), lim.peek()) {
assert!(arbys.iter()
.find(|arby| low <= **arby && **arby <= high)
.is_some(),
"Arbitrary doesn't generate numbers in {}..={}", low, high)
}
)*};
}

#[test]
fn arby_int() {
rep(&mut || {
let n: isize = arby();
assert!(n >= -5 && n <= 5);
});
arby_int!(true, i8, i16, i32, i64, isize, i128);
}

#[test]
fn arby_uint() {
rep(&mut || {
let n: usize = arby();
assert!(n <= 5);
});
}

fn arby<A: super::Arbitrary>() -> A {
super::Arbitrary::arbitrary(&mut gen())
arby_int!(false, u8, u16, u32, u64, usize, u128);
}

fn gen() -> super::StdGen<rand::rngs::ThreadRng> {
super::StdGen::new(rand::thread_rng(), 5)
}

fn rep<F>(f: &mut F)
where
F: FnMut() -> (),
{
for _ in 0..100 {
f()
}
fn arby<A: Arbitrary>() -> A {
Arbitrary::arbitrary(&mut StdGen::new(rand::thread_rng(), 5))
}

// Shrink testing.
Expand Down
11 changes: 6 additions & 5 deletions src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,19 +132,20 @@ fn is_prime(n: usize) -> bool {
#[test]
#[should_panic]
fn sieve_not_prime() {
fn prop_all_prime(n: usize) -> bool {
sieve(n).into_iter().all(is_prime)
fn prop_all_prime(n: u8) -> bool {
sieve(n as usize).into_iter().all(is_prime)
}
quickcheck(prop_all_prime as fn(usize) -> bool);
quickcheck(prop_all_prime as fn(u8) -> bool);
}

#[test]
#[should_panic]
fn sieve_not_all_primes() {
fn prop_prime_iff_in_the_sieve(n: usize) -> bool {
fn prop_prime_iff_in_the_sieve(n: u8) -> bool {
let n = n as usize;
sieve(n) == (0..(n + 1)).filter(|&i| is_prime(i)).collect::<Vec<_>>()
}
quickcheck(prop_prime_iff_in_the_sieve as fn(usize) -> bool);
quickcheck(prop_prime_iff_in_the_sieve as fn(u8) -> bool);
}

#[test]
Expand Down