auto merge of #19640 : aliblong/rust/power_of_two_reform, r=Gankro

The `is_power_of_two()` method of the `UnsignedInt` trait currently returns `true` for `self == 0`. Zero is not a power of two, assuming an integral exponent `k >= 0`. I've therefore moved this functionality to the new method `is_power_of_two_or_zero()` and reformed `is_power_of_two()` to return false for `self == 0`.

To illustrate the usefulness of the existence of both functions, consider `HashMap`. Its capacity must be zero or a power of two; conversely, it also requires a (non-zero) power of two for key and val alignment.

Also, added a small amount of documentation regarding #18604.

4 files changed, 43 insertions, 37 deletions

diff --git a/src/libcollections/vec.rs b/src/libcollections/vec.rs
index 94e6103f05f..e986b204430 100644
--- a/src/libcollections/vec.rs
+++ b/src/libcollections/vec.rs
@@ -710,20 +710,10 @@ impl<T> Vec<T> {
     #[unstable = "matches collection reform specification, waiting for dust to settle"]
     pub fn reserve(&mut self, additional: uint) {
         if self.cap - self.len < additional {
-            match self.len.checked_add(additional) {
-                None => panic!("Vec::reserve: `uint` overflow"),
-                // if the checked_add
-                Some(new_cap) => {
-                    let amort_cap = new_cap.next_power_of_two();
-                    // next_power_of_two will overflow to exactly 0 for really big capacities
-                    let cap = if amort_cap == 0 {
-                        new_cap
-                    } else {
-                        amort_cap
-                    };
-                    self.grow_capacity(cap)
-                }
-            }
+            let err_msg = "Vec::reserve: `uint` overflow";
+            let new_cap = self.len.checked_add(additional).expect(err_msg)
+                                  .checked_next_power_of_two().expect(err_msg);
+            self.grow_capacity(new_cap);
         }
     }
 
diff --git a/src/libcore/num/mod.rs b/src/libcore/num/mod.rs
index b4f867b4bb4..84d1d8e459a 100644
--- a/src/libcore/num/mod.rs
+++ b/src/libcore/num/mod.rs
@@ -673,35 +673,30 @@ signed_int_impl! { int }
 #[unstable = "recently settled as part of numerics reform"]
 pub trait UnsignedInt: Int {
     /// Returns `true` iff `self == 2^k` for some `k`.
+    #[inline]
     fn is_power_of_two(self) -> bool {
-        (self - Int::one()) & self == Int::zero()
+        (self - Int::one()) & self == Int::zero() && !(self == Int::zero())
     }
 
     /// Returns the smallest power of two greater than or equal to `self`.
+    /// Unspecified behavior on overflow.
     #[inline]
     fn next_power_of_two(self) -> Self {
-        let halfbits = size_of::<Self>() * 4;
-        let mut tmp = self - Int::one();
-        let mut shift = 1u;
-        while shift <= halfbits {
-            tmp = tmp | (tmp >> shift);
-            shift = shift << 1u;
-        }
-        tmp + Int::one()
+        let bits = size_of::<Self>() * 8;
+        let one: Self = Int::one();
+        one << ((bits - (self - one).leading_zeros()) % bits)
     }
 
     /// Returns the smallest power of two greater than or equal to `n`. If the
     /// next power of two is greater than the type's maximum value, `None` is
     /// returned, otherwise the power of two is wrapped in `Some`.
     fn checked_next_power_of_two(self) -> Option<Self> {
-        let halfbits = size_of::<Self>() * 4;
-        let mut tmp = self - Int::one();
-        let mut shift = 1u;
-        while shift <= halfbits {
-            tmp = tmp | (tmp >> shift);
-            shift = shift << 1u;
+        let npot = self.next_power_of_two();
+        if npot >= self {
+            Some(npot)
+        } else {
+            None
         }
-        tmp.checked_add(Int::one())
     }
 }
 
diff --git a/src/libstd/collections/hash/map.rs b/src/libstd/collections/hash/map.rs
index 04dd5afdfa2..6bfea7e3cb2 100644
--- a/src/libstd/collections/hash/map.rs
+++ b/src/libstd/collections/hash/map.rs
@@ -623,10 +623,10 @@ impl<K: Eq + Hash<S>, V, S, H: Hasher<S>> HashMap<K, V, H> {
     /// Resizes the internal vectors to a new capacity. It's your responsibility to:
     ///   1) Make sure the new capacity is enough for all the elements, accounting
     ///      for the load factor.
-    ///   2) Ensure new_capacity is a power of two.
+    ///   2) Ensure new_capacity is a power of two or zero.
     fn resize(&mut self, new_capacity: uint) {
         assert!(self.table.size() <= new_capacity);
-        assert!(new_capacity.is_power_of_two());
+        assert!(new_capacity.is_power_of_two() || new_capacity == 0);
 
         let mut old_table = replace(&mut self.table, RawTable::new(new_capacity));
         let old_size = old_table.size();
diff --git a/src/libstd/num/mod.rs b/src/libstd/num/mod.rs
index a568aafe1ed..fdece4fbc0d 100644
--- a/src/libstd/num/mod.rs
+++ b/src/libstd/num/mod.rs
@@ -664,11 +664,32 @@ mod tests {
         assert_eq!(third.checked_mul(4), None);
     }
 
+    macro_rules! test_is_power_of_two {
+        ($test_name:ident, $T:ident) => (
+            fn $test_name() {
+                #![test]
+                assert_eq!((0 as $T).is_power_of_two(), false);
+                assert_eq!((1 as $T).is_power_of_two(), true);
+                assert_eq!((2 as $T).is_power_of_two(), true);
+                assert_eq!((3 as $T).is_power_of_two(), false);
+                assert_eq!((4 as $T).is_power_of_two(), true);
+                assert_eq!((5 as $T).is_power_of_two(), false);
+                assert!(($T::MAX / 2 + 1).is_power_of_two(), true);
+            }
+        )
+    }
+
+    test_is_power_of_two!{ test_is_power_of_two_u8, u8 }
+    test_is_power_of_two!{ test_is_power_of_two_u16, u16 }
+    test_is_power_of_two!{ test_is_power_of_two_u32, u32 }
+    test_is_power_of_two!{ test_is_power_of_two_u64, u64 }
+    test_is_power_of_two!{ test_is_power_of_two_uint, uint }
+
     macro_rules! test_next_power_of_two {
         ($test_name:ident, $T:ident) => (
             fn $test_name() {
                 #![test]
-                assert_eq!((0 as $T).next_power_of_two(), 0);
+                assert_eq!((0 as $T).next_power_of_two(), 1);
                 let mut next_power = 1;
                 for i in range::<$T>(1, 40) {
                      assert_eq!(i.next_power_of_two(), next_power);
@@ -688,15 +709,15 @@ mod tests {
         ($test_name:ident, $T:ident) => (
             fn $test_name() {
                 #![test]
-                assert_eq!((0 as $T).checked_next_power_of_two(), None);
+                assert_eq!((0 as $T).checked_next_power_of_two(), Some(1));
+                assert!(($T::MAX / 2).checked_next_power_of_two().is_some());
+                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;
                 for i in range::<$T>(1, 40) {
                      assert_eq!(i.checked_next_power_of_two(), Some(next_power));
                      if i == next_power { next_power *= 2 }
                 }
-                assert!(($T::MAX / 2).checked_next_power_of_two().is_some());
-                assert_eq!(($T::MAX - 1).checked_next_power_of_two(), None);
-                assert_eq!($T::MAX.checked_next_power_of_two(), None);
             }
         )
     }