Thread overview
[Issue 2023] New: Returning from foreach body doesn't work as expected.
Apr 21, 2008
d-bugmail
Apr 21, 2008
Max Samukha
Apr 21, 2008
d-bugmail
Apr 21, 2008
d-bugmail
Apr 21, 2008
d-bugmail
April 21, 2008
http://d.puremagic.com/issues/show_bug.cgi?id=2023

           Summary: Returning from foreach body doesn't work as expected.
           Product: D
           Version: 2.012
          Platform: PC
        OS/Version: Windows
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: bugzilla@digitalmars.com
        ReportedBy: samukha@voliacable.com


The bug is reproduced by the following code. Instead of making foo return, the 'return' statement in foo's foreach body breaks the loop:

----
struct S
{
    int a[];

    int opApply(int delegate(ref int) dg)
    {
        foreach (x; a)
        {
            if(dg(x))
                return 1;
        }

        return 0;
    }
}

void foo()
{
    auto s = S([1, 2, 3]);

    foreach (x; s)
    {
        if (x == 1)
            return; // breaks the loop instead of returning from foo.
    }

    assert(false); // should never reach here.
}

void main()
{
    foo();
}
----


-- 

April 21, 2008
<d-bugmail@puremagic.com> wrote in message news:bug-2023-3@http.d.puremagic.com/issues/...
> http://d.puremagic.com/issues/show_bug.cgi?id=2023
>
>           Summary: Returning from foreach body doesn't work as expected.
>           Product: D
>           Version: 2.012
>          Platform: PC
>        OS/Version: Windows
>            Status: NEW
>          Severity: normal
>          Priority: P2
>         Component: DMD
>        AssignedTo: bugzilla@digitalmars.com
>        ReportedBy: samukha@voliacable.com

> struct S
> {
>    int a[];
>
>    int opApply(int delegate(ref int) dg)
>    {
>        foreach (x; a)
>        {
>            if(dg(x))
>                return 1;
>        }
>
>        return 0;
>    }
> }


You're implementing the opApply wrong.

if(auto result = dg(x))
    return result;

The result of the delegate is an int, not a bool, for a reason.  There are many possible return values, not just 1 or 0; you're just faking out the runtime by doing so.


April 21, 2008
http://d.puremagic.com/issues/show_bug.cgi?id=2023





------- Comment #1 from wbaxter@gmail.com  2008-04-21 07:57 -------
Here's my opportunity to restate my opinion that the programmer should not be responsible for passing around opaque magic values generated by the compiler. At least not in a supposedly modern language.

I wrote a proposal about how to make opApply more user-friendly
Here: digitalmars.D:64686
And a few clarifications here: digitalmars.D:64706

Unfortunately I think it really requires macro() to make it pretty.


-- 

April 21, 2008
http://d.puremagic.com/issues/show_bug.cgi?id=2023





------- Comment #2 from wbaxter@gmail.com  2008-04-21 07:59 -------
(In reply to comment #1)

I think there's some way to get links to NG posts here automatically but I can never remember what it is...

> I wrote a proposal about how to make opApply more user-friendly Here: digitalmars.D:64686

http://www.digitalmars.com/pnews/read.php?server=news.digitalmars.com&group=digitalmars.D&artnum=64686&header

> And a few clarifications here: digitalmars.D:64706

http://www.digitalmars.com/pnews/read.php?server=news.digitalmars.com&group=digitalmars.D&artnum=64706&header


-- 

April 21, 2008
On Mon, 21 Apr 2008 08:46:55 -0400, "Jarrett Billingsley" <kb3ctd2@yahoo.com> wrote:

><d-bugmail@puremagic.com> wrote in message news:bug-2023-3@http.d.puremagic.com/issues/...
>> http://d.puremagic.com/issues/show_bug.cgi?id=2023
>>
>>           Summary: Returning from foreach body doesn't work as expected.
>>           Product: D
>>           Version: 2.012
>>          Platform: PC
>>        OS/Version: Windows
>>            Status: NEW
>>          Severity: normal
>>          Priority: P2
>>         Component: DMD
>>        AssignedTo: bugzilla@digitalmars.com
>>        ReportedBy: samukha@voliacable.com
>
>> struct S
>> {
>>    int a[];
>>
>>    int opApply(int delegate(ref int) dg)
>>    {
>>        foreach (x; a)
>>        {
>>            if(dg(x))
>>                return 1;
>>        }
>>
>>        return 0;
>>    }
>> }
>
>
>You're implementing the opApply wrong.
>
>if(auto result = dg(x))
>    return result;
>
>The result of the delegate is an int, not a bool, for a reason.  There are many possible return values, not just 1 or 0; you're just faking out the runtime by doing so.
>

Oops. Anybody knows why I assumed it's boolean? That is crazy.
April 21, 2008
http://d.puremagic.com/issues/show_bug.cgi?id=2023


samukha@voliacable.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |INVALID




------- Comment #3 from samukha@voliacable.com  2008-04-21 09:46 -------
Maybe it makes sense to post your proposal to bugzilla as enhancement? For now, I'm closing the bug as the example is invalid.


--