----
import std.stdio;
class C
{
uint i;
@property bool empty() {return i == 2;}
@property uint front() {return i;}
void popFront() {++i;}
@property C save() {auto c = new C; c.i = this.i; return c;}
C opSlice() {return save;}
}
void main()
{
auto c = new C;
foreach(e; c)
{
assert(e == c.front); /* fails */
}
}
----
The assert passes without opSlice.
This makes std.range.refRange unusable with foreach:
----
import std.range: empty, refRange;
void main()
{
auto a = [1, 2, 3];
foreach(e; refRange(&a)) {}
assert(a.empty); /* fails */
}
----
This has apparently been introduced to allow foreach over containers; see issue 5605. I think slicing should only be done when the aggregate isn't foreachable itself already.
Comment #1 by issues.dlang — 2015-05-24T22:56:10Z
Well, regardless of whether opSlice should be called on a type which is already usable with foreach, it's a bug for RefRange to define opSlice with no parameters, because that's not an operation that the range API supports.
Comment #2 by schveiguy — 2016-08-11T12:49:04Z
*** Issue 16374 has been marked as a duplicate of this issue. ***
Comment #3 by schveiguy — 2017-11-16T18:32:55Z
So I tried fixing this in a simple way (simply preferring the presence of the range functions over opSlice), and the first place I see a failure is druntime's rt.util.container.array.Array: https://github.com/dlang/druntime/blob/master/src/rt/util/container/array.d#L14
This type is not copyable, and has opSlice, front, back, popBack, empty, but not popFront.
My first attempt was just to prefer front over opSlice (was surprised to see that only front is searched for), and obviously Array has this so it fails to compile (cannot be copied).
My second attempt is to require front, popFront, and empty (or back popBack and empty if foreach_reverse). There were a few tests with foreach_reverse on an Array, and this fails (cannot be copied).
So really, the foreachable range definition is that it needs front/back, popFront/popBack, and empty, AND it needs to be copyable.
The problem is that the code that determines WHICH aggregate to use (or whether opApply should be used or not), is run FIRST, and THEN the compiler tries to compile with those choices. So the Array type fails because it first picks to use the range functions in Array, but then realizes it cannot use that in foreach.
What I would expect is the following logic:
1. If opApply is present, use it.
2. If foreach with front, popFront, empty (or back, popBack, empty) compiles, then use it.
3. If the aggregate can slice, do it, and check for range primitives, don't try and slice again.
The key here in the second step (to get Array to work) is that the reduced for-line should compile.
Even this logic has issues, because there could be other reasons it doesn't compile. Part of me says the error here is in Array, and not the compiler (don't define back, popBack, and empty if you don't want it to be foreachable directly).
The current logic is:
1. If opApply is present, use it.
2. If it can slice, then do it. Assume the resulting type has range primitives.
3. If it can't slice, try range primitives.
In order to reorder the logic, I have to combine the two functions, and rearrange the original logic. Moving one simple piece of logic around was within my capabilities, but this is like a complete rewrite, and the code is VERY complex if you don't know what everything means, or the right tools to use. So I give up, but it definitely can be done.
The code that generates the pseudo for-loop:
https://github.com/dlang/dmd/blob/ba36f3a3b2f82fd1ec249476e4477a6a3457aad3/src/ddmd/opover.d#L1729
And the code that figures out whether to slice the aggregate (called very early in the above function):
https://github.com/dlang/dmd/blob/ba36f3a3b2f82fd1ec249476e4477a6a3457aad3/src/ddmd/opover.d#L1729