From f7c641b8b62a36ce23bd000644f19cf8761007b0 Mon Sep 17 00:00:00 2001 From: Irfan Hudda Date: Tue, 21 Mar 2017 22:11:13 +0530 Subject: [PATCH 1/8] Improve documentation of next_power_of_two Clarify overflow behavior of `next_power_of_two`. Related Issue: #18604 --- src/libcore/num/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libcore/num/mod.rs b/src/libcore/num/mod.rs index f665cfdee77ae..487b0dba3c4e9 100644 --- a/src/libcore/num/mod.rs +++ b/src/libcore/num/mod.rs @@ -2328,7 +2328,7 @@ macro_rules! uint_impl { } /// Returns the smallest power of two greater than or equal to `self`. - /// Unspecified behavior on overflow. + /// More details about overflow behavior can be found in [RFC 560]. /// /// # Examples /// @@ -2338,6 +2338,8 @@ macro_rules! uint_impl { /// assert_eq!(2u8.next_power_of_two(), 2); /// assert_eq!(3u8.next_power_of_two(), 4); /// ``` + /// + /// [RFC 560]: https://github.com/rust-lang/rfcs/blob/master/text/0560-integer-overflow.md #[stable(feature = "rust1", since = "1.0.0")] #[inline] pub fn next_power_of_two(self) -> Self { From 04e9c20228c38c20c5d4f84328332b2c5c98d5a2 Mon Sep 17 00:00:00 2001 From: Irfan Hudda Date: Wed, 19 Apr 2017 23:59:43 +0530 Subject: [PATCH 2/8] next_power_of_two panic on overflow --- src/libcore/num/mod.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libcore/num/mod.rs b/src/libcore/num/mod.rs index 487b0dba3c4e9..15ffe9db8ce6b 100644 --- a/src/libcore/num/mod.rs +++ b/src/libcore/num/mod.rs @@ -2345,7 +2345,11 @@ macro_rules! uint_impl { pub fn next_power_of_two(self) -> Self { let bits = size_of::() * 8; let one: Self = 1; - one << ((bits - self.wrapping_sub(one).leading_zeros() as usize) % bits) + if self == 0 { + 1 + } else { + one << (bits - self.wrapping_sub(one).leading_zeros() as usize) + } } /// Returns the smallest power of two greater than or equal to `n`. If @@ -2363,7 +2367,9 @@ macro_rules! uint_impl { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn checked_next_power_of_two(self) -> Option { - let npot = self.next_power_of_two(); + let bits = size_of::() * 8; + let one: Self = 1; + let npot = one << ((bits - self.wrapping_sub(one).leading_zeros() as usize) % bits); if npot >= self { Some(npot) } else { From f7bd370818b003f57e47a1894ff0d791ad23628d Mon Sep 17 00:00:00 2001 From: Irfan Hudda Date: Thu, 20 Apr 2017 19:51:38 +0530 Subject: [PATCH 3/8] remove redundant wrapping_sub --- src/libcore/num/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/num/mod.rs b/src/libcore/num/mod.rs index 15ffe9db8ce6b..bef7a20cd7f3e 100644 --- a/src/libcore/num/mod.rs +++ b/src/libcore/num/mod.rs @@ -2348,7 +2348,7 @@ macro_rules! uint_impl { if self == 0 { 1 } else { - one << (bits - self.wrapping_sub(one).leading_zeros() as usize) + one << (bits - (self - one).leading_zeros() as usize) } } From 8f9caff988954ab19f4a242d905aa6092a03bddd Mon Sep 17 00:00:00 2001 From: Irfan Hudda Date: Fri, 28 Apr 2017 21:50:02 +0530 Subject: [PATCH 4/8] Add helper functions for `next_power_of_two` and `checked_next_power_of_two` --- src/libcore/num/mod.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/libcore/num/mod.rs b/src/libcore/num/mod.rs index bef7a20cd7f3e..0c7832168bb93 100644 --- a/src/libcore/num/mod.rs +++ b/src/libcore/num/mod.rs @@ -2327,6 +2327,18 @@ macro_rules! uint_impl { (self.wrapping_sub(1)) & self == 0 && !(self == 0) } + fn round_up_to_one_less_than_a_power_of_two(self) -> Self { + let bits = size_of::() as u32 * 8; + let z = self.leading_zeros(); + (if z == bits { 0 as Self } else { !0 }).wrapping_shr(z) + } + + fn one_less_than_next_power_of_two(self) -> Self { + self.wrapping_sub(1) + .round_up_to_one_less_than_a_power_of_two() + .wrapping_add(if self == 0 { 1 } else { 0 }) + } + /// Returns the smallest power of two greater than or equal to `self`. /// More details about overflow behavior can be found in [RFC 560]. /// @@ -2343,13 +2355,7 @@ macro_rules! uint_impl { #[stable(feature = "rust1", since = "1.0.0")] #[inline] pub fn next_power_of_two(self) -> Self { - let bits = size_of::() * 8; - let one: Self = 1; - if self == 0 { - 1 - } else { - one << (bits - (self - one).leading_zeros() as usize) - } + self.one_less_than_next_power_of_two() + 1 } /// Returns the smallest power of two greater than or equal to `n`. If @@ -2367,9 +2373,7 @@ macro_rules! uint_impl { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn checked_next_power_of_two(self) -> Option { - let bits = size_of::() * 8; - let one: Self = 1; - let npot = one << ((bits - self.wrapping_sub(one).leading_zeros() as usize) % bits); + let npot = self.one_less_than_next_power_of_two().wrapping_add(1); if npot >= self { Some(npot) } else { From 67684a399c2020495075b2e6c848850e7a8d60e9 Mon Sep 17 00:00:00 2001 From: Irfan Hudda Date: Sat, 29 Apr 2017 00:06:54 +0530 Subject: [PATCH 5/8] Add comment about overflow behavior for next_power_of_two --- src/libcore/num/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libcore/num/mod.rs b/src/libcore/num/mod.rs index 0c7832168bb93..8c4a6e871e125 100644 --- a/src/libcore/num/mod.rs +++ b/src/libcore/num/mod.rs @@ -2340,6 +2340,9 @@ macro_rules! uint_impl { } /// Returns the smallest power of two greater than or equal to `self`. + /// When return value overflows, it panics in debug mode and return + /// value is wrapped in release mode. + /// /// More details about overflow behavior can be found in [RFC 560]. /// /// # Examples From ed24829985cd01ae7ab980d785b6dff2e8332868 Mon Sep 17 00:00:00 2001 From: Irfan Hudda Date: Sat, 29 Apr 2017 01:51:54 +0530 Subject: [PATCH 6/8] Simplify `checked_next_power_of_two` function --- src/libcore/num/mod.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/libcore/num/mod.rs b/src/libcore/num/mod.rs index 8c4a6e871e125..cc1f9ca79a150 100644 --- a/src/libcore/num/mod.rs +++ b/src/libcore/num/mod.rs @@ -2376,12 +2376,7 @@ macro_rules! uint_impl { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn checked_next_power_of_two(self) -> Option { - let npot = self.one_less_than_next_power_of_two().wrapping_add(1); - if npot >= self { - Some(npot) - } else { - None - } + self.one_less_than_next_power_of_two().checked_add(1) } } } From 93219a262706447c52f3f20819e5caa7b2e2e3a2 Mon Sep 17 00:00:00 2001 From: Irfan Hudda Date: Tue, 23 May 2017 23:04:23 +0530 Subject: [PATCH 7/8] Add comments to explain helper functions --- src/libcore/num/mod.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/libcore/num/mod.rs b/src/libcore/num/mod.rs index cc1f9ca79a150..8fd21d77dc316 100644 --- a/src/libcore/num/mod.rs +++ b/src/libcore/num/mod.rs @@ -2327,12 +2327,22 @@ macro_rules! uint_impl { (self.wrapping_sub(1)) & self == 0 && !(self == 0) } + // Returns one less than next greater power of two. + // (For 8u8 next greater power of two is 16u8 and for 6u8 it is 8u8) + // + // 8u8.round_up_to_one_less_than_a_power_of_two() == 15 + // 6u8.round_up_to_one_less_than_a_power_of_two() == 7 fn round_up_to_one_less_than_a_power_of_two(self) -> Self { let bits = size_of::() as u32 * 8; let z = self.leading_zeros(); (if z == bits { 0 as Self } else { !0 }).wrapping_shr(z) } + // Returns one less than next power of two. + // (For 8u8 next power of two is 8u8 and for 6u8 it is 8u8) + // + // 8u8.one_less_than_next_power_of_two() == 7 + // 6u8.one_less_than_next_power_of_two() == 7 fn one_less_than_next_power_of_two(self) -> Self { self.wrapping_sub(1) .round_up_to_one_less_than_a_power_of_two() @@ -2340,10 +2350,10 @@ macro_rules! uint_impl { } /// Returns the smallest power of two greater than or equal to `self`. - /// When return value overflows, it panics in debug mode and return - /// value is wrapped in release mode. /// - /// More details about overflow behavior can be found in [RFC 560]. + /// When return value overflows (i.e. `self > (1 << (N-1))` for type + /// `uN`), it panics in debug mode and return value is wrapped to 0 in + /// release mode (the only situation in which method can return 0). /// /// # Examples /// @@ -2353,8 +2363,6 @@ macro_rules! uint_impl { /// assert_eq!(2u8.next_power_of_two(), 2); /// assert_eq!(3u8.next_power_of_two(), 4); /// ``` - /// - /// [RFC 560]: https://github.com/rust-lang/rfcs/blob/master/text/0560-integer-overflow.md #[stable(feature = "rust1", since = "1.0.0")] #[inline] pub fn next_power_of_two(self) -> Self { From 18fadb61c4aab3d1b2fc49ac7d7fea85b3914fb3 Mon Sep 17 00:00:00 2001 From: Irfan Hudda Date: Wed, 31 May 2017 22:25:15 +0530 Subject: [PATCH 8/8] Simplify helper functions Based on @scottmcm 's suggestion --- src/libcore/num/mod.rs | 28 +++++++++++++--------------- src/libstd/num.rs | 5 ++++- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/libcore/num/mod.rs b/src/libcore/num/mod.rs index 8fd21d77dc316..e5a8cfdd9a1d5 100644 --- a/src/libcore/num/mod.rs +++ b/src/libcore/num/mod.rs @@ -15,7 +15,6 @@ use convert::TryFrom; use fmt; use intrinsics; -use mem::size_of; use str::FromStr; /// Provides intentionally-wrapped arithmetic on `T`. @@ -2327,26 +2326,25 @@ macro_rules! uint_impl { (self.wrapping_sub(1)) & self == 0 && !(self == 0) } - // Returns one less than next greater power of two. - // (For 8u8 next greater power of two is 16u8 and for 6u8 it is 8u8) - // - // 8u8.round_up_to_one_less_than_a_power_of_two() == 15 - // 6u8.round_up_to_one_less_than_a_power_of_two() == 7 - fn round_up_to_one_less_than_a_power_of_two(self) -> Self { - let bits = size_of::() as u32 * 8; - let z = self.leading_zeros(); - (if z == bits { 0 as Self } else { !0 }).wrapping_shr(z) - } - // Returns one less than next power of two. // (For 8u8 next power of two is 8u8 and for 6u8 it is 8u8) // // 8u8.one_less_than_next_power_of_two() == 7 // 6u8.one_less_than_next_power_of_two() == 7 + // + // This method cannot overflow, as in the `next_power_of_two` + // overflow cases it instead ends up returning the maximum value + // of the type, and can return 0 for 0. fn one_less_than_next_power_of_two(self) -> Self { - self.wrapping_sub(1) - .round_up_to_one_less_than_a_power_of_two() - .wrapping_add(if self == 0 { 1 } else { 0 }) + if self <= 1 { return 0; } + + // Because `p > 0`, it cannot consist entirely of leading zeros. + // That means the shift is always in-bounds, and some processors + // (such as intel pre-haswell) have more efficient ctlz + // intrinsics when the argument is non-zero. + let p = self - 1; + let z = p.leading_zeros(); + <$SelfT>::max_value() >> z } /// Returns the smallest power of two greater than or equal to `self`. diff --git a/src/libstd/num.rs b/src/libstd/num.rs index 5f83d077a1368..894a6e6796ca6 100644 --- a/src/libstd/num.rs +++ b/src/libstd/num.rs @@ -176,7 +176,10 @@ mod tests { fn $test_name() { #![test] assert_eq!((0 as $T).checked_next_power_of_two(), Some(1)); - assert!(($T::MAX / 2).checked_next_power_of_two().is_some()); + let smax = $T::MAX >> 1; + assert_eq!(smax.checked_next_power_of_two(), Some(smax+1)); + assert_eq!((smax + 1).checked_next_power_of_two(), Some(smax + 1)); + assert_eq!((smax + 2).checked_next_power_of_two(), None); assert_eq!(($T::MAX - 1).checked_next_power_of_two(), None); assert_eq!($T::MAX.checked_next_power_of_two(), None); let mut next_power = 1;