Skip to content

Commit

Permalink
Initial test for using Borrow
Browse files Browse the repository at this point in the history
  • Loading branch information
sicking committed Jun 10, 2018
1 parent ec3d7ef commit 8b2dc23
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 13 deletions.
46 changes: 35 additions & 11 deletions src/distributions/uniform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@

#[cfg(feature = "std")]
use std::time::Duration;
use core::borrow::Borrow;


use Rng;
use distributions::Distribution;
Expand Down Expand Up @@ -229,12 +231,10 @@ pub trait UniformSampler: Sized {
/// sampling only a single value from the specified range. The default
/// implementation simply calls `UniformSampler::new` then `sample` on the
/// result.
fn sample_single<R: Rng + ?Sized>(low: Self::X, high: Self::X, rng: &mut R)
fn sample_single<R: Rng + ?Sized, B1, B2>(low: B1, high: B2, rng: &mut R)
-> Self::X
{
let uniform: Self = UniformSampler::new(low, high);
uniform.sample(rng)
}
where B1: Borrow<Self::X> + Sized,
B2: Borrow<Self::X> + Sized;

This comment has been minimized.

Copy link
@dhardy

dhardy Jun 11, 2018

Can we keep the default implementation? I guess this version would require X: Clone but presumably the full implementation would use Borrow in new too, so wouldn't.

Edit: I see there's less motivation for making this change to new, but in any case I don't think requiring X: Clone is a big deal?

This comment has been minimized.

Copy link
@sicking

sicking Jun 11, 2018

Author Owner

See this comment. The default implementation is only removed because this is a partial implementation. Once new uses Borrow it's no problem keeping the default implementation as-is without requiring Clone or anything else.

}

impl<X: SampleUniform> From<::core::ops::Range<X>> for Uniform<X> {
Expand Down Expand Up @@ -362,10 +362,13 @@ macro_rules! uniform_int_impl {
}
}

fn sample_single<R: Rng + ?Sized>(low: Self::X,
high: Self::X,
rng: &mut R) -> Self::X
fn sample_single<R: Rng + ?Sized, B1, B2>(low_b: B1, high_b: B2, rng: &mut R)
-> Self::X
where B1: Borrow<Self::X> + Sized,
B2: Borrow<Self::X> + Sized
{
let low = *low_b.borrow();
let high = *high_b.borrow();

This comment has been minimized.

Copy link
@dhardy

dhardy Jun 11, 2018

Ah, Self::X == $ty which is known at compile time; this type must support Copy for this operation, even though I'd expect the copy to not always be necessary.

Implementations for larger types like BigInt can find their own approach to copying/referencing here, so not a problem.

This comment has been minimized.

Copy link
@sicking

sicking Jun 11, 2018

Author Owner

Right. This only uses Copy here because we know we're dealing with intXs which are cheap to copy. Other types can copy, clone or just operate on the reference directly. Whatever is appropriate for that type.

assert!(low < high,
"Uniform::sample_single called with low >= high");
let range = high.wrapping_sub(low) as $unsigned as $u_large;
Expand Down Expand Up @@ -565,9 +568,13 @@ macro_rules! uniform_float_impl {
value1_2 * self.scale + self.offset
}

fn sample_single<R: Rng + ?Sized>(low: Self::X,
high: Self::X,
rng: &mut R) -> Self::X {
fn sample_single<R: Rng + ?Sized, B1, B2>(low_b: B1, high_b: B2, rng: &mut R)
-> Self::X
where B1: Borrow<Self::X> + Sized,
B2: Borrow<Self::X> + Sized
{
let low = *low_b.borrow();
let high = *high_b.borrow();
assert!(low < high,
"Uniform::sample_single called with low >= high");
let scale = high - low;
Expand Down Expand Up @@ -679,6 +686,15 @@ impl UniformSampler for UniformDuration {

self.offset + d
}

fn sample_single<R: Rng + ?Sized, B1, B2>(low: B1, high: B2, rng: &mut R)
-> Self::X
where B1: Borrow<Self::X> + Sized,
B2: Borrow<Self::X> + Sized
{
let uniform: Self = UniformSampler::new(*low.borrow(), *high.borrow());
uniform.sample(rng)
}
}

#[cfg(test)]
Expand Down Expand Up @@ -809,6 +825,7 @@ mod tests {

#[test]
fn test_custom_uniform() {
use core::borrow::Borrow;
#[derive(Clone, Copy, PartialEq, PartialOrd)]
struct MyF32 {
x: f32,
Expand All @@ -830,6 +847,13 @@ mod tests {
fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> Self::X {
MyF32 { x: self.inner.sample(rng) }
}
fn sample_single<R: Rng + ?Sized, B1, B2>(low: B1, high: B2, rng: &mut R)
-> Self::X
where B1: Borrow<Self::X> + Sized,
B2: Borrow<Self::X> + Sized
{
Self::new(*low.borrow(), *high.borrow()).sample(rng)
}
}
impl SampleUniform for MyF32 {
type Sampler = UniformMyF32;
Expand Down
6 changes: 4 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,10 @@ pub mod isaac {


use core::{mem, slice};
use core::borrow::Borrow;
use distributions::{Distribution, Standard};
use distributions::uniform::{SampleUniform, UniformSampler};


/// An automatically-implemented extension trait on [`RngCore`] providing high-level
/// generic methods for sampling values and other convenience methods.
///
Expand Down Expand Up @@ -387,7 +387,9 @@ pub trait Rng: RngCore {
/// ```
///
/// [`Uniform`]: distributions/uniform/struct.Uniform.html
fn gen_range<T: SampleUniform>(&mut self, low: T, high: T) -> T {
fn gen_range<T: SampleUniform, B1, B2>(&mut self, low: B1, high: B2) -> T
where B1: Borrow<T> + Sized,
B2: Borrow<T> + Sized {
T::Sampler::sample_single(low, high, self)
}

Expand Down

0 comments on commit 8b2dc23

Please sign in to comment.