Skip to content
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

Derive PartialEq+Eq. #975

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -8,6 +8,9 @@ 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]
- Derive PartialEq+Eq for StdRng and SmallRng

## [0.7.3] - 2020-01-10
### Fixes
- The `Bernoulli` distribution constructors now reports an error on NaN and on
Expand Down
6 changes: 3 additions & 3 deletions Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "rand"
version = "0.7.3"
version = "0.7.4"
authors = ["The Rand Project Developers", "The Rust Project Developers"]
license = "MIT OR Apache-2.0"
readme = "README.md"
Expand Down Expand Up @@ -55,7 +55,7 @@ members = [
]

[dependencies]
rand_core = { path = "rand_core", version = "0.5.1" }
rand_core = { path = "rand_core", version = "0.5.2" }
rand_pcg = { path = "rand_pcg", version = "0.2", optional = true }
log = { version = "0.4.4", optional = true }

Expand All @@ -73,7 +73,7 @@ libc = { version = "0.2.22", optional = true, default-features = false }
# Emscripten does not support 128-bit integers, which are used by ChaCha code.
# We work around this by using a different RNG.
[target.'cfg(not(target_os = "emscripten"))'.dependencies]
rand_chacha = { path = "rand_chacha", version = "0.2.1", default-features = false, optional = true }
rand_chacha = { path = "rand_chacha", version = "0.2.2", default-features = false, optional = true }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your change will be in 0.2.3, if that's why you updated the number. (If not, well, 0.2.2 was not a mandatory upgrade.)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, good catch, will fix.

[target.'cfg(target_os = "emscripten")'.dependencies]
rand_hc = { path = "rand_hc", version = "0.2", optional = true }

Expand Down
3 changes: 3 additions & 0 deletions rand_chacha/CHANGELOG.md
Expand Up @@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
- Derive PartialEq+Eq for ChaChaXRng and ChaChaXCore

## [0.2.2] - 2020-03-09
- Integrate `c2-chacha`, reducing dependency count (#931)
- Add CryptoRng to ChaChaXCore (#944)
Expand Down
2 changes: 1 addition & 1 deletion rand_chacha/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "rand_chacha"
version = "0.2.2"
version = "0.2.3"
authors = ["The Rand Project Developers", "The Rust Project Developers", "The CryptoCorrosion Contributors"]
license = "MIT OR Apache-2.0"
readme = "README.md"
Expand Down
14 changes: 12 additions & 2 deletions rand_chacha/src/chacha.rs
Expand Up @@ -61,11 +61,21 @@ impl<T> fmt::Debug for Array64<T> {
write!(f, "Array64 {{}}")
}
}
impl<T> ::core::cmp::PartialEq for Array64<T>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some places used full paths for the ops traits, so I wasn't sure if that's preferred here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't care a lot. Since Edition 2018 you don't need the leading ::.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have no preference, I'll just leave as is. If you'd prefer I remove the leading :: I can do that too.

where T: ::core::cmp::PartialEq {
fn eq(&self, other: &Array64<T>) -> bool {
&self.0[..] == &other.0[..]
}
}
impl<T> ::core::cmp::Eq for Array64<T>
where T: ::core::cmp::Eq {
}


macro_rules! chacha_impl {
($ChaChaXCore:ident, $ChaChaXRng:ident, $rounds:expr, $doc:expr) => {
#[doc=$doc]
#[derive(Clone)]
#[derive(Clone, PartialEq, Eq)]
pub struct $ChaChaXCore {
state: ChaCha,
}
Expand Down Expand Up @@ -140,7 +150,7 @@ macro_rules! chacha_impl {
///
/// [^2]: [eSTREAM: the ECRYPT Stream Cipher Project](
/// http://www.ecrypt.eu.org/stream/)
#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct $ChaChaXRng {
rng: BlockRng<$ChaChaXCore>,
}
Expand Down
13 changes: 13 additions & 0 deletions rand_chacha/src/guts.rs
Expand Up @@ -28,6 +28,19 @@ pub struct ChaCha {
pub(crate) d: vec128_storage,
}

// Custom PartialEq implementation as `vec128_storage` doesn't implement it.
impl ::core::cmp::PartialEq for ChaCha {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems pretty non-ideal, but I'm hoping to not have to also try to get this change into vec128_storage.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm vec128_storage doesn't provide this Into implementation on all platforms. I'm going to do a transmute to [u32; 4] to see if that gets the build passing, but I'd like some input here on what an acceptable approach is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing wrong with a custom implementation, but that unsafe code — we've been trying to minimise usage, so I'm not keen on this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I go try to get PartialEq+Eq derived for vec128_storage upstream? I don't think there's a cross target way currently available to compare these without unsafe.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could ask @kazcw about that, but looks to me like you can just do let b: [u32; 4] = self.b.into();.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My previous version used Into<[u32; 4]> but it's not implemented on all platforms. Though that doesn't make a ton of sense to me based on the code you linked. Relevant build log: https://travis-ci.org/github/rust-random/rand/jobs/682708945

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this code was added since ppv-lite 0.2.6. @kazcw could you release a new version please?

I don't wish to add this unsafe code, which makes us blocked on ppv-lite.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhardy Sure, I can do that this weekend.

fn eq(&self, other: &ChaCha) -> bool {
Into::<[u128; 1]>::into(self.b) == Into::<[u128; 1]>::into(other.b)
&& Into::<[u128; 1]>::into(self.c) == Into::<[u128; 1]>::into(other.c)
&& Into::<[u128; 1]>::into(self.d) == Into::<[u128; 1]>::into(other.d)
}
}

// Custom Eq implementation as `vec128_storage` doesn't implement it.
impl ::core::cmp::Eq for ChaCha {
}

#[derive(Clone)]
pub struct State<V> {
pub(crate) a: V,
Expand Down
4 changes: 4 additions & 0 deletions rand_core/CHANGELOG.md
Expand Up @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
- Derive PartialEq+Eq for BlockRng and BlockRng64
- Add PartialEq+Eq constraint to BlockRngCore::Results
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this reasonable? Also should BlockRngCore itself have a where Self: PartialEq + Eq constraint at this point?


## [0.5.1] - 2019-08-28
- `OsRng` added to `rand_core` (#863)
- `Error::INTERNAL_START` and `Error::CUSTOM_START` constants (#864)
Expand Down
2 changes: 1 addition & 1 deletion rand_core/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "rand_core"
version = "0.5.1"
version = "0.5.2"
authors = ["The Rand Project Developers", "The Rust Project Developers"]
license = "MIT OR Apache-2.0"
readme = "README.md"
Expand Down
6 changes: 3 additions & 3 deletions rand_core/src/block.rs
Expand Up @@ -67,7 +67,7 @@ pub trait BlockRngCore {

/// Results type. This is the 'block' an RNG implementing `BlockRngCore`
/// generates, which will usually be an array like `[u32; 16]`.
type Results: AsRef<[Self::Item]> + AsMut<[Self::Item]> + Default;
type Results: AsRef<[Self::Item]> + AsMut<[Self::Item]> + Default + PartialEq + Eq;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be okay, but need to think about it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively I can conditionally do PartialEq+Eq on BlockRng where R::Results: PartialEq.


/// Generate a new block of results.
fn generate(&mut self, results: &mut Self::Results);
Expand Down Expand Up @@ -109,7 +109,7 @@ pub trait BlockRngCore {
/// [`next_u64`]: RngCore::next_u64
/// [`fill_bytes`]: RngCore::fill_bytes
/// [`try_fill_bytes`]: RngCore::try_fill_bytes
#[derive(Clone)]
#[derive(Clone, PartialEq, Eq)]
#[cfg_attr(feature = "serde1", derive(Serialize, Deserialize))]
pub struct BlockRng<R: BlockRngCore + ?Sized> {
results: R::Results,
Expand Down Expand Up @@ -272,7 +272,7 @@ impl<R: BlockRngCore + SeedableRng> SeedableRng for BlockRng<R> {
/// [`next_u64`]: RngCore::next_u64
/// [`fill_bytes`]: RngCore::fill_bytes
/// [`try_fill_bytes`]: RngCore::try_fill_bytes
#[derive(Clone)]
#[derive(Clone, PartialEq, Eq)]
#[cfg_attr(feature = "serde1", derive(Serialize, Deserialize))]
pub struct BlockRng64<R: BlockRngCore + ?Sized> {
results: R::Results,
Expand Down
3 changes: 3 additions & 0 deletions rand_hc/CHANGELOG.md
Expand Up @@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
- Derive PartialEq+Eq for Hc128Rng and Hc128Core

## [0.2.0] - 2019-06-12
- Bump minor crate version since rand_core bump is a breaking change
- Switch to Edition 2018
Expand Down
2 changes: 1 addition & 1 deletion rand_hc/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "rand_hc"
version = "0.2.0"
version = "0.2.1"
authors = ["The Rand Project Developers"]
license = "MIT OR Apache-2.0"
readme = "README.md"
Expand Down
14 changes: 13 additions & 1 deletion rand_hc/src/hc128.rs
Expand Up @@ -63,7 +63,7 @@ const SEED_WORDS: usize = 8; // 128 bit key followed by 128 bit iv
///
/// [^5]: Internet Engineering Task Force (February 2015),
/// ["Prohibiting RC4 Cipher Suites"](https://tools.ietf.org/html/rfc7465).
#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Hc128Rng(BlockRng<Hc128Core>);

impl RngCore for Hc128Rng {
Expand Down Expand Up @@ -118,6 +118,18 @@ impl fmt::Debug for Hc128Core {
}
}

// Custom PartialEq implementation as it can't currently be derived from an array of size 1024
impl ::core::cmp::PartialEq for Hc128Core {
fn eq(&self, other: &Hc128Core) -> bool {
&self.t[..] == &other.t[..]
&& self.counter1024 == other.counter1024
}
}

// Custom Eq implementation as it can't currently be derived from an array of size 1024
impl ::core::cmp::Eq for Hc128Core {
}

impl BlockRngCore for Hc128Core {
type Item = u32;
type Results = [u32; 16];
Expand Down
3 changes: 3 additions & 0 deletions rand_pcg/CHANGELOG.md
Expand Up @@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
- Derive PartialEq+Eq for Lcg64Xsh32, Lcg128Xsl64, and Mcg128Xsl64

## [0.2.1] - 2019-10-22
- Bump `bincode` version to 1.1.4 to fix minimal-dependency builds
- Removed unused `autocfg` build dependency.
Expand Down
2 changes: 1 addition & 1 deletion rand_pcg/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "rand_pcg"
version = "0.2.1"
version = "0.2.2"
authors = ["The Rand Project Developers"]
license = "MIT OR Apache-2.0"
readme = "README.md"
Expand Down
4 changes: 2 additions & 2 deletions rand_pcg/src/pcg128.rs
Expand Up @@ -29,7 +29,7 @@ use rand_core::{le, Error, RngCore, SeedableRng};
/// Despite the name, this implementation uses 32 bytes (256 bit) space
/// comprising 128 bits of state and 128 bits stream selector. These are both
/// set by `SeedableRng`, using a 256-bit seed.
#[derive(Clone)]
#[derive(Clone, PartialEq, Eq)]
#[cfg_attr(feature = "serde1", derive(Serialize, Deserialize))]
pub struct Lcg128Xsl64 {
state: u128,
Expand Down Expand Up @@ -130,7 +130,7 @@ impl RngCore for Lcg128Xsl64 {
/// Note that compared to the standard `pcg64` (128-bit LCG with PCG-XSL-RR
/// output function), this RNG is faster, also has a long cycle, and still has
/// good performance on statistical tests.
#[derive(Clone)]
#[derive(Clone, PartialEq, Eq)]
#[cfg_attr(feature = "serde1", derive(Serialize, Deserialize))]
pub struct Mcg128Xsl64 {
state: u128,
Expand Down
2 changes: 1 addition & 1 deletion rand_pcg/src/pcg64.rs
Expand Up @@ -29,7 +29,7 @@ const MULTIPLIER: u64 = 6364136223846793005;
/// Despite the name, this implementation uses 16 bytes (128 bit) space
/// comprising 64 bits of state and 64 bits stream selector. These are both set
/// by `SeedableRng`, using a 128-bit seed.
#[derive(Clone)]
#[derive(Clone, PartialEq, Eq)]
#[cfg_attr(feature = "serde1", derive(Serialize, Deserialize))]
pub struct Lcg64Xsh32 {
state: u64,
Expand Down
2 changes: 1 addition & 1 deletion src/rngs/small.rs
Expand Up @@ -73,7 +73,7 @@ type Rng = rand_pcg::Pcg32;
/// [`thread_rng`]: crate::thread_rng
/// [rand_chacha]: https://crates.io/crates/rand_chacha
/// [rand_pcg]: https://crates.io/crates/rand_pcg
#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct SmallRng(Rng);

impl RngCore for SmallRng {
Expand Down
2 changes: 1 addition & 1 deletion src/rngs/std.rs
Expand Up @@ -32,7 +32,7 @@ pub(crate) use rand_hc::Hc128Core as Core;
/// the [rand_chacha] crate directly.
///
/// [rand_chacha]: https://crates.io/crates/rand_chacha
#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct StdRng(Rng);

impl RngCore for StdRng {
Expand Down