D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 3514 - opApply should be the first-choice foreach iteration method.
Summary: opApply should be the first-choice foreach iteration method.
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: Other Windows
: P2 normal
Assignee: No Owner
URL:
Keywords:
: 2984 (view as issue list)
Depends on:
Blocks:
 
Reported: 2009-11-16 06:18 UTC by David Simcha
Modified: 2015-06-09 01:27 UTC (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description David Simcha 2009-11-16 06:18:55 UTC
struct Foo {
    uint front() {
        return 1;
    }

    int opApply(int delegate(ref uint) dg) {
        return 1;
    }
}

void main() {
    Foo foo;
    foreach(elem; foo) {}
}

test8.d(68): Error: no property 'empty' for type 'Foo'
test8.d(68): Error: no property 'popFront' for type 'Foo'

Clearly, DMD saw that front() was present and tried to use range foreach.  This is incorrect because:

1.  Only part of the range interface existed.  The opApply interface was complete and should have worked.

2.  If someone defines both a range interface and an opApply interface with the same types, they probably have a good reason, since ranges serve other purposes, but opApply exists **only** for foreach.

3.  Some things, like iterating over trees, can be done more efficiently with control of the stack than without.

Also, once opSlice becomes able to define implicit conversions to ranges for foreach loops, any opApply's defined should take precedence over this for the reasons mentioned above.
Comment 1 David Simcha 2009-11-16 06:21:38 UTC
Forgot to mention:  Also, when using virtual functions, it may sometimes be reasonable to define an opApply as an optimization for foreach loops, even if the range-based foreach has the exact same semantics.  For example:

class Foo {
    SomeType front() {  return _front;}
    void popFront() {  
        // do stuff.
    }

    bool empty() {
        return _empty;
    }
}

In this case each iteration will require three virtual function calls, whereas if opApply were used, the overhead would be reduced to a single delegate call.
Comment 2 Don 2009-11-16 06:40:54 UTC
Doing this will also fix bug 2984 (which is marked as a regression, but isn't really).
Comment 3 Leandro Lucarella 2009-11-16 12:44:28 UTC

*** This issue has been marked as a duplicate of issue 2984 ***
Comment 4 Steven Schveighoffer 2009-11-16 13:07:44 UTC
This bug supersedes bug 2984, because solving this bug will not only address the problem in 2984, but address a very important problem that is not identified in 2984.  That is:  if opApply is present *at all* it should override any range functionality when sending to foreach.  This includes the case not identified in 2984 in which valid range operations are present *alongside* opApply.  e.g.:

import std.stdio;

struct S
{
    int front()
    {
        return 0;
    }

    bool empty()
    {
        return true;
    }

    void popFront()
    {
    }

    int opApply(int delegate(ref int x) dg)
    {
        writeln("inside opapply");
        int x = 0;
        return dg(x);
    }
}

void main()
{
    S s;
    foreach(i; s)
    {
        writeln("inside loop %d", i);
    }
}

Currently outputs nothing, it should output:
inside opapply
inside loop 0
Comment 5 Leandro Lucarella 2009-11-16 13:46:50 UTC
Then bug 2984 should be marked as a duplicate for this bug, right? There is definitely duplication in this 2 bugs, and there is no point to keep both open.

I won't do it just in case I'm messing it up again ;)
Comment 6 Steven Schveighoffer 2009-11-16 14:01:40 UTC
Yeah, probably :)  I'll do it.
Comment 7 Steven Schveighoffer 2009-11-16 14:04:03 UTC
*** Issue 2984 has been marked as a duplicate of this issue. ***
Comment 8 Leandro Lucarella 2009-12-15 16:52:04 UTC
http://www.dsource.org/projects/dmd/changeset/295
Comment 10 Walter Bright 2009-12-31 11:20:21 UTC
Fixed dmd 2.038