http://d.puremagic.com/issues/show_bug.cgi?id=4231

           Summary: Solidary opUnary Postincrement and Postdecrement user
                    defined operators are broken.
           Product: D
           Version: unspecified
          Platform: Other
        OS/Version: All
            Status: NEW
          Keywords: rejects-valid
          Severity: regression
          Priority: P3
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: sandford@jhu.edu


--- Comment #0 from Rob Jacques <sandford@jhu.edu> 2010-05-24 21:45:40 PDT ---
It appears that the operator re-writing for the opUnary Post increment and Post decrement operators is incorrect for single line expressions.

struct Foo{
    int opUnary(string op)() { return 1; }
}

void main() {
    Foo foo;
    foo++;   // Error: var has no effect in expression (__tmp608)

}

I've marked this as a regression since this was possible (and still is) using the old operator overloading style.

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

http://d.puremagic.com/issues/show_bug.cgi?id=4231


Don <clugdbug@yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |performance
                 CC|                            |clugdbug@yahoo.com.au
            Summary|Solidary opUnary            |Solitary opUnary
                   |Postincrement and           |Postincrement and
                   |Postdecrement user defined  |Postdecrement user defined
                   |operators are broken.       |operators are broken.


--- Comment #1 from Don <clugdbug@yahoo.com.au> 2010-06-01 12:26:55 PDT ---
Root cause: the temporary variable should not be created if the return value isn't required. This would mean that when the value is not required, preincrement and postincrement are identical, resulting in optimal performance.

Note that the same situation (no return value required) occurs inside comma
expressions.

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

http://d.puremagic.com/issues/show_bug.cgi?id=4231


Brad Roberts <braddr@puremagic.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |braddr@puremagic.com


--- Comment #2 from Brad Roberts <braddr@puremagic.com> 2010-06-01 17:20:53 PDT ---
Doesn't the optimizer take care of eliminating the unused temporary and copy?

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

http://d.puremagic.com/issues/show_bug.cgi?id=4231


Andrei Alexandrescu <andrei@metalanguage.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrei@metalanguage.com


--- Comment #3 from Andrei Alexandrescu <andrei@metalanguage.com> 2010-06-01 17:29:18 PDT ---
Thanks, Don!

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

http://d.puremagic.com/issues/show_bug.cgi?id=4231



--- Comment #4 from Don <clugdbug@yahoo.com.au> 2010-06-02 08:53:44 PDT ---
(In reply to comment #2)
> Doesn't the optimizer take care of eliminating the unused temporary and copy?

Good question. Maybe it does. It sees:
auto t = e, foo(e), t;
Does it know in general that foo() cannot reach t?

BTW -- should the compiler be allowed to eliminate the temporary, if there's a postblit? Ie, is it *forced* to perform the rewrite:

(auto t = e, --e, t)

even if the return value is not used? The optimizer certainly couldn't eliminate it in the general case, but it'd be possible if the front-end is allowed to elide it.

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

http://d.puremagic.com/issues/show_bug.cgi?id=4231



--- Comment #5 from Andrei Alexandrescu <andrei@metalanguage.com> 2010-06-02 08:57:50 PDT ---
That looks like a very specialized optimization to me. In particular, if the postblit has side effects, the optimizer must have advanced knowledge in order to elide it. This is a path that C++ has taken with copy constructor elision, and it's not a path we should take.

I think the language definition should clarify that postincrement and postdecrement are lowered into their pre- counterparts if the result is not taken.

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

http://d.puremagic.com/issues/show_bug.cgi?id=4231



--- Comment #6 from Brad Roberts <braddr@puremagic.com> 2010-06-02 10:22:56 PDT ---
I was thinking about the code post-inlining.  MOST of the time the operators will be inlined and at that point it should be dead simple for it to eliminate dead stores and thus the temporaries would just go away, no special knowledge or techniques required.  Without inlining, yeah, it can't make assumptions what can occur inside the function calls.

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

http://d.puremagic.com/issues/show_bug.cgi?id=4231



--- Comment #7 from Andrei Alexandrescu <andrei@metalanguage.com> 2010-06-02 10:53:53 PDT ---
(In reply to comment #6)
> I was thinking about the code post-inlining.  MOST of the time the operators will be inlined and at that point it should be dead simple for it to eliminate dead stores and thus the temporaries would just go away, no special knowledge or techniques required.  Without inlining, yeah, it can't make assumptions what can occur inside the function calls.

Inlining is irrelevant. If a this(this) has a writeln() in it, the optimizer
must honor it no questions asked. That's why elision must come from a higher
level.

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

http://d.puremagic.com/issues/show_bug.cgi?id=4231



--- Comment #8 from Brad Roberts <braddr@puremagic.com> 2010-06-02 11:19:00 PDT ---
(In reply to comment #7)
> Inlining is irrelevant. If a this(this) has a writeln() in it, the optimizer
> must honor it no questions asked. That's why elision must come from a higher
> level.

It's entirely relevant for the original issue: removal of unnecessary temporaries.  Yes, there are opportunities for the language to define away some parts, but that's a separate discussion for a separate bug report.

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

http://d.puremagic.com/issues/show_bug.cgi?id=4231



--- Comment #9 from Don <clugdbug@yahoo.com.au> 2010-06-06 13:03:45 PDT ---
Bug 3966 is the same as this one. But I'm loathe to mark either as duplicate since 4231 contains useful discussions and 3966 has a vote.

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