On Wednesday, August 10, 2016 21:00:01 Lodovico Giaretta via Digitalmars-d- learn wrote:
> Wow. Thanks. I didn't know the compiler would try opSlice. I will file it.

It does that so that you can use foreach with containers without having to call something on the container. The idea is that the container will implement opSlice and make it return a range over the container, and foreach will then use that range to iterate over the container.

- Jonathan M Davis

On 8/11/16 12:28 AM, Jonathan M Davis via Digitalmars-d-learn wrote:
> On Wednesday, August 10, 2016 21:00:01 Lodovico Giaretta via Digitalmars-d-
> learn wrote:
>> Wow. Thanks. I didn't know the compiler would try opSlice. I will
>> file it.
>
> It does that so that you can use foreach with containers without having to
> call something on the container. The idea is that the container will
> implement opSlice and make it return a range over the container, and foreach
> will then use that range to iterate over the container.

I get that. But it shouldn't try opSlice *first* if the item itself is a range (and it does do this). Many random-access ranges define opSlice, and most of the time range[] returns this. But in this case, it doesn't.

But it's a no-op for ranges, why waste time calling it?

-Steve

On 8/10/16 5:14 PM, ag0aep6g wrote:
> On 08/10/2016 10:54 PM, Steven Schveighoffer wrote:
>> The issue is that it tries using [] on the item to see if it defines a
>> range-like thing. Since you don't define opSlice(), it automatically
>> goes to the subrange.
>>
>> This breaks for int[] as well as Array.
>>
>> If I add opSlice to your code (and return this) it works.
>>
>> This is definitely a bug, it should try range functions before opSlice.
>> Please file.
>
> Related:
>
> https://issues.dlang.org/show_bug.cgi?id=14619

That's the same issue. Thanks. Your case is even worse, because calling save could be unnecessary and costly.

Note that the compiler will turn this:

foreach(...; foo[]) into this: foreach(...; foo[][])

ugly...

-Steve

On Thursday, August 11, 2016 08:42:27 Steven Schveighoffer via Digitalmars-d- learn wrote:
> On 8/11/16 12:28 AM, Jonathan M Davis via Digitalmars-d-learn wrote:
> > On Wednesday, August 10, 2016 21:00:01 Lodovico Giaretta via Digitalmars-d-
> >
> > learn wrote:
> >> Wow. Thanks. I didn't know the compiler would try opSlice. I will file it.
> >
> > It does that so that you can use foreach with containers without having to call something on the container. The idea is that the container will implement opSlice and make it return a range over the container, and foreach will then use that range to iterate over the container.
>
> I get that. But it shouldn't try opSlice *first* if the item itself is a range (and it does do this). Many random-access ranges define opSlice, and most of the time range[] returns this. But in this case, it doesn't.
>
> But it's a no-op for ranges, why waste time calling it?

I was just explaining what the deal with opSlice and containers was, since the OP wasn't familiar with it. I wasn't really saying anything about the suggested change. However, I have to agree that the suggested change is likely a good one. As it stands, no range should be implementing opSlice with no arguments, since it's a container function, not a range function. No range trait tests for it, and no range-based code should be using it. It's just supposed to be used on containers to get a range not on the ranges themselves. But some folks put it on ranges anyway (IIRC, there are even at least a couple of cases in Phobos where opSlice is implemented on ranges when it shouldn't be), and it causes problems. There might be a reason why it would be a bad idea to change it so that opSlice is ignored by foreach if the type is a range, but I can't think of any right now. Unfortunately, ranges and foreach have a tendency to get a bit funny thanks to the differing behavior between types (e.g. ranges that are implicitly saved when used with foreach vs those that aren't), so mucking around with it should be done with care, but it at least seems like your suggestion to skip opSlice if the range primitives are there is a good one.

- Jonathan M Davis