https://issues.dlang.org/show_bug.cgi?id=14619

Jonathan M Davis <issues.dlang@jmdavisProg.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |issues.dlang@jmdavisProg.co
                   |                            |m

--- Comment #1 from Jonathan M Davis <issues.dlang@jmdavisProg.com> ---
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.

--

https://issues.dlang.org/show_bug.cgi?id=14619

Lodovico Giaretta <lodovico@giaretart.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://issues.dlang.org/sh
                   |                            |ow_bug.cgi?id=16374

--

https://issues.dlang.org/show_bug.cgi?id=14619

Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |lodovico@giaretart.net

--- Comment #2 from Steven Schveighoffer <schveiguy@yahoo.com> ---
*** Issue 16374 has been marked as a duplicate of this issue. ***

--

https://issues.dlang.org/show_bug.cgi?id=14619

Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy@yahoo.com

--- Comment #3 from Steven Schveighoffer <schveiguy@yahoo.com> ---
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

--

https://issues.dlang.org/show_bug.cgi?id=14619

ag0aep6g <ag0aep6g@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://issues.dlang.org/sh
                   |                            |ow_bug.cgi?id=18807

--

https://issues.dlang.org/show_bug.cgi?id=14619

Dennis <dkorpel@live.nl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dkorpel@live.nl

--- Comment #4 from Dennis <dkorpel@live.nl> ---
I was surprised today that opSlice() / opIndex() even worked at all to make a
struct foreach-able. I looked at the spec, and couldn't find anything:

https://dlang.org/spec/statement.html#foreach-statement https://dlang.org/spec/operatoroverloading.html#slice

So aside from the behavior, the documentation should also be updated.

Btw, the responsible code is currently here: https://github.com/dlang/dmd/blob/4522e0236f2abde017c3ec1192f76c6cf661b7c2/src/dmd/opover.d#L1521

--

https://issues.dlang.org/show_bug.cgi?id=14619

kdevel <kdevel@vogtner.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kdevel@vogtner.de

--

https://issues.dlang.org/show_bug.cgi?id=14619

Iain Buclaw <ibuclaw@gdcproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P1                          |P3

--