View mode: basic / threaded / horizontal-split · Log in · Help
July 11, 2012
opApply not called for foeach(container)
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?
July 11, 2012
Re: opApply not called for foeach(container)
"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
July 11, 2012
Re: opApply not called for foeach(container)
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.
July 11, 2012
Re: opApply not called for foeach(container)
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?
July 11, 2012
Re: opApply not called for foeach(container)
"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
July 11, 2012
Re: opApply not called for foeach(container)
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
July 11, 2012
Re: opApply not called for foeach(container)
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
July 12, 2012
Re: opApply not called for foeach(container)
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?
July 12, 2012
Re: opApply not called for foeach(container)
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.
July 12, 2012
Re: opApply not called for foeach(container)
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
« First   ‹ Prev
1 2
Top | Discussion index | About this forum | D home