https://issues.dlang.org/show_bug.cgi?id=14619
Steven Schveighoffer <schvei...@yahoo.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |schvei...@yahoo.com
--- Comment #3 from Steven Schveighoffer <schvei...@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
--
