If you create a class/struct that can give you a (forward) range via "opSlice()", and that range gives you access to "opApply", then you get two different behaviors:

----
MyClass arr = ...;

foreach(a; arr)
   ...

foreach(a; arr[])
   ...

----
In the first case, foreach will call opSlice(), and then walk through the resulting arr[] range with the front/popFront/empty troika.

In the second case, foreach will call opApply on the range "arr[]".

----
I'm wondering if this is the correct behavior? In particular, since foreach guarantees a call to opSlice(), so writing "arr[]" *should* be redundant, yet the final behavior is different.

That said, the "issue" *could* be fixed if the base class defines opApply as: "return opSlice().opApply(dg)" (or more complex). However:
a) The implementer of class has no obligation to do this, since he has provided a perfectly valid range.
b) This would force implementers into more generic useless boilerplate code.

What are your thoughts? Which is the "correct" solution? Is it a bug with foreach, or should the base struct/class provide an opApply?

PS: Related: "foreach over range (with opApply) should save range."
http://d.puremagic.com/issues/show_bug.cgi?id=4347
On this assigned issue, is the conclusion that foreach will eventually save the range?

"monarch_dodra" , dans le message (digitalmars.D:171868), a écrit :
> I'm wondering if this is the correct behavior? In particular, since foreach guarantees a call to opSlice(), so writing "arr[]" *should* be redundant, yet the final behavior is different.
> 
> That said, the "issue" *could* be fixed if the base class defines
> opApply as: "return opSlice().opApply(dg)" (or more complex).
> However:
> a) The implementer of class has no obligation to do this, since
> he has provided a perfectly valid range.
> b) This would force implementers into more generic useless
> boilerplate code.
> 
> What are your thoughts? Which is the "correct" solution? Is it a bug with foreach, or should the base struct/class provide an opApply?

I think foreach should never call opSlice. That's not in the online documentation (http://dlang.org/statement.html#ForeachStatement), unless I missed something. If you want to use foreach on a class with an opSlice, then yes, you should define opApply. Otherwise, the user have to call opSlice himself, which seems reasonable. That's how I understand the doc.

-- 
Christophe

On Wednesday, 11 July 2012 at 14:10:35 UTC, travert@phare.normalesup.org (Christophe Travert) wrote:
> I think foreach should never call opSlice. That's not in the online
> documentation (http://dlang.org/statement.html#ForeachStatement), unless
> I missed something. If you want to use foreach on a class with an
> opSlice, then yes, you should define opApply. Otherwise, the user have
> to call opSlice himself, which seems reasonable. That's how I understand
> the doc.

Hum... One thing is for sure, it _does_ call opSlice when it is defined.

I just re-read the docs you linked to, and if that was my only source, I'd reach the same conclusion as you. however, my "The D Programming Language", states:
*12: Operator Overloading
**9: Overloading foreach
***1: foreach with Iteration Primitives
"Last but not least, if the iterated object offers the slice operator with no arguments lst[], __c is initialized with lst[] instead of lst. This is in order to allow “extracting” the iteration means out of a container without requiring the container to define the three iteration primitives."

Another thing I find strange about the doc is: "If the foreach range properties do not exist, the opApply method will be used instead." This sounds backwards to me.

I think the online documentation (http://dlang.org/statement.html#ForeachStatement) is not sufficient.

foreach (e; aggr) { ...body...}

Current dmd translates above foreach statement like follows.

1. If aggr has opApply or opApplyReverse, it's used.

2. If aggr has empty/front/popFront:
 2a. If aggr has slice operation, it's translated to:

  for (auto __r = aggr[];  // If aggr is a container (e.g. std.container.Array),
                           // foreach will get its range object for
the iteration.
      !__r.empty;
      __r.popFront()) { auto e = __r.front; ...body... }

 2b. If aggr doesn't have slice operation, it's translated to:

  for (auto __r = aggr;  // If aggr is copyable, saves the original range.
      !__r.empty;
      __r.popFront()) { auto e = __r.front; ...body... }

3. If aggr is static or dynamic array, it's translated to:

  for (auto __tmp = aggr[], __key = 0;  // If aggr is static array,
get its slice for iteration.
      !__key < __tmp.length;
      ++__key) { auto e = __tmp[__key]; ...body... }

These come from the dmd source code. https://github.com/D-Programming-Language/dmd/blob/master/src/opover.c#L1226 https://github.com/D-Programming-Language/dmd/blob/master/src/statement.c#L1522

Bye

Kenji Hara

2012/7/11 monarch_dodra <monarch_dodra@gmail.com>:
> If you create a class/struct that can give you a (forward) range via
> "opSlice()", and that range gives you access to "opApply", then you get two
> different behaviors:
>
> ----
> MyClass arr = ...;
>
> foreach(a; arr)
>    ...
>
> foreach(a; arr[])
>    ...
>
> ----
> In the first case, foreach will call opSlice(), and then walk through the resulting arr[] range with the front/popFront/empty troika.
>
> In the second case, foreach will call opApply on the range "arr[]".
>
> ----
> I'm wondering if this is the correct behavior? In particular, since foreach guarantees a call to opSlice(), so writing "arr[]" *should* be redundant, yet the final behavior is different.
>
> That said, the "issue" *could* be fixed if the base class defines opApply
> as: "return opSlice().opApply(dg)" (or more complex). However:
> a) The implementer of class has no obligation to do this, since he has
> provided a perfectly valid range.
> b) This would force implementers into more generic useless boilerplate code.
>
> What are your thoughts? Which is the "correct" solution? Is it a bug with foreach, or should the base struct/class provide an opApply?
>
> PS: Related: "foreach over range (with opApply) should save range."
> http://d.puremagic.com/issues/show_bug.cgi?id=4347
> On this assigned issue, is the conclusion that foreach will eventually save
> the range?

"monarch_dodra" , dans le message (digitalmars.D:171902), a écrit :
> I just re-read the docs you linked to, and if that was my only source, I'd reach the same conclusion as you.

I think the reference spec for D should be the community driven and widely available website, not a commercial book. But that's not the issue here.

> however, my "The D
> Programming Language", states:
> *12: Operator Overloading
> **9: Overloading foreach
> ***1: foreach with Iteration Primitives
> "Last but not least, if the iterated object offers the slice
> operator with no arguments lst[], __c is initialized with lst[]
> instead of lst. This is in order to allow ?extracting? the
> iteration means out of a container without requiring the
> container to define the three iteration primitives."
> 
> Another thing I find strange about the doc is: "If the foreach range properties do not exist, the opApply method will be used instead." This sounds backwards to me.

Skipping the last paragraph, a reasonable interpretation would be that
foreach try to use, in order of preference:
 - for each over array
 - opApply
 - the three range primitives (preferably four if we include save)
 - opSlice (iteration on the result of opSlice is determined by the same
system).

 opApply should come first, since if someone defines opApply, he or she
obviously wants to override the range primitive iteration.
 opApply and range primitives may be reached via an alias this.
 opSlice is called only if no way to iterate the struct/class is found.
 I would not complain if the fourth rule didn't exist, because it's not
described in dlang.org, but it is a reasonable feature that have be
taken from TDPL (but then it should be added in dlang.org).


This way, if arr is a container that defines an opSlice, and that does not define an opApply, and does not define range primitives:

foreach (a, arr) ...
and
foreach (a, arr[]) ...
should be stricly equivalent. Since the first is translated into the
second. Both work only if arr[] is iterable.

I think you hit a bug.

-- 
Christophe

On Wednesday, July 11, 2012 14:10:35 Christophe Travert wrote:
> "monarch_dodra" , dans le message (digitalmars.D:171868), a écrit :
> > I'm wondering if this is the correct behavior? In particular, since foreach guarantees a call to opSlice(), so writing "arr[]" *should* be redundant, yet the final behavior is different.
> > 
> > That said, the "issue" *could* be fixed if the base class defines
> > opApply as: "return opSlice().opApply(dg)" (or more complex).
> > However:
> > a) The implementer of class has no obligation to do this, since
> > he has provided a perfectly valid range.
> > b) This would force implementers into more generic useless
> > boilerplate code.
> > 
> > What are your thoughts? Which is the "correct" solution? Is it a bug with foreach, or should the base struct/class provide an opApply?
> 
> I think foreach should never call opSlice. That's not in the online documentation (http://dlang.org/statement.html#ForeachStatement), unless I missed something. If you want to use foreach on a class with an opSlice, then yes, you should define opApply. Otherwise, the user have to call opSlice himself, which seems reasonable. That's how I understand the doc.

TDPL says that opSplice is called on a type passed to foreach if it defines it. Look at pages 380 - 381. The last paragraph of section 12.9.1 explicitly mentions it.

- Jonathna M Davis

On Wednesday, July 11, 2012 15:43:13 Christophe Travert wrote:
> "monarch_dodra" , dans le message (digitalmars.D:171902), a écrit :
> > I just re-read the docs you linked to, and if that was my only source, I'd reach the same conclusion as you.
> 
> I think the reference spec for D should be the community driven and widely available website, not a commercial book. But that's not the issue here.

The problem is that it's essentially spread out across 3 places: the online spec, TDPL, and the compiler. The online spec is _supposed_ to have all of the information and be correct, but sometimes behavior and/or planned behavior has been changed without properly updating the online spec, and sometimes the online spec just doesn't include enough information. TDPL includes intended behavior, which in most cases is correct and matches what the compiler currently does, but in some cases it includes features which just haven't been fully implemented yet (e.g. having multiple alias thises). The compiler is of course the current behavior.

So, when they don't match, we have to figure out what the intended behavior is and, in some cases, what we now want the behavior to be (regardless of what we intended before).

The source for the website is freely available ( https://github.com/D- Programming-Language/d-programming-language.org ) - including the spec - and anyone can create a pull request for it. The main problem is probably a combination of the fact that the people who know enough to fix it are very busy with other things (like fixing compiler bugs) and the fact that they aren't always aware of what needs fixing. They're also much more likely to interpret what's there as was intended rather than what a newbie would take it to mean and not realize that it needs improvement. And I suspect that most of the people who know what they're doing around here don't actually look at the spec much. Raising specific issues when you find them and putting them in bugzilla will help.

So, yes, the online spec should have everything needed to specify D in it, but making it so requires time and manpower, and for the most part, that has been spent on other stuff (much of which is also critical - and often more critical).

- Jonathan M Davis

On Wednesday, 11 July 2012 at 20:24:42 UTC, Jonathan M Davis wrote:
> The problem is that it's essentially spread out across 3 places: the online
> spec, TDPL, and the compiler.
>
> ...
>
> - Jonathan M Davis

The problem is not only the documentation, it's getting authoritative answers. I see some strange behaviors, and I can't for the life of find out if it is a bug in the documentation/compiler, or a pitfall/misunderstanding.

What is your take on the issue at hand? Bug or Pitfall?

On 07/12/2012 04:50 PM, monarch_dodra wrote:
> On Wednesday, 11 July 2012 at 20:24:42 UTC, Jonathan M Davis wrote:
>> The problem is that it's essentially spread out across 3 places: the
>> online
>> spec, TDPL, and the compiler.
>>
>> ...
>>
>> - Jonathan M Davis
>
> The problem is not only the documentation, it's getting authoritative
> answers. I see some strange behaviors, and I can't for the life of find
> out if it is a bug in the documentation/compiler, or a
> pitfall/misunderstanding.
>
> What is your take on the issue at hand? Bug or Pitfall?

A pitfall is to be considered a bug. Code should have the obvious
behaviour or be illegal.

On Wed, 11 Jul 2012 11:10:16 -0400, kenji hara <k.hara.pg@gmail.com> wrote:

> I think the online documentation
> (http://dlang.org/statement.html#ForeachStatement) is not sufficient.
>
> foreach (e; aggr) { ...body...}
>
> Current dmd translates above foreach statement like follows.
>
> 1. If aggr has opApply or opApplyReverse, it's used.
>
> 2. If aggr has empty/front/popFront:
>  2a. If aggr has slice operation, it's translated to:
>
>   for (auto __r = aggr[];  // If aggr is a container (e.g. std.container.Array),
>                            // foreach will get its range object for
> the iteration.
>       !__r.empty;
>       __r.popFront()) { auto e = __r.front; ...body... }
>
>  2b. If aggr doesn't have slice operation, it's translated to:
>
>   for (auto __r = aggr;  // If aggr is copyable, saves the original range.
>       !__r.empty;
>       __r.popFront()) { auto e = __r.front; ...body... }
>
> 3. If aggr is static or dynamic array, it's translated to:
>
>   for (auto __tmp = aggr[], __key = 0;  // If aggr is static array,
> get its slice for iteration.
>       !__key < __tmp.length;
>       ++__key) { auto e = __tmp[__key]; ...body... }
>
> These come from the dmd source code.
> https://github.com/D-Programming-Language/dmd/blob/master/src/opover.c#L1226
> https://github.com/D-Programming-Language/dmd/blob/master/src/statement.c#L1522
>

This is wrong.  Why should we require aggr having empty/front/popFront to trigger a call to opSlice, which could have completely different type from aggr?  What if the result of opSlice has opApply?

If opSlice is to be used, this is how it should go (in order of precedence):

1. if aggr has opApply or opApplyReverse, use it.

2. if aggr has opSlice, and the result of aggr.opSlice() has opApply or opApplyReverse, use it.

3. if aggr has opSlice, and the result of aggr.opSlice() has empty/front/popfront, use it as in your 2a above.

4. if aggr has empty/front/popFront, use it as in your 2b above.

5. static or dynamic array.

I should also note that the existence of opApply should not preclude later possibilities if that opApply can't compile for the given foreach parameters.

-Steve