Skip to content

Commit 3deb97f

Browse files
cuviperGankra
cuviper
authored and
Gankra
committed
bitv: Fix all() for nbits that are multiples of u32::BITS
The old logic would be ok with *either* 0 or all 1s in the last word, because it didn't compute a proper mask for the case where nbits is an exact multiple of u32::BITS. Add mask_for_bits() to compute this properly, and use it in all(). Add all/none assertions to most of the tests. Note in particular, the all-zero bitv in test_32_elements() was incorrectly all()==true before this patch.
1 parent 8f194de commit 3deb97f

File tree

1 file changed

+43
-8
lines changed

1 file changed

+43
-8
lines changed

‎src/libcollections/bit.rs

+43-8
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,12 @@ fn blocks_for_bits(bits: uint) -> uint {
174174

175175
}
176176

177+
/// Computes the bitmask for the final word of the vector
178+
fn mask_for_bits(bits: uint) -> u32 {
179+
// Note especially that a perfect multiple of u32::BITS should mask all 1s.
180+
!0u32 >> (u32::BITS - bits % u32::BITS) % u32::BITS
181+
}
182+
177183
impl Bitv {
178184
/// Applies the given operation to the blocks of self and other, and sets self to
179185
/// be the result.
@@ -526,7 +532,7 @@ impl Bitv {
526532
last_word = elem;
527533
tmp == !0u32
528534
// and then check the last one has enough ones
529-
}) && (last_word == ((1 << self.nbits % u32::BITS) - 1) || last_word == !0u32)
535+
}) && (last_word == mask_for_bits(self.nbits))
530536
}
531537

532538
/// Returns an iterator over the elements of the vector in order.
@@ -788,15 +794,15 @@ impl Bitv {
788794
let new_nblocks = blocks_for_bits(new_nbits);
789795
let full_value = if value { !0 } else { 0 };
790796

791-
// Correct the old tail word
797+
// Correct the old tail word, setting or clearing formerly unused bits
792798
let old_last_word = blocks_for_bits(self.nbits) - 1;
793799
if self.nbits % u32::BITS > 0 {
794-
let overhang = self.nbits % u32::BITS; // # of already-used bits
795-
let mask = !((1 << overhang) - 1); // e.g. 5 unused bits => 111110..0
800+
let mask = mask_for_bits(self.nbits);
796801
if value {
797-
self.storage[old_last_word] |= mask;
802+
self.storage[old_last_word] |= !mask;
798803
} else {
799-
self.storage[old_last_word] &= !mask;
804+
// Extra bits are already supposed to be zero by invariant, but play it safe...
805+
self.storage[old_last_word] &= mask;
800806
}
801807
}
802808

@@ -1808,14 +1814,17 @@ mod tests {
18081814
let act = Bitv::new();
18091815
let exp = Vec::from_elem(0u, false);
18101816
assert!(act.eq_vec(exp.as_slice()));
1817+
assert!(act.none() && act.all());
18111818
}
18121819

18131820
#[test]
18141821
fn test_1_element() {
18151822
let mut act = Bitv::from_elem(1u, false);
18161823
assert!(act.eq_vec(&[false]));
1824+
assert!(act.none() && !act.all());
18171825
act = Bitv::from_elem(1u, true);
18181826
assert!(act.eq_vec(&[true]));
1827+
assert!(!act.none() && act.all());
18191828
}
18201829

18211830
#[test]
@@ -1824,6 +1833,7 @@ mod tests {
18241833
b.set(0, true);
18251834
b.set(1, false);
18261835
assert_eq!(b.to_string(), "10");
1836+
assert!(!b.none() && !b.all());
18271837
}
18281838

18291839
#[test]
@@ -1834,10 +1844,12 @@ mod tests {
18341844
act = Bitv::from_elem(10u, false);
18351845
assert!((act.eq_vec(
18361846
&[false, false, false, false, false, false, false, false, false, false])));
1847+
assert!(act.none() && !act.all());
18371848
// all 1
18381849

18391850
act = Bitv::from_elem(10u, true);
18401851
assert!((act.eq_vec(&[true, true, true, true, true, true, true, true, true, true])));
1852+
assert!(!act.none() && act.all());
18411853
// mixed
18421854

18431855
act = Bitv::from_elem(10u, false);
@@ -1847,6 +1859,7 @@ mod tests {
18471859
act.set(3u, true);
18481860
act.set(4u, true);
18491861
assert!((act.eq_vec(&[true, true, true, true, true, false, false, false, false, false])));
1862+
assert!(!act.none() && !act.all());
18501863
// mixed
18511864

18521865
act = Bitv::from_elem(10u, false);
@@ -1856,6 +1869,7 @@ mod tests {
18561869
act.set(8u, true);
18571870
act.set(9u, true);
18581871
assert!((act.eq_vec(&[false, false, false, false, false, true, true, true, true, true])));
1872+
assert!(!act.none() && !act.all());
18591873
// mixed
18601874

18611875
act = Bitv::from_elem(10u, false);
@@ -1864,6 +1878,7 @@ mod tests {
18641878
act.set(6u, true);
18651879
act.set(9u, true);
18661880
assert!((act.eq_vec(&[true, false, false, true, false, false, true, false, false, true])));
1881+
assert!(!act.none() && !act.all());
18671882
}
18681883

18691884
#[test]
@@ -1876,13 +1891,15 @@ mod tests {
18761891
&[false, false, false, false, false, false, false, false, false, false, false,
18771892
false, false, false, false, false, false, false, false, false, false, false,
18781893
false, false, false, false, false, false, false, false, false]));
1894+
assert!(act.none() && !act.all());
18791895
// all 1
18801896

18811897
act = Bitv::from_elem(31u, true);
18821898
assert!(act.eq_vec(
18831899
&[true, true, true, true, true, true, true, true, true, true, true, true, true,
18841900
true, true, true, true, true, true, true, true, true, true, true, true, true,
18851901
true, true, true, true, true]));
1902+
assert!(!act.none() && act.all());
18861903
// mixed
18871904

18881905
act = Bitv::from_elem(31u, false);
@@ -1898,6 +1915,7 @@ mod tests {
18981915
&[true, true, true, true, true, true, true, true, false, false, false, false, false,
18991916
false, false, false, false, false, false, false, false, false, false, false,
19001917
false, false, false, false, false, false, false]));
1918+
assert!(!act.none() && !act.all());
19011919
// mixed
19021920

19031921
act = Bitv::from_elem(31u, false);
@@ -1913,6 +1931,7 @@ mod tests {
19131931
&[false, false, false, false, false, false, false, false, false, false, false,
19141932
false, false, false, false, false, true, true, true, true, true, true, true, true,
19151933
false, false, false, false, false, false, false]));
1934+
assert!(!act.none() && !act.all());
19161935
// mixed
19171936

19181937
act = Bitv::from_elem(31u, false);
@@ -1927,6 +1946,7 @@ mod tests {
19271946
&[false, false, false, false, false, false, false, false, false, false, false,
19281947
false, false, false, false, false, false, false, false, false, false, false,
19291948
false, false, true, true, true, true, true, true, true]));
1949+
assert!(!act.none() && !act.all());
19301950
// mixed
19311951

19321952
act = Bitv::from_elem(31u, false);
@@ -1937,6 +1957,7 @@ mod tests {
19371957
&[false, false, false, true, false, false, false, false, false, false, false, false,
19381958
false, false, false, false, false, true, false, false, false, false, false, false,
19391959
false, false, false, false, false, false, true]));
1960+
assert!(!act.none() && !act.all());
19401961
}
19411962

19421963
#[test]
@@ -1949,13 +1970,15 @@ mod tests {
19491970
&[false, false, false, false, false, false, false, false, false, false, false,
19501971
false, false, false, false, false, false, false, false, false, false, false,
19511972
false, false, false, false, false, false, false, false, false, false]));
1973+
assert!(act.none() && !act.all());
19521974
// all 1
19531975

19541976
act = Bitv::from_elem(32u, true);
19551977
assert!(act.eq_vec(
19561978
&[true, true, true, true, true, true, true, true, true, true, true, true, true,
19571979
true, true, true, true, true, true, true, true, true, true, true, true, true,
19581980
true, true, true, true, true, true]));
1981+
assert!(!act.none() && act.all());
19591982
// mixed
19601983

19611984
act = Bitv::from_elem(32u, false);
@@ -1971,6 +1994,7 @@ mod tests {
19711994
&[true, true, true, true, true, true, true, true, false, false, false, false, false,
19721995
false, false, false, false, false, false, false, false, false, false, false,
19731996
false, false, false, false, false, false, false, false]));
1997+
assert!(!act.none() && !act.all());
19741998
// mixed
19751999

19762000
act = Bitv::from_elem(32u, false);
@@ -1986,6 +2010,7 @@ mod tests {
19862010
&[false, false, false, false, false, false, false, false, false, false, false,
19872011
false, false, false, false, false, true, true, true, true, true, true, true, true,
19882012
false, false, false, false, false, false, false, false]));
2013+
assert!(!act.none() && !act.all());
19892014
// mixed
19902015

19912016
act = Bitv::from_elem(32u, false);
@@ -2001,6 +2026,7 @@ mod tests {
20012026
&[false, false, false, false, false, false, false, false, false, false, false,
20022027
false, false, false, false, false, false, false, false, false, false, false,
20032028
false, false, true, true, true, true, true, true, true, true]));
2029+
assert!(!act.none() && !act.all());
20042030
// mixed
20052031

20062032
act = Bitv::from_elem(32u, false);
@@ -2012,6 +2038,7 @@ mod tests {
20122038
&[false, false, false, true, false, false, false, false, false, false, false, false,
20132039
false, false, false, false, false, true, false, false, false, false, false, false,
20142040
false, false, false, false, false, false, true, true]));
2041+
assert!(!act.none() && !act.all());
20152042
}
20162043

20172044
#[test]
@@ -2024,13 +2051,15 @@ mod tests {
20242051
&[false, false, false, false, false, false, false, false, false, false, false,
20252052
false, false, false, false, false, false, false, false, false, false, false,
20262053
false, false, false, false, false, false, false, false, false, false, false]));
2054+
assert!(act.none() && !act.all());
20272055
// all 1
20282056

20292057
act = Bitv::from_elem(33u, true);
20302058
assert!(act.eq_vec(
20312059
&[true, true, true, true, true, true, true, true, true, true, true, true, true,
20322060
true, true, true, true, true, true, true, true, true, true, true, true, true,
20332061
true, true, true, true, true, true, true]));
2062+
assert!(!act.none() && act.all());
20342063
// mixed
20352064

20362065
act = Bitv::from_elem(33u, false);
@@ -2046,6 +2075,7 @@ mod tests {
20462075
&[true, true, true, true, true, true, true, true, false, false, false, false, false,
20472076
false, false, false, false, false, false, false, false, false, false, false,
20482077
false, false, false, false, false, false, false, false, false]));
2078+
assert!(!act.none() && !act.all());
20492079
// mixed
20502080

20512081
act = Bitv::from_elem(33u, false);
@@ -2061,6 +2091,7 @@ mod tests {
20612091
&[false, false, false, false, false, false, false, false, false, false, false,
20622092
false, false, false, false, false, true, true, true, true, true, true, true, true,
20632093
false, false, false, false, false, false, false, false, false]));
2094+
assert!(!act.none() && !act.all());
20642095
// mixed
20652096

20662097
act = Bitv::from_elem(33u, false);
@@ -2076,6 +2107,7 @@ mod tests {
20762107
&[false, false, false, false, false, false, false, false, false, false, false,
20772108
false, false, false, false, false, false, false, false, false, false, false,
20782109
false, false, true, true, true, true, true, true, true, true, false]));
2110+
assert!(!act.none() && !act.all());
20792111
// mixed
20802112

20812113
act = Bitv::from_elem(33u, false);
@@ -2088,6 +2120,7 @@ mod tests {
20882120
&[false, false, false, true, false, false, false, false, false, false, false, false,
20892121
false, false, false, false, false, true, false, false, false, false, false, false,
20902122
false, false, false, false, false, false, true, true, true]));
2123+
assert!(!act.none() && !act.all());
20912124
}
20922125

20932126
#[test]
@@ -2234,15 +2267,17 @@ mod tests {
22342267
#[test]
22352268
fn test_small_clear() {
22362269
let mut b = Bitv::from_elem(14, true);
2270+
assert!(!b.none() && b.all());
22372271
b.clear();
2238-
assert!(b.none());
2272+
assert!(b.none() && !b.all());
22392273
}
22402274

22412275
#[test]
22422276
fn test_big_clear() {
22432277
let mut b = Bitv::from_elem(140, true);
2278+
assert!(!b.none() && b.all());
22442279
b.clear();
2245-
assert!(b.none());
2280+
assert!(b.none() && !b.all());
22462281
}
22472282

22482283
#[test]

0 commit comments

Comments
 (0)