According to the documentation, std.range.zip "offers mutation and swapping if all ranges offer it". However, as the attached code sample shows, attempting to write to elements of a zip in a foreach loop consistently fail, even when accessed by ref and when all ranges in the zip are mutable. The sample code shows (i) foreach'ing over a lockstep of a non-mutable range and an array, writing to the array; (ii) foreach'ing over a zip of a non-mutable range and an array; (iii) foreach'ing over a zip of two arrays, writing to the second, with only the elements of the second accessed via ref; (iv) foreach'ing over a zip of two arrays, accessing the elements of both via ref. The foreach over the lockstep results in an array with correctly-written values, but the arrays that should be written to via foreach over zip remain full of nan's. This problem is also a block to the proposal in #8155 to deprecate lockstep in favour of zip, since it makes it impossible to do a blanket replace 's/lockstep/zip/' and have the resulting code work. At a minimum, zip should be corrected to ensure that its elements are mutable in line with the statement made in the docs (i.e., the 3rd and 4th cases in the sample code should result in arrays filled with correct values). Ideally, elements of a zip that come from a mutable range should themselves be mutable even if the other ranges in the zip aren't (i.e. the 2nd case in the sample code should also work).
Created attachment 1229 [details] Sample code to illustrate the problem.
Re-tagged as a major issue, 'ref' not having an effect is a big issue IMHO. Simple test-case: ----- import std.range; void main() { int[] x = [1, 1, 1]; int[] y = [1, 2, 3]; foreach (a, ref b; lockstep(x, y)) b += a; assert(y == [2, 3, 4]); // ok foreach (a, ref b; zip(x, y)) b += a; assert(y == [3, 4, 5]); // fails, it's still [2, 3, 4] } ----- Without zip supporting ref access Issue 8155 should not be considered yet.
More than just allowing Zip to be used in a "foreach ref", the real issue is the code compiling at all: Issue 11934 - Allow `ref` in `foreach` over range iff `front` returns by `ref` https://d.puremagic.com/issues/show_bug.cgi?id=11934 Issue 11934 - Allow `ref` in `foreach` over range iff `front` returns by `ref` https://d.puremagic.com/issues/show_bug.cgi?id=11935 Issue 4707 - auto ref for foreach loops https://d.puremagic.com/issues/show_bug.cgi?id=4707 As for this specific issue, I don't really see any way to fix Zip, short of putting lockstep's opApply code into Zip. As a matter of fact, why don't we just do that? Performance? I seem to remember opApply base foreach is slow (creates a delegate)?
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/phobos/issues/9987 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB