www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 10009] New: foreach_reverse and AA.byKey/byValue

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

           Summary: foreach_reverse and AA.byKey/byValue
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: henning still-hidden.de


--- Comment #0 from Henning Pohl <henning still-hidden.de> 2013-04-29 12:22:36
PDT ---
void main() {
    auto aa = [1 : 2, 2 : 3];
    foreach_reverse (e; aa.byKey()) { }
}

-----
test.d(4): Error: invalid foreach aggregate aa.byKey().state
-----

foreach works like a charm.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 29 2013
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10009


Andrej Mitrovic <andrej.mitrovich gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrej.mitrovich gmail.com


--- Comment #1 from Andrej Mitrovic <andrej.mitrovich gmail.com> 2013-04-29
13:04:56 PDT ---
I think foreach_reverse was only ever meant to be used with arrays (and is
scheduled for deprecations), for ranges you can use retro() from std.algorithm.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 29 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10009


bearophile_hugs eml.cc changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bearophile_hugs eml.cc


--- Comment #2 from bearophile_hugs eml.cc 2013-04-29 15:49:24 PDT ---
(In reply to comment #1)
 I think foreach_reverse was only ever meant to be used with arrays (and is
 scheduled for deprecations), for ranges you can use retro() from std.algorithm.
I have hundreds of usages of foreach_reverse in my D2 codebase. You aren't going to deprecate it. retro() is not nearly as efficient for arrays. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Apr 29 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10009



--- Comment #3 from bearophile_hugs eml.cc 2013-04-29 15:59:48 PDT ---
(In reply to comment #2)

 I have hundreds of usages of foreach_reverse in my D2 codebase. You aren't
 going to deprecate it. retro() is not nearly as efficient for arrays.
Sorry, my brain was switched off. And the usages are probably under one hundred. ----------------- (In reply to comment #1)
 I think foreach_reverse was only ever meant to be used with arrays (and is
 scheduled for deprecations), for ranges you can use retro() from std.algorithm.
Maybe the compiler can rewrite this: foreach_reverse(x; someRange) as: foreach(x; someRange.retro) But I don't know what problems this causes. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Apr 29 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10009



--- Comment #4 from Andrej Mitrovic <andrej.mitrovich gmail.com> 2013-04-29
16:06:07 PDT ---
(In reply to comment #2)
 (In reply to comment #1)
 I think foreach_reverse was only ever meant to be used with arrays (and is
 scheduled for deprecations), for ranges you can use retro() from std.algorithm.
I have hundreds of usages of foreach_reverse in my D2 codebase. You aren't going to deprecate it. retro() is not nearly as efficient for arrays.
Why is retro less efficient though? -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Apr 29 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10009


Jonathan M Davis <jmdavisProg gmx.com> changed:

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


--- Comment #5 from Jonathan M Davis <jmdavisProg gmx.com> 2013-04-29 17:16:04
PDT ---
 Why is retro less efficient though?
Probably something to do with how the decoding is done and/or some extra steps which can be skipped. I don't think that I ever investigated it in detail, but I know that I used foreach_reverse in some functions in std.string because it was definitely faster according to the benchmarks that I did, much as I would prefer to never use it. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Apr 29 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10009



--- Comment #6 from Andrej Mitrovic <andrej.mitrovich gmail.com> 2013-05-02
04:01:37 PDT ---
Well we better decide whether or not foreach_reverse is actually scheduled for
deprecation. If it is then we shouldn't add more features to it.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
May 02 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10009


yebblies <yebblies gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |yebblies gmail.com


--- Comment #7 from yebblies <yebblies gmail.com> 2013-05-03 03:40:45 EST ---
Leave foreach_reverse alone!

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
May 02 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10009


Steven Schveighoffer <schveiguy yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy yahoo.com


--- Comment #8 from Steven Schveighoffer <schveiguy yahoo.com> 2013-05-03
14:43:31 PDT ---
The problem I see with removing foreach_reverse is that the compiler actually
rewrites it directly instead of calling a function.  Requiring to use retro
when it is in std.range is not good for modularity.

In addition, foreach_reverse(x..y) is nice to use instead of some combination
of ranges.

However, I don't think foreach_reverse is valid for this use case.  An AA is
only forward-traversable.  foreach_reverse is not possible, as buckets are
stored as singly-linked lists.  If you want that, you need to first capture it
as an array.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
May 03 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10009



--- Comment #9 from bearophile_hugs eml.cc 2013-05-03 15:59:38 PDT ---
(In reply to comment #8)

 However, I don't think foreach_reverse is valid for this use case.  An AA is
 only forward-traversable.  foreach_reverse is not possible, as buckets are
 stored as singly-linked lists.  If you want that, you need to first capture it
 as an array.
Yet this compiles: import std.stdio: writeln; void main() { auto aa = [1: 2, 2: 3]; foreach (k, v; aa) writeln(k, " ", v); foreach_reverse (k, v; aa) writeln(k, " ", v); } And outputs: 1 2 2 3 1 2 2 3 The foreach seems to give the same sequence as the foreach_reverse :-) -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
May 03 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10009



--- Comment #10 from Jonathan M Davis <jmdavisProg gmx.com> 2013-05-04 17:16:54
PDT ---
 The foreach seems to give the same sequence as the foreach_reverse
Which is essentially the same problem as bug# 6251. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
May 04 2013
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10009



--- Comment #11 from Steven Schveighoffer <schveiguy yahoo.com> 2013-05-06
11:25:25 PDT ---
(In reply to comment #10)

 Which is essentially the same problem as bug# 6251.
Not exactly. AA is a type, not a delegate. It is allowed to define opApplyReverse. A delegate cannot possibly be valid with foreach_reverse (and that bug should be killed with prejudice). Even if opApplyReverse is defined for AAs, the implementation will be wrong (as bearophile described) because it's built from singly linked lists. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
May 06 2013