Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

libcore: Add range_step and range_rev functions. #4313

Closed
wants to merge 2 commits into from

5 participants

Huon Wilson Graydon Hoare Brian Anderson Ben Striegel Tim Chevalier
Huon Wilson
Owner

Per #1817

Counting-down range is named range_rev rather than rrange to distinguish it from plain range more (obviously this can change).

Graydon Hoare

r+, though could probably use a handful of tests to guard against the ever-present "whoops, got my boundary conditions slightly wrong" bugs. I can write those if you don't feel like it though.

Thanks!

Huon Wilson
Owner
Brian Anderson
Owner

@dbaupp library tests go at the bottom of the file, language tests go in the tests directory.

Huon Wilson
Owner

@graydon Added some tests, and found that I had met the "'negative' unsigned numbers act strangely" bug.

Previously, calling for uint::range_step(10, 0, -1) |_| { .. } didn't iterate at all (the expected behaviour is iterating over 10,9,...,1), because -1 > 0 for unsigned numbers and so it didn't detect that it should be iterating downwards. Fixed by breaking out the single range_step function into 2 functions iterating in a specific direction.

I'm slightly uncomfortable about having a different API for signed vs unsigned (i.e. range_step vs range_step_up & range_step_down), but I'm not sure there is a better resolution.

Huon Wilson huonw libcore: Correct behaviour of range_step for uint, tests for range* f…
…unctions.

Splits the range_step function into the two directions (up, low -> high,
and down, high -> low) for the uint types, since there is no way to have
`step < 0` for a backwards range.
aeb3cbd
Brian Anderson
Owner

@dbaupp Thanks.

@graydon What do you think of these API changes? Is it ok to have two functions here? We could also just use int for all the step values.

Graydon Hoare

Yeah, I'd just take an int for the step and have one variant.

Huon Wilson
Owner

I'm unsure if it should be int (or i64?) or the equivalently sized signed type (i.e. i8 for u8). The former leads to either inexpressible increments for u64 (or inefficiencies, for i64) on 32-bit platforms (is this a concern?) and the latter ends up cluttering up the inst modules with a T_SIGNED type for just this purpose.

(Unless there is already a technique to get the type iN from uN already?)

Ben Striegel
Collaborator

A whole new function for range_rev feels unnecessary. Can't range just count down rather than up when its first argument is greater than its second? i.e. have it count over the range [first..second) rather than [lo..hi).

Brian Anderson
Owner

I vote for adding T_SIGNED and making the step value a signed type of the equivalent size. I'm not sure about range_rev. #1817 suggested to make range also iterate backwards but notes that python's range function doesn't automatically count backwards without a step of -1.

Ben Striegel
Collaborator

Considering Python's precedent, I vote for the following functions:

int::range(lo: int, high: int);
int::range_step(start: int, end: int, step: int);
uint::range(lo: uint, high: uint);
uint::range_step(start: uint, end: uint, step: int);

Where reverse is just achieved by providing a negative step value to range_step, and the final element is always omitted.

Tim Chevalier

I'm adding the T_SIGNED type and making the simplification @brson suggested. Waiting for tests to run.

Graydon Hoare

Yeah. +1, these are convenience functions to be used casually; step will almost always be 1, 2, -1, -2 or a similarly small number. Iterating by MAXINT-ish steps is rare and probably best done manually or by a user-written loop / iterator.

Tim Chevalier

Merged at last! 982cf90

Tim Chevalier

(Oh, and thanks to @dbaupp :-)

Huon Wilson huonw deleted the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 10, 2013
  1. Huon Wilson
Commits on Jan 11, 2013
  1. Huon Wilson

    libcore: Correct behaviour of range_step for uint, tests for range* f…

    huonw authored
    …unctions.
    
    Splits the range_step function into the two directions (up, low -> high,
    and down, high -> low) for the uint types, since there is no way to have
    `step < 0` for a backwards range.
This page is out of date. Refresh to see the latest.
Showing with 162 additions and 11 deletions.
  1. +69 −6 src/libcore/int-template.rs
  2. +93 −5 src/libcore/uint-template.rs
75 src/libcore/int-template.rs
View
@@ -54,15 +54,36 @@ pub pure fn is_nonpositive(x: T) -> bool { x <= 0 as T }
pub pure fn is_nonnegative(x: T) -> bool { x >= 0 as T }
#[inline(always)]
-/// Iterate over the range [`lo`..`hi`)
-pub fn range(lo: T, hi: T, it: fn(T) -> bool) {
- let mut i = lo;
- while i < hi {
- if !it(i) { break }
- i += 1 as T;
+/// Iterate over the range [`start`,`start`+`step`..`stop`)
+pub pure fn range_step(start: T, stop: T, step: T, it: fn(T) -> bool) {
+ let mut i = start;
+ if step == 0 {
+ fail ~"range_step called with step == 0";
+ } else if step > 0 { // ascending
+ while i < stop {
+ if !it(i) { break }
+ i += step;
+ }
+ } else { // descending
+ while i > stop {
+ if !it(i) { break }
+ i += step;
+ }
}
}
+#[inline(always)]
+/// Iterate over the range [`lo`..`hi`)
+pub pure fn range(lo: T, hi: T, it: fn(T) -> bool) {
+ range_step(lo, hi, 1 as T, it);
+}
+
+#[inline(always)]
+/// Iterate over the range [`hi`..`lo`)
+pub pure fn range_rev(hi: T, lo: T, it: fn(T) -> bool) {
+ range_step(hi, lo, -1 as T, it);
+}
+
/// Computes the bitwise complement
pub pure fn compl(i: T) -> T {
-1 as T ^ i
@@ -274,3 +295,45 @@ fn test_times_negative() {
use iter::Times;
for (-10).times { log(error, ~"nope!"); }
}
+
+#[test]
+pub fn test_ranges() {
+ let mut l = ~[];
+
+ for range(0,3) |i| {
+ l.push(i);
+ }
+ for range_rev(13,10) |i| {
+ l.push(i);
+ }
+ for range_step(20,26,2) |i| {
+ l.push(i);
+ }
+ for range_step(36,30,-2) |i| {
+ l.push(i);
+ }
+ assert l == ~[0,1,2,
+ 13,12,11,
+ 20,22,24,
+ 36,34,32];
+
+ // None of the `fail`s should execute.
+ for range(10,0) |_i| {
+ fail ~"unreachable";
+ }
+ for range_rev(0,10) |_i| {
+ fail ~"unreachable";
+ }
+ for range_step(10,0,1) |_i| {
+ fail ~"unreachable";
+ }
+ for range_step(0,10,-1) |_i| {
+ fail ~"unreachable";
+ }
+}
+
+#[test]
+#[should_fail]
+fn test_range_step_zero_step() {
+ for range_step(0,10,0) |_i| {}
+}
98 src/libcore/uint-template.rs
View
@@ -52,15 +52,55 @@ pub pure fn is_nonpositive(x: T) -> bool { x <= 0 as T }
pub pure fn is_nonnegative(x: T) -> bool { x >= 0 as T }
#[inline(always)]
-/// Iterate over the range [`lo`..`hi`)
-pub pure fn range(lo: T, hi: T, it: fn(T) -> bool) {
- let mut i = lo;
- while i < hi {
+/**
+ * Iterate over the range [`start`,`start`+`step`..`stop`)
+ *
+ * Note that `uint` requires separate `range_step` functions for each
+ * direction.
+ *
+ */
+pub pure fn range_step_up(start: T, stop: T, step: T, it: fn(T) -> bool) {
+ let mut i = start;
+ if step == 0 {
+ fail ~"range_step_up called with step == 0";
+ }
+ while i < stop {
if !it(i) { break }
- i += 1 as T;
+ i += step;
}
}
+#[inline(always)]
+/**
+ * Iterate over the range [`start`,`start`-`step`..`stop`)
+ *
+ * Note that `uint` requires separate `range_step` functions for each
+ * direction.
+ *
+ */
+pub pure fn range_step_down(start: T, stop: T, step: T, it: fn(T) -> bool) {
+ let mut i = start;
+ if step == 0 {
+ fail ~"range_step_down called with step == 0";
+ }
+ while i > stop {
+ if !it(i) { break }
+ i -= step;
+ }
+}
+
+#[inline(always)]
+/// Iterate over the range [`lo`..`hi`)
+pub pure fn range(lo: T, hi: T, it: fn(T) -> bool) {
+ range_step_up(lo, hi, 1 as T, it);
+}
+
+#[inline(always)]
+/// Iterate over the range [`hi`..`lo`)
+pub pure fn range_rev(hi: T, lo: T, it: fn(T) -> bool) {
+ range_step_down(hi, lo, 1 as T, it);
+}
+
/// Computes the bitwise complement
pub pure fn compl(i: T) -> T {
max_value ^ i
@@ -292,3 +332,51 @@ pub fn test_times() {
for ten.times { accum += 1; }
assert (accum == 10);
}
+use io;
+#[test]
+pub fn test_ranges() {
+ let mut l = ~[];
+
+ for range(0,3) |i| {
+ l.push(i);
+ }
+ for range_rev(13,10) |i| {
+ l.push(i);
+ }
+ for range_step_up(20,26,2) |i| {
+ l.push(i);
+ }
+ for range_step_down(36,30,2) |i| {
+ l.push(i);
+ }
+
+ assert l == ~[0,1,2,
+ 13,12,11,
+ 20,22,24,
+ 36,34,32];
+
+ // None of the `fail`s should execute.
+ for range(0,0) |_i| {
+ fail ~"unreachable";
+ }
+ for range_rev(0,0) |_i| {
+ fail ~"unreachable";
+ }
+ for range_step_up(10,0,1) |_i| {
+ fail ~"unreachable";
+ }
+ for range_step_down(0,10,1) |_i| {
+ fail ~"unreachable";
+ }
+}
+
+#[test]
+#[should_fail]
+fn test_range_step_up_zero_step() {
+ for range_step_up(0,10,0) |_i| {}
+}
+#[test]
+#[should_fail]
+fn test_range_step_down_zero_step() {
+ for range_step_down(0,10,0) |_i| {}
+}
Something went wrong with that request. Please try again.