-
-
Notifications
You must be signed in to change notification settings - Fork 450
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
Derive PartialEq+Eq. #975
Changes from all commits
264c668
3ec2677
1ac7a99
1f985df
617355d
20459bf
b20a21e
52c94bb
85e2825
17afa0a
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 |
---|---|---|
|
@@ -28,6 +28,24 @@ pub struct ChaCha { | |
pub(crate) d: vec128_storage, | ||
} | ||
|
||
// Custom PartialEq implementation as `vec128_storage` doesn't implement it. | ||
impl ::core::cmp::PartialEq for ChaCha { | ||
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 seems pretty non-ideal, but I'm hoping to not have to also try to get this change into 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. Hmm 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. Nothing wrong with a custom implementation, but that 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. Should I go try to get 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. You could ask @kazcw about that, but looks to me like you can just do 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. My previous version used 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 see this code was added since I don't wish to add this unsafe code, which makes us blocked on 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. @dhardy Sure, I can do that this weekend. |
||
fn eq(&self, other: &ChaCha) -> bool { | ||
unsafe { | ||
::core::mem::transmute::<vec128_storage, [u32; 4]>(self.b) | ||
== ::core::mem::transmute::<vec128_storage, [u32; 4]>(other.b) | ||
&& ::core::mem::transmute::<vec128_storage, [u32; 4]>(self.c) | ||
== ::core::mem::transmute::<vec128_storage, [u32; 4]>(other.c) | ||
&& ::core::mem::transmute::<vec128_storage, [u32; 4]>(self.d) | ||
== ::core::mem::transmute::<vec128_storage, [u32; 4]>(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, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
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 might be okay, but need to think about it. 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. Alternatively I can conditionally do |
||
|
||
/// Generate a new block of results. | ||
fn generate(&mut self, results: &mut Self::Results); | ||
|
@@ -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, | ||
|
@@ -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, | ||
|
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.
Some places used full paths for the ops traits, so I wasn't sure if that's preferred here.
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 care a lot. Since Edition 2018 you don't need the leading
::
.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.
If you have no preference, I'll just leave as is. If you'd prefer I remove the leading
::
I can do that too.