Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

better support of foreach for array #683

Closed
wants to merge 3 commits into from

2 participants

@monarchdodra
Collaborator

See discussion "foreach ref very broken: fails to call front(val)" here:
http://forum.dlang.org/thread/uoptrrkgfczyozxtkkvi@forum.dlang.org

This fix allows writting things like:
Array!int arr;
foreach(ref a; arr)
++a;

In the sense that:
1) arr actually gets modified as intended by ref.
2) "++a" compiles correctly, even though "++front" doesn't allow it.

I think the pull request is complete:
I implemented both opApply and opApplyReverse.
I implemented it both for Array.Range and Array (see: http://forum.dlang.org/thread/rxpbtrawpjzvdfuuwmwp@forum.dlang.org)
I implemented correct behavior for "break" cases.
I documented the public opApply.
I wrote a complete unit test.
I tested && unittested it.

This is my first time doing this, so please forgive me if I did something wrong :(

monarchdodra added some commits
@monarchdodra monarchdodra Giving Array opApply, so as to properly work with foreach(_reverse) w…
…hen the elements are accessed by reference.
ec4cb55
@monarchdodra monarchdodra Adding an enforce to make sure _b is still smaller than _outer.length.
Correcting indentation (4 spaces instead of two.
Adding a few words and a period to the doc.
ce85861
@monarchdodra monarchdodra checking _payload is isInitialized before accessing it. Not an enforc…
…ement, as empty Array s can be legitimately not isInitialized.


Ranges don't need the check, as they iterate on indexes.
c91d302
@andralex
Owner

We need to improve ranges instead of using opApply for such trivial iteration patterns. Apologies that the thread didn't make this clear. Will close this now - sorry.

@andralex andralex closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 11, 2012
  1. @monarchdodra

    Giving Array opApply, so as to properly work with foreach(_reverse) w…

    monarchdodra authored
    …hen the elements are accessed by reference.
Commits on Jul 12, 2012
  1. @monarchdodra

    Adding an enforce to make sure _b is still smaller than _outer.length.

    monarchdodra authored
    Correcting indentation (4 spaces instead of two.
    Adding a few words and a period to the doc.
Commits on Jul 13, 2012
  1. @monarchdodra

    checking _payload is isInitialized before accessing it. Not an enforc…

    monarchdodra authored
    …ement, as empty Array s can be legitimately not isInitialized.
    
    
    Ranges don't need the check, as they iterate on indexes.
This page is out of date. Refresh to see the latest.
Showing with 76 additions and 0 deletions.
  1. +76 −0 std/container.d
View
76 std/container.d
@@ -2354,6 +2354,28 @@ Defines the container's primary range, which is a random-access range.
mixin("_outer._payload.ptr[_a + i] "~op~"= value;");
}
+ int opApply(int delegate(ref T) dg)
+ {
+ enforce(_b <= _outer.length);
+ foreach(i; _a .. _b)
+ {
+ int result = dg(_outer._data._payload[i]);
+ if(result) return result;
+ }
+ return 0;
+ }
+
+ int opApplyReverse(int delegate(ref T) dg)
+ {
+ enforce(_b <= _outer.length);
+ foreach_reverse(i; _a .. _b)
+ {
+ int result = dg(_outer._data._payload[i]);
+ if(result) return result;
+ }
+ return 0;
+ }
+
@property size_t length() const {
return _b - _a;
}
@@ -2462,6 +2484,44 @@ Complexity: $(BIGOH 1)
}
/**
+Iterates over over the elements of the container, applying delegate dg
+to each element in forward order.
+
+Complexity: $(BIGOH n)
+ */
+ int opApply(int delegate(ref T) dg)
+ {
+ if(_data.RefCounted.isInitialized())
+ {
+ foreach(ref v; _data._payload)
+ {
+ int result = dg(v);
+ if(result) return result;
+ }
+ }
+ return 0;
+ }
+
+/**
+Iterates over over the elements of the container, applying delegate dg
+to each element in reverse order.
+
+Complexity: $(BIGOH n)
+ */
+ int opApplyReverse(int delegate(ref T) dg)
+ {
+ if(_data.RefCounted.isInitialized())
+ {
+ foreach_reverse(ref v; _data._payload)
+ {
+ int result = dg(v);
+ if(result) return result;
+ }
+ }
+ return 0;
+ }
+
+/**
@@@BUG@@@ This doesn't work yet
*/
size_t opDollar() const
@@ -3047,6 +3107,22 @@ unittest
assertThrown(a.replace(r, [42]));
assertThrown(a.linearRemove(r));
}
+// Tests that foreach(_reverse) with(out) ref works as intended
+unittest
+{
+ auto a = Array!int(1, 2, 3, 4);
+ foreach(v; a) ++v; //Foreach without ref
+ assert(equal(a[], [1, 2, 3, 4]));
+ foreach(ref v; a) ++v; //Foreach ref on container
+ assert(equal(a[], [2, 3, 4, 5]));
+ foreach(ref v; a[]) v = 0; //Foreach ref on range
+ assert(equal(a[], [0, 0, 0, 0]));
+ int i = 0;
+ foreach_reverse(ref v; a[]) {v += ++i;} //Foreach_ ref with reverse
+ assert(equal(a[], [4, 3, 2, 1]));
+ foreach_reverse(ref v; a[]) {if(v == 3) break; v = 0;} //foreach with break
+ assert(equal(a[], [4, 3, 0, 0]));
+}
// BinaryHeap
/**
Something went wrong with that request. Please try again.