www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 1553] New: foreach_reverse is allowed for delegates

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1553

           Summary: foreach_reverse is allowed for delegates
           Product: D
           Version: 1.022
          Platform: PC
        OS/Version: Windows
            Status: NEW
          Severity: enhancement
          Priority: P4
         Component: DMD
        AssignedTo: bugzilla digitalmars.com
        ReportedBy: thecybershadow gmail.com


Suggestion: do not allow to use delegates with foreach_reverse. Since, in the
case of delegates, foreach_reverse is synonymous with foreach, it can be a
cause of bugs, especially in the case of rewriting code that looped over a
pre-built array to loop using a delegate.


--
Oct 07 2007
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1553


yebblies <yebblies gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |accepts-invalid, patch
           Priority|P4                          |P3
                 CC|                            |yebblies gmail.com
           Platform|x86                         |All
            Version|1.022                       |D1 & D2
         OS/Version|Windows                     |All
           Severity|enhancement                 |normal



https://github.com/D-Programming-Language/dmd/pull/184

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 30 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1553


Walter Bright <bugzilla digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |bugzilla digitalmars.com
         Resolution|                            |WONTFIX



18:24:24 PDT ---
I don't think it's right to make foreach_reverse crippled compared with
foreach. I'm going to mark as won't fix.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 01 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1553




18:29:03 PDT ---
Walter, did you understand the bug report correctly? My 4-year-old explanation
could have been better, but what I meant is that:

foreach (v; someDelegate) { ... }

did the exact same thing as 

foreach_reverse (v; someDelegate) { ... }

Someone may try to use foreach_reverse intuitively over a delegate, hoping
it'll "just work", and get unexpected results (as I have 4 years ago).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 01 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1553




18:48:10 PDT ---
Yes, I understood your point. I agree that one could make an error this way. I
disagree that the solution is to remove the feature.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 01 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1553




18:50:16 PDT ---
Why? This is clearly an accepts-invalid bug! What would a better solution be,
anyway? reverse_delegate?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 01 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1553




19:16:27 PDT ---
The compiler cannot tell what the delegate does, so there's no way it can
diagnose an error.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 01 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1553




19:18:00 PDT ---
Exactly! We can only assume that all delegates are written for forward
iteration. I am having trouble understanding your problem with disabling
foreach_reverse with delegates specifically. You say it'd be removing a
feature, but how can it be a feature if it neither is nor can be implemented?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 01 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1553


Jonathan M Davis <jmdavisProg gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jmdavisProg gmx.com



PDT ---
But why couldn't a delegate be written for reverse iteration? And if it does
exactly the same thing for both foreach and foreach_reverse, I don't really see
the problem. Granted, it may be a bit weird, but I don't see the bug.

Though truth be told, I've been wondering whether foreach_reverse is actually
supposed to be sticking around, given the general move towards ranges and the
fact that foreach_reverse isn't actually mentioned in TDPL (at least, it's not
in the index).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 01 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1553




19:38:33 PDT ---


 But why couldn't a delegate be written for reverse iteration? 


So put the semantics in the delegate name, instead of expecting the user to
always use the correct one of the two semantically-opposite but actually
synonymous keywords. This can easily become a point of confusion, and I'm
surprised I need to elaborate in so much detail why this is plain bad.

What's wrong with writing it like this?

foreach (v; &foo.reverseIterator) { ... }

If you start writing it like this:

foreach_reverse (v; &foo.reverseIterator)

Sooner or later someone will forget the second "reverse", the code will look
and compile right, and work wrong!

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 01 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1553




19:39:51 PDT ---


 But why couldn't a delegate be written for reverse iteration?


Exactly. It can be. Why take this away?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 01 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1553




19:41:10 PDT ---


 Exactly. It can be. Why take this away?


You get the same thing by putting the semantic in the delegate name, without
risking bugs and misunderstanding.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 01 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1553


Steven Schveighoffer <schveiguy yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
                 CC|                            |schveiguy yahoo.com
         Resolution|WONTFIX                     |
           Severity|normal                      |enhancement



05:44:32 PDT ---
Have to chime in to agree with Vladimir here.

foreach_reverse is such a special case (that is technically no longer useful),
that I think it should only work on arrays.

Essentially, using a delegate takes care of foreach_reverse for structs or
classes:

foreach(x; &obj.inReverse)

and retro takes care of ranges,  so all that is left is builtins.  The only two
builtin types that can be iterated are arrays and associative arrays. 
Reversing the order of iterating an associative array makes no sense at all. 
All that is left is arrays.  I agree arrays need something that currently does
not exist if we wanted to deep-six foreach_reverse, but it has no use outside
arrays.  I can't think of a single case where foreach_reverse should be used
instead of foreach on a delegate.  I'd say 100% of the time, it is an error. 
If you hear any complaining, I'll personally defend the choice :)

I'm marking this as an enhancement, as technically foreach_reverse works as
specified.  If you do not think it's worth leaving open as an enhancement
request, I'll leave it alone.  The one good thing about this 'bug' is that it's
not likely happen -- It likely happens when someone tries foreach_reverse on a
delegate, finds it doesn't do what they want, and switches it back, never to be
used again.  I'd hazard to guess that if you implemented this fix, no code will
break.  Anywhere.

If this is closed, I'll reopen for updating the documentation to say "using
foreach_reverse on a delegate does *exactly the same thing* as using foreach on
a delegate".  If it won't be fixed, at least it should be documented as
something you should never do ;)

BTW, related is bug 2498 that would make the &obj.inReverse be writable as
obj.inReverse (more pleasant to read/write):

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 04 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1553


Walter Bright <bugzilla digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |WONTFIX



12:39:42 PDT ---


 If this is closed, I'll reopen for updating the documentation to say "using
 foreach_reverse on a delegate does *exactly the same thing* as using foreach on
 a delegate".  If it won't be fixed, at least it should be documented as
 something you should never do ;)


That would be fine.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 04 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1553




*** Issue 6251 has been marked as a duplicate of this issue. ***

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 06 2012
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1553


Denis Shelomovskij <verylonglogin.reg gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
                 CC|                            |verylonglogin.reg gmail.com
            Version|D1 & D2                     |D2
         Resolution|WONTFIX                     |



2013-10-21 19:28:30 MSD ---
Reopened for D2 because of a community support of the described problem. See NG
thread:
http://forum.dlang.org/thread/l40er2$1dhu$1 digitalmars.com

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 21 2013