-
Notifications
You must be signed in to change notification settings - Fork 50
Feat: Implement Pod
trait on Uint types
#292
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
Changes from all commits
415bc16
db56a3d
61cf000
c28b3f2
d1f00e4
31b1ba4
5c4fc91
d64e88e
fc4d683
d4abbcc
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 |
---|---|---|
@@ -0,0 +1,82 @@ | ||
//! Support for the [`bytemuck`](https://crates.io/crates/bytemuck) crate. | ||
#![cfg(feature = "bytemuck")] | ||
#![cfg_attr(docsrs, doc(cfg(feature = "bytemuck")))] | ||
|
||
use bytemuck::{Pod, Zeroable}; | ||
use ruint::Uint; | ||
|
||
// Implement Zeroable for all `Uint` types. | ||
unsafe impl<const BITS: usize, const LIMBS: usize> Zeroable for Uint<{ BITS }, { LIMBS }> {} | ||
|
||
// Implement the `Pod` trait for `Uint` types with a size that is a multiple of | ||
// 64, up to 1024. Note that implementors must have a size that is divisible by | ||
// 64, and using `Uint` sizes not divisible by 64 would violate Pod's | ||
// guarantees potentially leading to undefined behavior. | ||
macro_rules! impl_pod { | ||
($(($bits:expr, $limbs:expr)),+ $(,)?) => { | ||
$( | ||
unsafe impl Pod for Uint<{$bits}, $limbs> {} | ||
)+ | ||
}; | ||
} | ||
|
||
impl_pod! { | ||
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. is it possible to replace this with a 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. Can you provide more insight on what this might look like? I can take a look into updating the implementation to use Would you want this updated before merging the current changes? 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. Just following up on this, let me know what you think. I'm happy to take a look into it if this would be blocking to get the PR merged. 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 don't consider this a blocker |
||
(64, 1), | ||
(128, 2), | ||
(192, 3), | ||
(256, 4), | ||
(320, 5), | ||
(384, 6), | ||
(448, 7), | ||
(512, 8), | ||
(576, 9), | ||
(640, 10), | ||
(704, 11), | ||
(768, 12), | ||
(832, 13), | ||
(896, 14), | ||
(960, 15), | ||
(1024, 16), | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use bytemuck::{Pod, Zeroable}; | ||
use ruint::Uint; | ||
|
||
#[test] | ||
fn test_uint_pod() { | ||
test_pod::<64, 1>(); | ||
test_pod::<128, 2>(); | ||
test_pod::<192, 3>(); | ||
test_pod::<256, 4>(); | ||
test_pod::<320, 5>(); | ||
test_pod::<384, 6>(); | ||
test_pod::<448, 7>(); | ||
test_pod::<512, 8>(); | ||
test_pod::<576, 9>(); | ||
test_pod::<640, 10>(); | ||
test_pod::<704, 11>(); | ||
test_pod::<768, 12>(); | ||
test_pod::<832, 13>(); | ||
test_pod::<896, 14>(); | ||
test_pod::<960, 15>(); | ||
test_pod::<1024, 16>(); | ||
} | ||
|
||
fn test_pod<const BITS: usize, const LIMBS: usize>() | ||
where | ||
Uint<{ BITS }, { LIMBS }>: Zeroable + Pod + Eq + Default, | ||
{ | ||
let val = Uint::<{ BITS }, { LIMBS }>::default(); | ||
let bytes = bytemuck::bytes_of(&val); | ||
|
||
assert_eq!( | ||
bytes.len(), | ||
std::mem::size_of::<Uint<{ BITS }, { LIMBS }>>() | ||
); | ||
|
||
let zeroed_val: Uint<{ BITS }, { LIMBS }> = Zeroable::zeroed(); | ||
assert_eq!(zeroed_val, Uint::<{ BITS }, { LIMBS }>::default()); | ||
} | ||
} |
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.
nit: these are really
literal
, as non-literalexpr
are invalid in this context. no change needed