-
-
Notifications
You must be signed in to change notification settings - Fork 447
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
Rand core changes #419
Rand core changes #419
Changes from all commits
15bd401
8492b75
8f3f57e
6c9013d
d8da965
3a8ae17
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 |
---|---|---|
|
@@ -27,7 +27,6 @@ use core::cmp::min; | |
use core::mem::size_of; | ||
use {RngCore, BlockRngCore, CryptoRng, SeedableRng, Error}; | ||
|
||
#[cfg(feature="serde1")] use serde::{Serialize, Deserialize}; | ||
|
||
/// Implement `next_u64` via `next_u32`, little-endian order. | ||
pub fn next_u64_via_u32<R: RngCore + ?Sized>(rng: &mut R) -> u64 { | ||
|
@@ -187,9 +186,6 @@ pub fn next_u64_via_fill<R: RngCore + ?Sized>(rng: &mut R) -> u64 { | |
#[derive(Clone)] | ||
#[cfg_attr(feature="serde1", derive(Serialize, Deserialize))] | ||
pub struct BlockRng<R: BlockRngCore + ?Sized> { | ||
#[cfg_attr(feature="serde1", serde(bound( | ||
serialize = "R::Results: Serialize", | ||
deserialize = "R::Results: Deserialize<'de>")))] | ||
results: R::Results, | ||
index: usize, | ||
core: R, | ||
|
@@ -253,7 +249,7 @@ impl<R: BlockRngCore> BlockRng<R> { | |
} | ||
|
||
impl<R: BlockRngCore<Item=u32>> RngCore for BlockRng<R> | ||
where <R as BlockRngCore>::Results: AsRef<[u32]> | ||
where <R as BlockRngCore>::Results: AsRef<[u32]> + AsMut<[u32]> | ||
{ | ||
#[inline(always)] | ||
fn next_u32(&mut self) -> u32 { | ||
|
@@ -386,9 +382,6 @@ impl<R: BlockRngCore + SeedableRng> SeedableRng for BlockRng<R> { | |
#[derive(Clone)] | ||
#[cfg_attr(feature="serde1", derive(Serialize, Deserialize))] | ||
pub struct BlockRng64<R: BlockRngCore + ?Sized> { | ||
#[cfg_attr(feature="serde1", serde(bound( | ||
serialize = "R::Results: Serialize", | ||
deserialize = "R::Results: Deserialize<'de>")))] | ||
results: R::Results, | ||
index: usize, | ||
half_used: bool, // true if only half of the previous result is used | ||
|
@@ -420,20 +413,42 @@ impl<R: BlockRngCore> BlockRng64<R> { | |
} | ||
} | ||
|
||
/// Return a reference the wrapped `BlockRngCore`. | ||
pub fn inner(&self) -> &R { | ||
&self.core | ||
} | ||
|
||
/// Return a mutable reference the wrapped `BlockRngCore`. | ||
pub fn inner(&mut self) -> &mut R { | ||
pub fn inner_mut(&mut self) -> &mut R { | ||
&mut self.core | ||
} | ||
|
||
// Reset the number of available results. | ||
// This will force a new set of results to be generated on next use. | ||
/// Get the index into the result buffer. | ||
/// | ||
/// If this is equal to or larger than the size of the result buffer then | ||
/// the buffer is "empty" and `generate()` must be called to produce new | ||
/// results. | ||
pub fn index(&self) -> usize { | ||
self.index | ||
} | ||
|
||
/// Reset the number of available results. | ||
/// This will force a new set of results to be generated on next use. | ||
pub fn reset(&mut self) { | ||
self.index = self.results.as_ref().len(); | ||
} | ||
|
||
/// Generate a new set of results immediately, setting the index to the | ||
/// given value. | ||
pub fn generate_and_set(&mut self, index: usize) { | ||
assert!(index < self.results.as_ref().len()); | ||
self.core.generate(&mut self.results); | ||
self.index = index; | ||
} | ||
} | ||
|
||
impl<R: BlockRngCore<Item=u64>> RngCore for BlockRng64<R> | ||
where <R as BlockRngCore>::Results: AsRef<[u64]> | ||
where <R as BlockRngCore>::Results: AsRef<[u64]> + AsMut<[u64]> | ||
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. Do we actually need the new bound here? (Also for 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. I initially tried to add the bound only on |
||
{ | ||
#[inline(always)] | ||
fn next_u32(&mut self) -> u32 { | ||
|
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.
Since we must expose both
inner
andinner_mut
and we will always have exactly one "core", there doesn't seem to be any advantage over just makingcore
apub
field (which would be nicer syntactically). Since we have to make a breaking change here anyway (and probably no other users will be affected yet), shall we just do this?For the other fields there is some advantage in keeping them private, but not really for
core
I think.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.
I don't think it matters much one way or the other. It feels a little strange to me to have one public field, but the rest private.
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.
It works reasonably well IMO — typical usage is like
ChaChaRng(BlockRng { core: ChaChaCore, .. })
; i.e. the outer type is free to access the inner type however is deemed fit, whereas the other fields are not quite so straightforward.