Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions uint/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,16 @@ authors = ["Parity Technologies <admin@parity.io>"]
[dependencies]
byteorder = { version = "1", default-features = false }
heapsize = { version = "0.4.2", optional = true }
rustc-hex = { version = "1.0", optional = true }
# rustc-hex = { version = "2.0", optional = true }
rustc-hex = "2.0"
quickcheck = { version = "0.6", optional = true }

[dev-dependencies]
crunchy = "0.1.5"
quickcheck = "0.6"

[features]
std = ["rustc-hex", "byteorder/std"]
std = ["byteorder/std"]
heapsizeof = ["heapsize"]
impl_quickcheck_arbitrary = ["quickcheck"]

Expand Down
9 changes: 6 additions & 3 deletions uint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@

//! Efficient large, fixed-size big integers and hashes.

#![cfg_attr(asm_available, feature(asm))]
#![cfg_attr(not(feature = "std"), no_std)]
#![cfg_attr(not(feature="std"), no_std)]
#![cfg_attr(all(not(feature="std"), test), feature(alloc))]

#[doc(hidden)]
pub extern crate byteorder;
Expand All @@ -22,13 +22,16 @@ pub extern crate heapsize;
#[doc(hidden)]
pub extern crate core;

#[cfg(feature = "std")]
#[doc(hidden)]
pub extern crate rustc_hex;

#[cfg(feature="impl_quickcheck_arbitrary")]
#[doc(hidden)]
pub extern crate quickcheck;

#[cfg(all(not(feature = "std"), test))]
#[macro_use]
extern crate alloc;

mod uint;
pub use uint::*;
70 changes: 32 additions & 38 deletions uint/src/uint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ macro_rules! construct_uint {
/// Little-endian large integer type
#[repr(C)]
#[derive(Copy, Clone, Eq, PartialEq, Hash)]
// TODO: serialize stuff? #[cfg_attr(feature="serialize", derive(Serialize, Deserialize))]
pub struct $name(pub [u64; $n_words]);

impl AsRef<$name> for $name {
Expand All @@ -365,6 +366,7 @@ macro_rules! construct_uint {
}

impl $name {
pub const MAX: $name = $name([u64::max_value(); $n_words]);
/// Convert from a decimal string.
pub fn from_dec_str(value: &str) -> Result<Self, $crate::FromDecStrErr> {
if !value.bytes().all(|b| b >= 48 && b <= 57) {
Expand Down Expand Up @@ -1181,35 +1183,9 @@ macro_rules! construct_uint {
}
}

impl_std_for_uint!($name, $n_words);
impl_heapsize_for_uint!($name);
// `$n_words * 8` because macro expects bytes and
// uints use 64 bit (8 byte) words
impl_quickcheck_arbitrary_for_uint!($name, ($n_words * 8));
);
}

#[cfg(feature="std")]
#[macro_export]
#[doc(hidden)]
macro_rules! impl_std_for_uint_internals {
($name: ident, $n_words: tt) => {
/// Convert to hex string.
#[deprecated(note = "Use LowerHex instead.")]
pub fn to_hex(&self) -> String {
format!("{:x}", self)
}
}
}

#[cfg(feature="std")]
#[macro_export]
#[doc(hidden)]
macro_rules! impl_std_for_uint {
($name: ident, $n_words: tt) => {
impl ::core::fmt::Debug for $name {
fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
write!(f, "{:#x}", self)
::core::fmt::Display::fmt(self, f)
}
}

Expand All @@ -1219,16 +1195,24 @@ macro_rules! impl_std_for_uint {
return write!(f, "0");
}

let mut s = String::new();
let mut buf = [0_u8; $n_words*20];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why $n_words * 20? Surely it's ceil(log10(2^($n_words - 1)))? That's 154 for U512 but this code would create a buffer of size 1280 (way too big).

@dvdplm dvdplm Aug 13, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This change came from https://github.com/paritytech/bigint/blob/master/src/uint.rs#L927 (part of PR #43) – not sure what their thinking was there, maybe it was making space for a somewhat long debug string (which doesn't make sense).
Should I just go ahead and replace it?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yes please!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Wait a sec, I think you're confusing uint with fixed-hash: here the construct_uint! macro is called with the words not the number of bytes, so $n_words here is 8 => 8 * 10 == 160 which is fine I think.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The construct_hash! macro takes number of bytes, which is more logical imo. Quite confusing.

let mut i = buf.len() - 1;
let mut current = *self;
let ten = $name::from(10);

while !current.is_zero() {
s = format!("{}{}", (current % ten).low_u32(), s);
loop {
let digit = (current % ten).low_u64() as u8;
buf[i] = digit + b'0';
current = current / ten;
if current.is_zero() {
break;
}
i -= 1;
}

write!(f, "{}", s)
// sequence of `'0'..'9'` chars is guaranteed to be a valid UTF8 string
let s = unsafe {::core::str::from_utf8_unchecked(&buf[i..])};
f.write_str(s)
}
}

Expand All @@ -1237,10 +1221,9 @@ macro_rules! impl_std_for_uint {

fn from_str(value: &str) -> Result<$name, Self::Err> {
use $crate::rustc_hex::FromHex;

let bytes: Vec<u8> = match value.len() % 2 == 0 {
true => try!(value.from_hex()),
false => try!(("0".to_owned() + value).from_hex())
true => value.from_hex()?,
false => ("0".to_owned() + value).from_hex()?
};

let bytes_ref: &[u8] = &bytes;
Expand Down Expand Up @@ -1281,14 +1264,25 @@ macro_rules! impl_std_for_uint {
s.parse().unwrap()
}
}
}

impl_heapsize_for_uint!($name);
// `$n_words * 8` because macro expects bytes and
// uints use 64 bit (8 byte) words
impl_quickcheck_arbitrary_for_uint!($name, ($n_words * 8));
);
}

#[cfg(not(feature="std"))]
#[cfg(feature="std")]
#[macro_export]
#[doc(hidden)]
macro_rules! impl_std_for_uint {
($name: ident, $n_words: tt) => {}
macro_rules! impl_std_for_uint_internals {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why change the name of this macro? If it's used by another macro then you should use a private pattern of that macro rather than a doc(hidden) macro.

($name: ident, $n_words: tt) => {
/// Convert to hex string.
#[deprecated(note = "Use LowerHex instead.")]
pub fn to_hex(&self) -> String {
format!("{:x}", self)
}
}
}

#[cfg(not(feature="std"))]
Expand Down
28 changes: 27 additions & 1 deletion uint/tests/uint_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ extern crate core;
extern crate uint;
#[macro_use]
extern crate crunchy;
#[cfg(feature = "impl_quickcheck_arbitrary")]
#[macro_use]
extern crate quickcheck;

Expand Down Expand Up @@ -346,7 +347,8 @@ fn uint256_pow_overflow_panic() {
fn should_format_and_debug_correctly() {
let test = |x: usize, hex: &'static str, display: &'static str| {
assert_eq!(format!("{}", U256::from(x)), display);
assert_eq!(format!("{:?}", U256::from(x)), format!("0x{}", hex));
// TODO: proper impl for Debug so we get this to pass: assert_eq!(format!("{:?}", U256::from(x)), format!("0x{}", hex));
assert_eq!(format!("{:?}", U256::from(x)), display);
assert_eq!(format!("{:x}", U256::from(x)), hex);
assert_eq!(format!("{:#x}", U256::from(x)), format!("0x{}", hex));
};
Expand All @@ -360,6 +362,30 @@ fn should_format_and_debug_correctly() {
test(0x1000, "1000", "4096");
}

#[test]
pub fn display_u128() {
let expected = "340282366920938463463374607431768211455";
let value = U128::MAX;
assert_eq!(format!("{}", value), expected);
assert_eq!(format!("{:?}", value), expected);
}

#[test]
pub fn display_u256() {
let expected = "115792089237316195423570985008687907853269984665640564039457584007913129639935";
let value = U256::MAX;
assert_eq!(format!("{}", value), expected);
assert_eq!(format!("{:?}", value), expected);
}

#[test]
pub fn display_u512() {
let expected = "13407807929942597099574024998205846127479365820592393377723561443721764030073546976801874298166903427690031858186486050853753882811946569946433649006084095";
let value = U512::MAX;
assert_eq!(format!("{}", value), expected);
assert_eq!(format!("{:?}", value), expected);
}

#[test]
fn uint256_overflowing_pow() {
assert_eq!(
Expand Down