Skip to content

Commit 1c2df5c

Browse files
committed
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.
2 parents cbe9fb4 + f6328b6 commit 1c2df5c

File tree

4 files changed

+43
-37
lines changed

4 files changed

+43
-37
lines changed

src/libcollections/vec.rs

+4-14
Original file line numberDiff line numberDiff line change
@@ -710,20 +710,10 @@ impl<T> Vec<T> {
710710
#[unstable = "matches collection reform specification, waiting for dust to settle"]
711711
pub fn reserve(&mut self, additional: uint) {
712712
if self.cap - self.len < additional {
713-
match self.len.checked_add(additional) {
714-
None => panic!("Vec::reserve: `uint` overflow"),
715-
// if the checked_add
716-
Some(new_cap) => {
717-
let amort_cap = new_cap.next_power_of_two();
718-
// next_power_of_two will overflow to exactly 0 for really big capacities
719-
let cap = if amort_cap == 0 {
720-
new_cap
721-
} else {
722-
amort_cap
723-
};
724-
self.grow_capacity(cap)
725-
}
726-
}
713+
let err_msg = "Vec::reserve: `uint` overflow";
714+
let new_cap = self.len.checked_add(additional).expect(err_msg)
715+
.checked_next_power_of_two().expect(err_msg);
716+
self.grow_capacity(new_cap);
727717
}
728718
}
729719

src/libcore/num/mod.rs

+11-16
Original file line numberDiff line numberDiff line change
@@ -673,35 +673,30 @@ signed_int_impl! { int }
673673
#[unstable = "recently settled as part of numerics reform"]
674674
pub trait UnsignedInt: Int {
675675
/// Returns `true` iff `self == 2^k` for some `k`.
676+
#[inline]
676677
fn is_power_of_two(self) -> bool {
677-
(self - Int::one()) & self == Int::zero()
678+
(self - Int::one()) & self == Int::zero() && !(self == Int::zero())
678679
}
679680

680681
/// Returns the smallest power of two greater than or equal to `self`.
682+
/// Unspecified behavior on overflow.
681683
#[inline]
682684
fn next_power_of_two(self) -> Self {
683-
let halfbits = size_of::<Self>() * 4;
684-
let mut tmp = self - Int::one();
685-
let mut shift = 1u;
686-
while shift <= halfbits {
687-
tmp = tmp | (tmp >> shift);
688-
shift = shift << 1u;
689-
}
690-
tmp + Int::one()
685+
let bits = size_of::<Self>() * 8;
686+
let one: Self = Int::one();
687+
one << ((bits - (self - one).leading_zeros()) % bits)
691688
}
692689

693690
/// Returns the smallest power of two greater than or equal to `n`. If the
694691
/// next power of two is greater than the type's maximum value, `None` is
695692
/// returned, otherwise the power of two is wrapped in `Some`.
696693
fn checked_next_power_of_two(self) -> Option<Self> {
697-
let halfbits = size_of::<Self>() * 4;
698-
let mut tmp = self - Int::one();
699-
let mut shift = 1u;
700-
while shift <= halfbits {
701-
tmp = tmp | (tmp >> shift);
702-
shift = shift << 1u;
694+
let npot = self.next_power_of_two();
695+
if npot >= self {
696+
Some(npot)
697+
} else {
698+
None
703699
}
704-
tmp.checked_add(Int::one())
705700
}
706701
}
707702

src/libstd/collections/hash/map.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -623,10 +623,10 @@ impl<K: Eq + Hash<S>, V, S, H: Hasher<S>> HashMap<K, V, H> {
623623
/// Resizes the internal vectors to a new capacity. It's your responsibility to:
624624
/// 1) Make sure the new capacity is enough for all the elements, accounting
625625
/// for the load factor.
626-
/// 2) Ensure new_capacity is a power of two.
626+
/// 2) Ensure new_capacity is a power of two or zero.
627627
fn resize(&mut self, new_capacity: uint) {
628628
assert!(self.table.size() <= new_capacity);
629-
assert!(new_capacity.is_power_of_two());
629+
assert!(new_capacity.is_power_of_two() || new_capacity == 0);
630630

631631
let mut old_table = replace(&mut self.table, RawTable::new(new_capacity));
632632
let old_size = old_table.size();

src/libstd/num/mod.rs

+26-5
Original file line numberDiff line numberDiff line change
@@ -664,11 +664,32 @@ mod tests {
664664
assert_eq!(third.checked_mul(4), None);
665665
}
666666

667+
macro_rules! test_is_power_of_two {
668+
($test_name:ident, $T:ident) => (
669+
fn $test_name() {
670+
#![test]
671+
assert_eq!((0 as $T).is_power_of_two(), false);
672+
assert_eq!((1 as $T).is_power_of_two(), true);
673+
assert_eq!((2 as $T).is_power_of_two(), true);
674+
assert_eq!((3 as $T).is_power_of_two(), false);
675+
assert_eq!((4 as $T).is_power_of_two(), true);
676+
assert_eq!((5 as $T).is_power_of_two(), false);
677+
assert!(($T::MAX / 2 + 1).is_power_of_two(), true);
678+
}
679+
)
680+
}
681+
682+
test_is_power_of_two!{ test_is_power_of_two_u8, u8 }
683+
test_is_power_of_two!{ test_is_power_of_two_u16, u16 }
684+
test_is_power_of_two!{ test_is_power_of_two_u32, u32 }
685+
test_is_power_of_two!{ test_is_power_of_two_u64, u64 }
686+
test_is_power_of_two!{ test_is_power_of_two_uint, uint }
687+
667688
macro_rules! test_next_power_of_two {
668689
($test_name:ident, $T:ident) => (
669690
fn $test_name() {
670691
#![test]
671-
assert_eq!((0 as $T).next_power_of_two(), 0);
692+
assert_eq!((0 as $T).next_power_of_two(), 1);
672693
let mut next_power = 1;
673694
for i in range::<$T>(1, 40) {
674695
assert_eq!(i.next_power_of_two(), next_power);
@@ -688,15 +709,15 @@ mod tests {
688709
($test_name:ident, $T:ident) => (
689710
fn $test_name() {
690711
#![test]
691-
assert_eq!((0 as $T).checked_next_power_of_two(), None);
712+
assert_eq!((0 as $T).checked_next_power_of_two(), Some(1));
713+
assert!(($T::MAX / 2).checked_next_power_of_two().is_some());
714+
assert_eq!(($T::MAX - 1).checked_next_power_of_two(), None);
715+
assert_eq!($T::MAX.checked_next_power_of_two(), None);
692716
let mut next_power = 1;
693717
for i in range::<$T>(1, 40) {
694718
assert_eq!(i.checked_next_power_of_two(), Some(next_power));
695719
if i == next_power { next_power *= 2 }
696720
}
697-
assert!(($T::MAX / 2).checked_next_power_of_two().is_some());
698-
assert_eq!(($T::MAX - 1).checked_next_power_of_two(), None);
699-
assert_eq!($T::MAX.checked_next_power_of_two(), None);
700721
}
701722
)
702723
}

0 commit comments

Comments
 (0)