From e21c9a5dc0bd49aa05ad4535efa4c8e39eaa5dee Mon Sep 17 00:00:00 2001 From: "H. S. Teoh" Date: Mon, 20 Jul 2015 11:32:18 -0700 Subject: [PATCH 1/2] Improve Lockstep docs. Remove stray // from generated docs. Reorder that example to flow better with the preceding prose. Fix coding style in code examples. --- std/range/package.d | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/std/range/package.d b/std/range/package.d index b641de2ec30..cf08580d8bf 100644 --- a/std/range/package.d +++ b/std/range/package.d @@ -3968,17 +3968,17 @@ private string lockstepMixin(Ranges...)(bool withIndex) By default $(D StoppingPolicy) is set to $(D StoppingPolicy.shortest). - BUGS: If a range does not offer lvalue access, but $(D ref) is used in the - $(D foreach) loop, it will be silently accepted but any modifications - to the variable will not be propagated to the underlying range. - - // Lockstep also supports iterating with an index variable: - Example: + Lockstep also supports iterating with an index variable: ------- - foreach(index, a, b; lockstep(arr1, arr2)) { + foreach (index, a, b; lockstep(arr1, arr2)) + { writefln("Index %s: a = %s, b = %s", index, a, b); } ------- + + BUGS: If a range does not offer lvalue access, but $(D ref) is used in the + $(D foreach) loop, it will be silently accepted but any modifications + to the variable will not be propagated to the underlying range. */ struct Lockstep(Ranges...) if (Ranges.length > 1 && allSatisfy!(isInputRange, Ranges)) @@ -4031,7 +4031,7 @@ unittest auto arr1 = [1,2,3,4,5]; auto arr2 = [6,7,8,9,10]; - foreach(ref a, ref b; lockstep(arr1, arr2)) + foreach (ref a, ref b; lockstep(arr1, arr2)) { a += b; } From 90ad7e5fe9d7aa30ecdc4ab19ba0405b7d565ff8 Mon Sep 17 00:00:00 2001 From: "H. S. Teoh" Date: Tue, 21 Jul 2015 10:03:24 -0700 Subject: [PATCH 2/2] Remove bug section from docs: bug has been fixed in compiler. Add unittest to prevent regressions in the future. --- std/range/package.d | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/std/range/package.d b/std/range/package.d index cf08580d8bf..1cf94a41ea6 100644 --- a/std/range/package.d +++ b/std/range/package.d @@ -3975,10 +3975,6 @@ private string lockstepMixin(Ranges...)(bool withIndex) writefln("Index %s: a = %s, b = %s", index, a, b); } ------- - - BUGS: If a range does not offer lvalue access, but $(D ref) is used in the - $(D foreach) loop, it will be silently accepted but any modifications - to the variable will not be propagated to the underlying range. */ struct Lockstep(Ranges...) if (Ranges.length > 1 && allSatisfy!(isInputRange, Ranges)) @@ -4125,6 +4121,37 @@ unittest foreach (x, y; lockstep(iota(0, 10), iota(0, 10))) { } } +unittest +{ + struct RvalueRange + { + int[] impl; + @property bool empty() { return impl.empty; } + @property int front() { return impl[0]; } // N.B. non-ref + void popFront() { impl.popFront(); } + } + auto data1 = [ 1, 2, 3, 4 ]; + auto data2 = [ 5, 6, 7, 8 ]; + auto r1 = RvalueRange(data1); + auto r2 = data2; + foreach (a, ref b; lockstep(r1, r2)) + { + a++; + b++; + } + assert(data1 == [ 1, 2, 3, 4 ]); // changes to a do not propagate to data + assert(data2 == [ 6, 7, 8, 9 ]); // but changes to b do. + + // Since r1 is by-value only, the compiler should reject attempts to + // foreach over it with ref. + static assert(!__traits(compiles, { + foreach (ref a, ref b; lockstep(r1, r2)) { a++; } + })); + static assert(__traits(compiles, { + foreach (a, ref b; lockstep(r1, r2)) { a++; } + })); +} + /** Creates a mathematical sequence given the initial values and a recurrence function that computes the next value from the existing