-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Implement Random
for array
#136732
base: master
Are you sure you want to change the base?
Implement Random
for array
#136732
Conversation
Implement `Random` for `[T; N]`, where `Random` is implemented for `T`.
r? @ibraheemdev rustbot has assigned @ibraheemdev. Use |
r? @joboet |
I think it would be better to implement this with a dedicated method on the |
That doesn't necessarily need specialization. We could add an internal |
Calling fn random_array() -> [u8; 1_000] {
array::from_fn(|_| u8::random(&mut DefaultRandomSource))
}
fn random_array2() -> [u8; 1_000] {
let mut arr = [0; 1_000];
DefaultRandomSource.fill_bytes(&mut arr);
arr
} Calling
|
library/core/src/array/mod.rs
Outdated
let mut buf = [const { MaybeUninit::uninit() }; N]; | ||
for elem in &mut buf { | ||
elem.write(T::random(source)); | ||
} | ||
// SAFETY: all elements of the array were initialized. | ||
unsafe { mem::transmute_copy(&buf) } |
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.
Here's a version that uses fill_bytes()
.
let mut buf = [const { MaybeUninit::uninit() }; N]; | |
for elem in &mut buf { | |
elem.write(T::random(source)); | |
} | |
// SAFETY: all elements of the array were initialized. | |
unsafe { mem::transmute_copy(&buf) } | |
let mut buf = [const { MaybeUninit::<T>::uninit() }; N]; | |
// SAFETY: this initializes every element in the buffer with random bytes. | |
unsafe { | |
let slice = core::slice::from_raw_parts_mut(&raw mut buf as *mut u8, N * mem::size_of::<T>()); | |
source.fill_bytes(slice); | |
mem::transmute_copy(&buf) | |
} |
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.
This is unsound for generic Ts. There's no guarantee that a Type's random
impl shovels those bytes into their memory representation. A trivial example would be NonZero<u8>
which could implement Random
via rejection sampling. Setting it to 0 would be unsound.
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.
@the8472 I did notice that, so we would need to restrict this to types that can hold arbitrary bytes (which currently includes every type implementing Random
except bool
). Otherwise, use a slower custom implementation with rejection sampling or modulo.
This comment has been minimized.
This comment has been minimized.
Implements specialized `Random` for the array whose elements are integer primitives. This reduces the number of system calls.
7c195ab
to
5435be8
Compare
library/core/src/array/mod.rs
Outdated
macro_rules! impl_random_for_integer_array { | ||
($t:ty) => { | ||
#[unstable(feature = "random", issue = "130703")] | ||
impl<const N: usize> Random for [$t; N] { |
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.
Specializations should be hidden behind a private trait. https://std-dev-guide.rust-lang.org/policy/specialization.html
Alternatively you can avoid specialization by implementing a buffering random source wrapper as mentioned in #136732 (comment)
Implement
Random
for[T; N]
, whereRandom
is implemented forT
.rust-lang/libs-team#393 (comment) states:
This PR is also based on #130703 (comment).
Tracking issue: #130703