Skip to content

Commit 53e934c

Browse files
borsbors
committed
auto merge of #7684 : pnkfelix/rust/fsk-invert-range-rev-halfclosedness-issue5270-2ndpr, r=cmr
Changes int/uint range_rev to iterate over range `(hi,lo]` instead of `[hi,lo)`. Fix #5270. Also: * Adds unit tests for int/uint range functions * Updates the uses of `range_rev` to account for the new semantics. (Note that pretty much all of the updates there were strict improvements to the code in question; yay!) * Exposes new function, `range_step_inclusive`, which does the range `[hi,lo]`, (at least when `hi-lo` is a multiple of the `step` parameter). * Special-cases when `|step| == 1` removing unnecessary bounds-check. (I did not check whether LLVM was already performing this optimization; I figure it would be a net win to not leave that analysis to the compiler. If reviewer objects, I can easily remove that from the refactored code.) (This pull request is a rebased version of PR #7524, which went stale due to recent unrelated changes to num libraries.)
2 parents 9db1903 + 3896d46 commit 53e934c

File tree

7 files changed

+395
-45
lines changed

7 files changed

+395
-45
lines changed

‎src/libextra/smallintmap.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,8 @@ impl<V> SmallIntMap<V> {
161161
/// Visit all key-value pairs in reverse order
162162
pub fn each_reverse<'a>(&'a self, it: &fn(uint, &'a V) -> bool) -> bool {
163163
for uint::range_rev(self.v.len(), 0) |i| {
164-
match self.v[i - 1] {
165-
Some(ref elt) => if !it(i - 1, elt) { return false; },
164+
match self.v[i] {
165+
Some(ref elt) => if !it(i, elt) { return false; },
166166
None => ()
167167
}
168168
}

‎src/libstd/num/int_macros.rs‎

Lines changed: 76 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -29,28 +29,38 @@ pub static bytes : uint = ($bits / 8);
2929
pub static min_value: $T = (-1 as $T) << (bits - 1);
3030
pub static max_value: $T = min_value - 1 as $T;
3131

32+
enum Range { Closed, HalfOpen }
33+
34+
#[inline]
3235
///
33-
/// Iterate over the range [`lo`..`hi`)
36+
/// Iterate through a range with a given step value.
3437
///
35-
/// # Arguments
38+
/// Let `term` denote the closed interval `[stop-step,stop]` if `r` is Closed;
39+
/// otherwise `term` denotes the half-open interval `[stop-step,stop)`.
40+
/// Iterates through the range `[x_0, x_1, ..., x_n]` where
41+
/// `x_j == start + step*j`, and `x_n` lies in the interval `term`.
3642
///
37-
/// * `lo` - lower bound, inclusive
38-
/// * `hi` - higher bound, exclusive
39-
///
40-
/// # Examples
41-
/// ~~~
42-
/// let mut sum = 0;
43-
/// for int::range(1, 5) |i| {
44-
/// sum += i;
45-
/// }
46-
/// assert!(sum == 10);
47-
/// ~~~
43+
/// If no such nonnegative integer `n` exists, then the iteration range
44+
/// is empty.
4845
///
49-
#[inline]
50-
pub fn range_step(start: $T, stop: $T, step: $T, it: &fn($T) -> bool) -> bool {
46+
fn range_step_core(start: $T, stop: $T, step: $T, r: Range, it: &fn($T) -> bool) -> bool {
5147
let mut i = start;
5248
if step == 0 {
5349
fail!(~"range_step called with step == 0");
50+
} else if step == (1 as $T) { // elide bounds check to tighten loop
51+
while i < stop {
52+
if !it(i) { return false; }
53+
// no need for overflow check;
54+
// cannot have i + 1 > max_value because i < stop <= max_value
55+
i += (1 as $T);
56+
}
57+
} else if step == (-1 as $T) { // elide bounds check to tighten loop
58+
while i > stop {
59+
if !it(i) { return false; }
60+
// no need for underflow check;
61+
// cannot have i - 1 < min_value because i > stop >= min_value
62+
i -= (1 as $T);
63+
}
5464
} else if step > 0 { // ascending
5565
while i < stop {
5666
if !it(i) { return false; }
@@ -66,19 +76,66 @@ pub fn range_step(start: $T, stop: $T, step: $T, it: &fn($T) -> bool) -> bool {
6676
i += step;
6777
}
6878
}
69-
return true;
79+
match r {
80+
HalfOpen => return true,
81+
Closed => return (i != stop || it(i))
82+
}
83+
}
84+
85+
#[inline]
86+
///
87+
/// Iterate through the range [`start`..`stop`) with a given step value.
88+
///
89+
/// Iterates through the range `[x_0, x_1, ..., x_n]` where
90+
/// * `x_i == start + step*i`, and
91+
/// * `n` is the greatest nonnegative integer such that `x_n < stop`
92+
///
93+
/// (If no such `n` exists, then the iteration range is empty.)
94+
///
95+
/// # Arguments
96+
///
97+
/// * `start` - lower bound, inclusive
98+
/// * `stop` - higher bound, exclusive
99+
///
100+
/// # Examples
101+
/// ~~~
102+
/// let mut sum = 0;
103+
/// for int::range(1, 5) |i| {
104+
/// sum += i;
105+
/// }
106+
/// assert!(sum == 10);
107+
/// ~~~
108+
///
109+
pub fn range_step(start: $T, stop: $T, step: $T, it: &fn($T) -> bool) -> bool {
110+
range_step_core(start, stop, step, HalfOpen, it)
111+
}
112+
113+
#[inline]
114+
///
115+
/// Iterate through a range with a given step value.
116+
///
117+
/// Iterates through the range `[x_0, x_1, ..., x_n]` where
118+
/// `x_i == start + step*i` and `x_n <= last < step + x_n`.
119+
///
120+
/// (If no such nonnegative integer `n` exists, then the iteration
121+
/// range is empty.)
122+
///
123+
pub fn range_step_inclusive(start: $T, last: $T, step: $T, it: &fn($T) -> bool) -> bool {
124+
range_step_core(start, last, step, Closed, it)
70125
}
71126
127+
72128
#[inline]
73129
/// Iterate over the range [`lo`..`hi`)
74130
pub fn range(lo: $T, hi: $T, it: &fn($T) -> bool) -> bool {
75131
range_step(lo, hi, 1 as $T, it)
76132
}
77133
78134
#[inline]
79-
/// Iterate over the range [`hi`..`lo`)
135+
/// Iterate over the range (`hi`..`lo`]
80136
pub fn range_rev(hi: $T, lo: $T, it: &fn($T) -> bool) -> bool {
81-
range_step(hi, lo, -1 as $T, it)
137+
if hi == min_value { return true; }
138+
range_step_inclusive(hi-1, lo, -1 as $T, it)
82139
}
83140
84141
impl Num for $T {}
@@ -841,7 +898,7 @@ mod tests {
841898
for range(0,3) |i| {
842899
l.push(i);
843900
}
844-
for range_rev(13,10) |i| {
901+
for range_rev(14,11) |i| {
845902
l.push(i);
846903
}
847904
for range_step(20,26,2) |i| {

‎src/libstd/num/uint_macros.rs‎

Lines changed: 80 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,40 +30,99 @@ pub static bytes : uint = ($bits / 8);
3030
pub static min_value: $T = 0 as $T;
3131
pub static max_value: $T = 0 as $T - 1 as $T;
3232

33+
enum Range { Closed, HalfOpen }
34+
3335
#[inline]
34-
/**
35-
* Iterate through a range with a given step value.
36-
*
37-
* # Examples
38-
* ~~~ {.rust}
39-
* let nums = [1,2,3,4,5,6,7];
40-
*
41-
* for uint::range_step(0, nums.len() - 1, 2) |i| {
42-
* println(fmt!("%d & %d", nums[i], nums[i+1]));
43-
* }
44-
* ~~~
45-
*/
46-
pub fn range_step(start: $T, stop: $T, step: $T_SIGNED, it: &fn($T) -> bool) -> bool {
36+
///
37+
/// Iterate through a range with a given step value.
38+
///
39+
/// Let `term` denote the closed interval `[stop-step,stop]` if `r` is Closed;
40+
/// otherwise `term` denotes the half-open interval `[stop-step,stop)`.
41+
/// Iterates through the range `[x_0, x_1, ..., x_n]` where
42+
/// `x_j == start + step*j`, and `x_n` lies in the interval `term`.
43+
///
44+
/// If no such nonnegative integer `n` exists, then the iteration range
45+
/// is empty.
46+
///
47+
fn range_step_core(start: $T, stop: $T, step: $T_SIGNED, r: Range, it: &fn($T) -> bool) -> bool {
4748
let mut i = start;
4849
if step == 0 {
4950
fail!("range_step called with step == 0");
50-
}
51-
if step >= 0 {
51+
} else if step == (1 as $T_SIGNED) { // elide bounds check to tighten loop
52+
while i < stop {
53+
if !it(i) { return false; }
54+
// no need for overflow check;
55+
// cannot have i + 1 > max_value because i < stop <= max_value
56+
i += (1 as $T);
57+
}
58+
} else if step == (-1 as $T_SIGNED) { // elide bounds check to tighten loop
59+
while i > stop {
60+
if !it(i) { return false; }
61+
// no need for underflow check;
62+
// cannot have i - 1 < min_value because i > stop >= min_value
63+
i -= (1 as $T);
64+
}
65+
} else if step > 0 { // ascending
5266
while i < stop {
5367
if !it(i) { return false; }
5468
// avoiding overflow. break if i + step > max_value
5569
if i > max_value - (step as $T) { return true; }
5670
i += step as $T;
5771
}
58-
} else {
72+
} else { // descending
5973
while i > stop {
6074
if !it(i) { return false; }
6175
// avoiding underflow. break if i + step < min_value
6276
if i < min_value + ((-step) as $T) { return true; }
6377
i -= -step as $T;
6478
}
6579
}
66-
return true;
80+
match r {
81+
HalfOpen => return true,
82+
Closed => return (i != stop || it(i))
83+
}
84+
}
85+
86+
#[inline]
87+
///
88+
/// Iterate through the range [`start`..`stop`) with a given step value.
89+
///
90+
/// Iterates through the range `[x_0, x_1, ..., x_n]` where
91+
/// - `x_i == start + step*i`, and
92+
/// - `n` is the greatest nonnegative integer such that `x_n < stop`
93+
///
94+
/// (If no such `n` exists, then the iteration range is empty.)
95+
///
96+
/// # Arguments
97+
///
98+
/// * `start` - lower bound, inclusive
99+
/// * `stop` - higher bound, exclusive
100+
///
101+
/// # Examples
102+
/// ~~~ {.rust}
103+
/// let nums = [1,2,3,4,5,6,7];
104+
///
105+
/// for uint::range_step(0, nums.len() - 1, 2) |i| {
106+
/// println(fmt!("%d & %d", nums[i], nums[i+1]));
107+
/// }
108+
/// ~~~
109+
///
110+
pub fn range_step(start: $T, stop: $T, step: $T_SIGNED, it: &fn($T) -> bool) -> bool {
111+
range_step_core(start, stop, step, HalfOpen, it)
112+
}
113+
114+
#[inline]
115+
///
116+
/// Iterate through a range with a given step value.
117+
///
118+
/// Iterates through the range `[x_0, x_1, ..., x_n]` where
119+
/// `x_i == start + step*i` and `x_n <= last < step + x_n`.
120+
///
121+
/// (If no such nonnegative integer `n` exists, then the iteration
122+
/// range is empty.)
123+
///
124+
pub fn range_step_inclusive(start: $T, last: $T, step: $T_SIGNED, it: &fn($T) -> bool) -> bool {
125+
range_step_core(start, last, step, Closed, it)
67126
}
68127

69128
#[inline]
@@ -73,9 +132,10 @@ pub fn range(lo: $T, hi: $T, it: &fn($T) -> bool) -> bool {
73132
}
74133

75134
#[inline]
76-
/// Iterate over the range [`hi`..`lo`)
135+
/// Iterate over the range (`hi`..`lo`]
77136
pub fn range_rev(hi: $T, lo: $T, it: &fn($T) -> bool) -> bool {
78-
range_step(hi, lo, -1 as $T_SIGNED, it)
137+
if hi == min_value { return true; }
138+
range_step_inclusive(hi-1, lo, -1 as $T_SIGNED, it)
79139
}
80140

81141
impl Num for $T {}
@@ -603,7 +663,7 @@ mod tests {
603663
for range(0,3) |i| {
604664
l.push(i);
605665
}
606-
for range_rev(13,10) |i| {
666+
for range_rev(14,11) |i| {
607667
l.push(i);
608668
}
609669
for range_step(20,26,2) |i| {

‎src/libstd/run.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -669,7 +669,7 @@ fn spawn_process_os(prog: &str, args: &[~str],
669669
fail!("failure in dup3(err_fd, 2): %s", os::last_os_error());
670670
}
671671
// close all other fds
672-
for int::range_rev(getdtablesize() as int - 1, 2) |fd| {
672+
for int::range_rev(getdtablesize() as int, 3) |fd| {
673673
close(fd as c_int);
674674
}
675675

‎src/libstd/trie.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ impl<T> TrieNode<T> {
287287

288288
fn each_reverse<'a>(&'a self, f: &fn(&uint, &'a T) -> bool) -> bool {
289289
for uint::range_rev(self.children.len(), 0) |idx| {
290-
match self.children[idx - 1] {
290+
match self.children[idx] {
291291
Internal(ref x) => if !x.each_reverse(|i,t| f(i,t)) { return false },
292292
External(k, ref v) => if !f(&k, v) { return false },
293293
Nothing => ()
@@ -488,7 +488,7 @@ mod test_map {
488488
m.insert(x, x / 2);
489489
}
490490

491-
let mut n = uint::max_value - 9999;
491+
let mut n = uint::max_value - 10000;
492492
for m.each |k, v| {
493493
if n == uint::max_value - 5000 { break }
494494
assert!(n < uint::max_value - 5000);
@@ -525,7 +525,7 @@ mod test_map {
525525
m.insert(x, x / 2);
526526
}
527527

528-
let mut n = uint::max_value;
528+
let mut n = uint::max_value - 1;
529529
for m.each_reverse |k, v| {
530530
if n == uint::max_value - 5000 { break }
531531
assert!(n > uint::max_value - 5000);

0 commit comments

Comments
 (0)