Skip to content

Commit

Permalink
Make Uniform constructors return a result (#1229)
Browse files Browse the repository at this point in the history
* Forbid unsafe code in crates without unsafe code

This helps tools like `cargo geiger`.

* Make `Uniform` constructors return a result

- This is a breaking change.
- The new error type had to be made public, otherwise `Uniform` could
  not be extended for user-defined types by implementing
  `UniformSampler`.
- `rand_distr` was updated accordingly.
- Also forbid unsafe code for crates where none is used.

Fixes #1195, #1211.

* Address review feedback
* Make `sample_single` return a `Result`
* Fix benchmarks
* Small fixes
* Update src/distributions/uniform.rs
  • Loading branch information
vks authored Feb 6, 2023
1 parent ae4b48e commit 7d73990
Show file tree
Hide file tree
Showing 23 changed files with 261 additions and 228 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ A [separate changelog is kept for rand_core](rand_core/CHANGELOG.md).

You may also find the [Upgrade Guide](https://rust-random.github.io/book/update.html) useful.

## [Unreleased API changing release]
## [0.9.0] - unreleased
### Distributions
- `{Uniform, UniformSampler}::{new, new_inclusive}` return a `Result` (instead of potentially panicking) (#1229)
- `Uniform` implements `TryFrom` instead of `From` for ranges (#1229)

### Other
- Simpler and faster implementation of Floyd's F2 (#1277). This
Expand Down
38 changes: 19 additions & 19 deletions benches/distributions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,36 +131,36 @@ macro_rules! distr {
}

// uniform
distr_int!(distr_uniform_i8, i8, Uniform::new(20i8, 100));
distr_int!(distr_uniform_i16, i16, Uniform::new(-500i16, 2000));
distr_int!(distr_uniform_i32, i32, Uniform::new(-200_000_000i32, 800_000_000));
distr_int!(distr_uniform_i64, i64, Uniform::new(3i64, 123_456_789_123));
distr_int!(distr_uniform_i128, i128, Uniform::new(-123_456_789_123i128, 123_456_789_123_456_789));
distr_int!(distr_uniform_usize16, usize, Uniform::new(0usize, 0xb9d7));
distr_int!(distr_uniform_usize32, usize, Uniform::new(0usize, 0x548c0f43));
distr_int!(distr_uniform_i8, i8, Uniform::new(20i8, 100).unwrap());
distr_int!(distr_uniform_i16, i16, Uniform::new(-500i16, 2000).unwrap());
distr_int!(distr_uniform_i32, i32, Uniform::new(-200_000_000i32, 800_000_000).unwrap());
distr_int!(distr_uniform_i64, i64, Uniform::new(3i64, 123_456_789_123).unwrap());
distr_int!(distr_uniform_i128, i128, Uniform::new(-123_456_789_123i128, 123_456_789_123_456_789).unwrap());
distr_int!(distr_uniform_usize16, usize, Uniform::new(0usize, 0xb9d7).unwrap());
distr_int!(distr_uniform_usize32, usize, Uniform::new(0usize, 0x548c0f43).unwrap());
#[cfg(target_pointer_width = "64")]
distr_int!(distr_uniform_usize64, usize, Uniform::new(0usize, 0x3a42714f2bf927a8));
distr_int!(distr_uniform_isize, isize, Uniform::new(-1060478432isize, 1858574057));
distr_int!(distr_uniform_usize64, usize, Uniform::new(0usize, 0x3a42714f2bf927a8).unwrap());
distr_int!(distr_uniform_isize, isize, Uniform::new(-1060478432isize, 1858574057).unwrap());

distr_float!(distr_uniform_f32, f32, Uniform::new(2.26f32, 2.319));
distr_float!(distr_uniform_f64, f64, Uniform::new(2.26f64, 2.319));
distr_float!(distr_uniform_f32, f32, Uniform::new(2.26f32, 2.319).unwrap());
distr_float!(distr_uniform_f64, f64, Uniform::new(2.26f64, 2.319).unwrap());

const LARGE_SEC: u64 = u64::max_value() / 1000;

distr_duration!(distr_uniform_duration_largest,
Uniform::new_inclusive(Duration::new(0, 0), Duration::new(u64::max_value(), 999_999_999))
Uniform::new_inclusive(Duration::new(0, 0), Duration::new(u64::max_value(), 999_999_999)).unwrap()
);
distr_duration!(distr_uniform_duration_large,
Uniform::new(Duration::new(0, 0), Duration::new(LARGE_SEC, 1_000_000_000 / 2))
Uniform::new(Duration::new(0, 0), Duration::new(LARGE_SEC, 1_000_000_000 / 2)).unwrap()
);
distr_duration!(distr_uniform_duration_one,
Uniform::new(Duration::new(0, 0), Duration::new(1, 0))
Uniform::new(Duration::new(0, 0), Duration::new(1, 0)).unwrap()
);
distr_duration!(distr_uniform_duration_variety,
Uniform::new(Duration::new(10000, 423423), Duration::new(200000, 6969954))
Uniform::new(Duration::new(10000, 423423), Duration::new(200000, 6969954)).unwrap()
);
distr_duration!(distr_uniform_duration_edge,
Uniform::new_inclusive(Duration::new(LARGE_SEC, 999_999_999), Duration::new(LARGE_SEC + 1, 1))
Uniform::new_inclusive(Duration::new(LARGE_SEC, 999_999_999), Duration::new(LARGE_SEC + 1, 1)).unwrap()
);

// standard
Expand Down Expand Up @@ -272,7 +272,7 @@ macro_rules! uniform_sample {
let high = black_box($high);
b.iter(|| {
for _ in 0..10 {
let dist = UniformInt::<$type>::new(low, high);
let dist = UniformInt::<$type>::new(low, high).unwrap();
for _ in 0..$count {
black_box(dist.sample(&mut rng));
}
Expand All @@ -291,7 +291,7 @@ macro_rules! uniform_inclusive {
let high = black_box($high);
b.iter(|| {
for _ in 0..10 {
let dist = UniformInt::<$type>::new_inclusive(low, high);
let dist = UniformInt::<$type>::new_inclusive(low, high).unwrap();
for _ in 0..$count {
black_box(dist.sample(&mut rng));
}
Expand All @@ -311,7 +311,7 @@ macro_rules! uniform_single {
let high = black_box($high);
b.iter(|| {
for _ in 0..(10 * $count) {
black_box(UniformInt::<$type>::sample_single(low, high, &mut rng));
black_box(UniformInt::<$type>::sample_single(low, high, &mut rng).unwrap());
}
});
}
Expand Down
2 changes: 1 addition & 1 deletion examples/monte-carlo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
use rand::distributions::{Distribution, Uniform};

fn main() {
let range = Uniform::new(-1.0f64, 1.0);
let range = Uniform::new(-1.0f64, 1.0).unwrap();
let mut rng = rand::thread_rng();

let total = 1_000_000;
Expand Down
2 changes: 1 addition & 1 deletion examples/monty-hall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ fn main() {
let num_simulations = 10000;

let mut rng = rand::thread_rng();
let random_door = Uniform::new(0u32, 3);
let random_door = Uniform::new(0u32, 3).unwrap();

let (mut switch_wins, mut switch_losses) = (0, 0);
let (mut keep_wins, mut keep_losses) = (0, 0);
Expand Down
2 changes: 1 addition & 1 deletion examples/rayon-monte-carlo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ static BATCH_SIZE: u64 = 10_000;
static BATCHES: u64 = 1000;

fn main() {
let range = Uniform::new(-1.0f64, 1.0);
let range = Uniform::new(-1.0f64, 1.0).unwrap();

let in_circle = (0..BATCHES)
.into_par_iter()
Expand Down
1 change: 1 addition & 0 deletions rand_chacha/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
html_favicon_url = "https://www.rust-lang.org/favicon.ico",
html_root_url = "https://rust-random.github.io/rand/"
)]
#![forbid(unsafe_code)]
#![deny(missing_docs)]
#![deny(missing_debug_implementations)]
#![doc(test(attr(allow(unused_variables), deny(warnings))))]
Expand Down
1 change: 1 addition & 0 deletions rand_distr/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [0.5.0] - unreleased
- Remove unused fields from `Gamma`, `NormalInverseGaussian` and `Zipf` distributions (#1184)
This breaks serialization compatibility with older versions.
- Upgrade Rand

## [0.4.3] - 2021-12-30
- Fix `no_std` build (#1208)
Expand Down
4 changes: 2 additions & 2 deletions rand_distr/src/binomial.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ impl Distribution<u64> for Binomial {
// return value
let mut y: i64;

let gen_u = Uniform::new(0., p4);
let gen_v = Uniform::new(0., 1.);
let gen_u = Uniform::new(0., p4).unwrap();
let gen_v = Uniform::new(0., 1.).unwrap();

loop {
// Step 1: Generate `u` for selecting the region. If region 1 is
Expand Down
2 changes: 1 addition & 1 deletion rand_distr/src/hypergeometric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ impl Distribution<u64> for Hypergeometric {
x
},
RejectionAcceptance { m, a, lambda_l, lambda_r, x_l, x_r, p1, p2, p3 } => {
let distr_region_select = Uniform::new(0.0, p3);
let distr_region_select = Uniform::new(0.0, p3).unwrap();
loop {
let (y, v) = loop {
let u = distr_region_select.sample(rng);
Expand Down
1 change: 1 addition & 0 deletions rand_distr/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
html_favicon_url = "https://www.rust-lang.org/favicon.ico",
html_root_url = "https://rust-random.github.io/rand/"
)]
#![forbid(unsafe_code)]
#![deny(missing_docs)]
#![deny(missing_debug_implementations)]
#![allow(
Expand Down
2 changes: 1 addition & 1 deletion rand_distr/src/unit_ball.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub struct UnitBall;
impl<F: Float + SampleUniform> Distribution<[F; 3]> for UnitBall {
#[inline]
fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> [F; 3] {
let uniform = Uniform::new(F::from(-1.).unwrap(), F::from(1.).unwrap());
let uniform = Uniform::new(F::from(-1.).unwrap(), F::from(1.).unwrap()).unwrap();
let mut x1;
let mut x2;
let mut x3;
Expand Down
2 changes: 1 addition & 1 deletion rand_distr/src/unit_circle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub struct UnitCircle;
impl<F: Float + SampleUniform> Distribution<[F; 2]> for UnitCircle {
#[inline]
fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> [F; 2] {
let uniform = Uniform::new(F::from(-1.).unwrap(), F::from(1.).unwrap());
let uniform = Uniform::new(F::from(-1.).unwrap(), F::from(1.).unwrap()).unwrap();
let mut x1;
let mut x2;
let mut sum;
Expand Down
2 changes: 1 addition & 1 deletion rand_distr/src/unit_disc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub struct UnitDisc;
impl<F: Float + SampleUniform> Distribution<[F; 2]> for UnitDisc {
#[inline]
fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> [F; 2] {
let uniform = Uniform::new(F::from(-1.).unwrap(), F::from(1.).unwrap());
let uniform = Uniform::new(F::from(-1.).unwrap(), F::from(1.).unwrap()).unwrap();
let mut x1;
let mut x2;
loop {
Expand Down
2 changes: 1 addition & 1 deletion rand_distr/src/unit_sphere.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub struct UnitSphere;
impl<F: Float + SampleUniform> Distribution<[F; 3]> for UnitSphere {
#[inline]
fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> [F; 3] {
let uniform = Uniform::new(F::from(-1.).unwrap(), F::from(1.).unwrap());
let uniform = Uniform::new(F::from(-1.).unwrap(), F::from(1.).unwrap()).unwrap();
loop {
let (x1, x2) = (uniform.sample(rng), uniform.sample(rng));
let sum = x1 * x1 + x2 * x2;
Expand Down
6 changes: 3 additions & 3 deletions rand_distr/src/weighted_alias.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,8 @@ impl<W: AliasableWeight> WeightedAliasIndex<W> {

// Prepare distributions for sampling. Creating them beforehand improves
// sampling performance.
let uniform_index = Uniform::new(0, n);
let uniform_within_weight_sum = Uniform::new(W::ZERO, weight_sum);
let uniform_index = Uniform::new(0, n).unwrap();
let uniform_within_weight_sum = Uniform::new(W::ZERO, weight_sum).unwrap();

Ok(Self {
aliases: aliases.aliases,
Expand Down Expand Up @@ -458,7 +458,7 @@ mod test {
let random_weight_distribution = Uniform::new_inclusive(
W::ZERO,
W::MAX / W::try_from_u32_lossy(NUM_WEIGHTS).unwrap(),
);
).unwrap();
for _ in 0..NUM_WEIGHTS {
weights.push(rng.sample(&random_weight_distribution));
}
Expand Down
1 change: 1 addition & 0 deletions rand_pcg/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
html_favicon_url = "https://www.rust-lang.org/favicon.ico",
html_root_url = "https://rust-random.github.io/rand/"
)]
#![forbid(unsafe_code)]
#![deny(missing_docs)]
#![deny(missing_debug_implementations)]
#![no_std]
Expand Down
7 changes: 4 additions & 3 deletions src/distributions/distribution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub trait Distribution<T> {
/// .collect();
///
/// // Dice-rolling:
/// let die_range = Uniform::new_inclusive(1, 6);
/// let die_range = Uniform::new_inclusive(1, 6).unwrap();
/// let mut roll_die = die_range.sample_iter(&mut rng);
/// while roll_die.next().unwrap() != 6 {
/// println!("Not a 6; rolling again!");
Expand Down Expand Up @@ -93,7 +93,7 @@ pub trait Distribution<T> {
///
/// let mut rng = thread_rng();
///
/// let die = Uniform::new_inclusive(1, 6);
/// let die = Uniform::new_inclusive(1, 6).unwrap();
/// let even_number = die.map(|num| num % 2 == 0);
/// while !even_number.sample(&mut rng) {
/// println!("Still odd; rolling again!");
Expand Down Expand Up @@ -227,7 +227,7 @@ mod tests {

#[test]
fn test_distributions_map() {
let dist = Uniform::new_inclusive(0, 5).map(|val| val + 15);
let dist = Uniform::new_inclusive(0, 5).unwrap().map(|val| val + 15);

let mut rng = crate::test::rng(212);
let val = dist.sample(&mut rng);
Expand All @@ -240,6 +240,7 @@ mod tests {
rng: &mut R,
) -> impl Iterator<Item = i32> + '_ {
Uniform::new_inclusive(1, 6)
.unwrap()
.sample_iter(rng)
.filter(|x| *x != 5)
.take(10)
Expand Down
4 changes: 2 additions & 2 deletions src/distributions/other.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ impl Distribution<char> for Standard {
// reserved for surrogates. This is the size of that gap.
const GAP_SIZE: u32 = 0xDFFF - 0xD800 + 1;

// Uniform::new(0, 0x11_0000 - GAP_SIZE) can also be used but it
// Uniform::new(0, 0x11_0000 - GAP_SIZE) can also be used, but it
// seemed slower.
let range = Uniform::new(GAP_SIZE, 0x11_0000);
let range = Uniform::new(GAP_SIZE, 0x11_0000).unwrap();

let mut n = range.sample(rng);
if n <= 0xDFFF {
Expand Down
2 changes: 1 addition & 1 deletion src/distributions/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl<'a, T> Slice<'a, T> {
0 => Err(EmptySlice),
len => Ok(Self {
slice,
range: Uniform::new(0, len),
range: Uniform::new(0, len).unwrap(),
}),
}
}
Expand Down
Loading

0 comments on commit 7d73990

Please sign in to comment.