-
Notifications
You must be signed in to change notification settings - Fork 421
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
[WIP] Implement Fill
for some MaybeUninit types
#1241
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,12 +8,13 @@ | |
// except according to those terms. | ||
|
||
//! [`Rng`] trait | ||
use core::num::Wrapping; | ||
use core::{mem, slice}; | ||
|
||
use rand_core::{Error, RngCore}; | ||
|
||
use crate::distributions::uniform::{SampleRange, SampleUniform}; | ||
use crate::distributions::{self, Distribution, Standard}; | ||
use core::num::Wrapping; | ||
use core::{mem, slice}; | ||
|
||
/// An automatically-implemented extension trait on [`RngCore`] providing high-level | ||
/// generic methods for sampling values and other convenience methods. | ||
|
@@ -91,7 +92,9 @@ pub trait Rng: RngCore { | |
/// [`Standard`]: distributions::Standard | ||
#[inline] | ||
fn gen<T>(&mut self) -> T | ||
where Standard: Distribution<T> { | ||
where | ||
Standard: Distribution<T>, | ||
{ | ||
Standard.sample(self) | ||
} | ||
|
||
|
@@ -129,7 +132,7 @@ pub trait Rng: RngCore { | |
fn gen_range<T, R>(&mut self, range: R) -> T | ||
where | ||
T: SampleUniform, | ||
R: SampleRange<T> | ||
R: SampleRange<T>, | ||
{ | ||
assert!(!range.is_empty(), "cannot sample empty range"); | ||
range.sample_single(self) | ||
|
@@ -216,7 +219,8 @@ pub trait Rng: RngCore { | |
/// [`fill_bytes`]: RngCore::fill_bytes | ||
/// [`try_fill`]: Rng::try_fill | ||
fn fill<T: Fill + ?Sized>(&mut self, dest: &mut T) { | ||
dest.try_fill(self).unwrap_or_else(|_| panic!("Rng::fill failed")) | ||
dest.try_fill(self) | ||
.unwrap_or_else(|_| panic!("Rng::fill failed")) | ||
} | ||
|
||
/// Fill any type implementing [`Fill`] with random data | ||
|
@@ -316,25 +320,25 @@ pub trait Fill { | |
fn try_fill<R: Rng + ?Sized>(&mut self, rng: &mut R) -> Result<(), Error>; | ||
} | ||
|
||
macro_rules! impl_fill_each { | ||
macro_rules! impl_fill_each_uninit { | ||
() => {}; | ||
($t:ty) => { | ||
impl Fill for [$t] { | ||
impl Fill for [mem::MaybeUninit<$t>] { | ||
fn try_fill<R: Rng + ?Sized>(&mut self, rng: &mut R) -> Result<(), Error> { | ||
for elt in self.iter_mut() { | ||
*elt = rng.gen(); | ||
*elt = mem::MaybeUninit::new(rng.gen()); | ||
} | ||
Ok(()) | ||
} | ||
} | ||
}; | ||
($t:ty, $($tt:ty,)*) => { | ||
impl_fill_each!($t); | ||
impl_fill_each!($($tt,)*); | ||
impl_fill_each_uninit!($t); | ||
impl_fill_each_uninit!($($tt,)*); | ||
}; | ||
} | ||
|
||
impl_fill_each!(bool, char, f32, f64,); | ||
impl_fill_each_uninit!(bool, char, f32, f64,); | ||
|
||
impl Fill for [u8] { | ||
fn try_fill<R: Rng + ?Sized>(&mut self, rng: &mut R) -> Result<(), Error> { | ||
|
@@ -392,10 +396,36 @@ macro_rules! impl_fill { | |
impl_fill!(u16, u32, u64, usize, u128,); | ||
impl_fill!(i8, i16, i32, i64, isize, i128,); | ||
|
||
macro_rules! impl_fill_delegate_to_maybe_uninit{ | ||
() => {}; | ||
($t:ty) => { | ||
impl Fill for [$t] { | ||
fn try_fill<R: Rng + ?Sized>(&mut self, rng: &mut R) -> Result<(), Error> { | ||
Fill::try_fill( | ||
unsafe { | ||
slice::from_raw_parts_mut(self.as_mut_ptr() | ||
as *mut mem::MaybeUninit<$t>, | ||
self.len() | ||
) | ||
}, | ||
rng, | ||
) | ||
} | ||
} | ||
}; | ||
($t:ty, $($tt:ty,)*) => { | ||
impl_fill_delegate_to_maybe_uninit!($t); | ||
impl_fill_delegate_to_maybe_uninit!($($tt,)*); | ||
}; | ||
} | ||
|
||
impl_fill_delegate_to_maybe_uninit!(bool, char, f32, f64,); | ||
Comment on lines
+399
to
+422
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This part doesn't make much sense to me: replacing what was a safe impl of |
||
|
||
#[cfg_attr(doc_cfg, doc(cfg(feature = "min_const_gen")))] | ||
#[cfg(feature = "min_const_gen")] | ||
impl<T, const N: usize> Fill for [T; N] | ||
where [T]: Fill | ||
where | ||
[T]: Fill, | ||
{ | ||
fn try_fill<R: Rng + ?Sized>(&mut self, rng: &mut R) -> Result<(), Error> { | ||
self[..].try_fill(rng) | ||
|
@@ -431,9 +461,10 @@ impl_fill_arrays!(!div 4096, N,N,N,N,N,N,N,); | |
#[cfg(test)] | ||
mod test { | ||
use super::*; | ||
use crate::test::rng; | ||
use crate::rngs::mock::StepRng; | ||
#[cfg(feature = "alloc")] use alloc::boxed::Box; | ||
use crate::test::rng; | ||
#[cfg(feature = "alloc")] | ||
use alloc::boxed::Box; | ||
|
||
#[test] | ||
fn test_fill_bytes_default() { | ||
|
@@ -485,6 +516,25 @@ mod test { | |
assert_eq!(array, gen); | ||
} | ||
|
||
#[test] | ||
fn test_gen_range_uninit_same_as_simple() { | ||
let mut r = rng(101); | ||
let mut mu_array: [mem::MaybeUninit<char>; 64] = unsafe { | ||
// Safe to assume init because inner values have uninit type. | ||
mem::MaybeUninit::uninit().assume_init() | ||
Comment on lines
+523
to
+524
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you read the doc on It sounds like you want to cast There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope, you are wrong. This code is same as in MaybeUninit docs actually. And I make an array of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct. It's just your comment that is wrong then: the reason it is safe is that the outer type is an array of |
||
}; | ||
r.fill(&mut mu_array); | ||
|
||
let mut r = rng(101); | ||
let mut array = ['\0'; 64]; | ||
r.fill(&mut array); | ||
|
||
assert!(mu_array.iter().zip(&array).all(|(&a, &b)| unsafe { | ||
// This assume_init should trigger Miri if we done something wrong | ||
a.assume_init() == b | ||
})) | ||
} | ||
|
||
#[test] | ||
fn test_fill_empty() { | ||
let mut array = [0u32; 0]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementing
Fill for [MaybeUninit<bool>]
(etc.) is okay in itself, but I don't think it very useful. In order to use it with uninitialized memory,assume_init
is needed somewhere, which thus requires the user useunsafe
code (and based on the assumption thatfill
succeeded). I mean, users can use this API correctly, but not easily, and it isn't difficult to implement equivalent functionality in userspace anyway.