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


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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch


--- Comment #10 from Don <clugdbug@yahoo.com.au> 2010-06-08 00:01:20 PDT ---
This patch only stops the side effect message, it doesn't turn x++; into ++x; Note that this patch deals with more difficult cases such as:

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

void main() {
    Foo foo;
    int w;
    ++w, foo++;
}
----

Index: expression.c ===================================================================
--- expression.c    (revision 526)
+++ expression.c    (working copy)
@@ -8520,6 +8520,14 @@

 int CommaExp::checkSideEffect(int flag)
 {
+    // Check for compiler-generated code of the form  auto __tmp, e, __tmp;
+    // In such cases, only check e for side effect (it's OK for __tmp to have
no side effect).
+    CommaExp * firstComma = this;
+    while (firstComma->e1->op == TOKcomma)
+        firstComma = (CommaExp *)firstComma->e1;
+    if (firstComma->e1->op == TOKdeclaration && e2->op == TOKvar)
+        return e1->checkSideEffect(flag);
+
     if (flag == 2)
         return e1->checkSideEffect(2) || e2->checkSideEffect(2);
     else

-- 
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 #11 from Brad Roberts <braddr@puremagic.com> 2010-06-08 00:24:37 PDT ---
Hrm.. I haven't studied the side effect code enough.  Do you know why Comma:Exp::checkSideEffect isn't just:

return e1->checkSideEffect(flag) || e2->checkSideEffect(flag)

ie, no conditional.

-- 
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 #12 from Don <clugdbug@yahoo.com.au> 2010-06-08 01:22:02 PDT ---
(In reply to comment #11)
> Hrm.. I haven't studied the side effect code enough.  Do you know why Comma:Exp::checkSideEffect isn't just:
> 
> return e1->checkSideEffect(flag) || e2->checkSideEffect(flag)
> 
> ie, no conditional.

If flag isn't 2, you still want to check for a useless subexpression.

Eg.
   int x;
   int y;
   ++y, x;
This should still be an error, since x; has no effect.

This shows me that my patch isn't quite right, it will erroneously allow

void main() {
    Foo foo;
    int w;
    foo++, w;
}

--
Revised patch (added one line): should ensure that the created variable is the same as the one which is returned.

    CommaExp * firstComma = this;
    while (firstComma->e1->op == TOKcomma)
        firstComma = (CommaExp *)firstComma->e1;
    if (firstComma->e1->op == TOKdeclaration && e2->op == TOKvar
+        && ((DeclarationExp *)firstComma->e1)->declaration ==
((VarExp*)e2)->var)
        return e1->checkSideEffect(flag);

-- 
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 #13 from Brad Roberts <braddr@puremagic.com> 2010-06-08 02:07:14 PDT ---
Something about that code bugs me, but I'm having trouble deciding exactly what it is.

Part of it is that there's redundant work.  Move the new code inside the else block?

Part of it is also that it presumes a good bit about the structure of the tree inside a comma expression.  The comment suggests that it can only come from generated code w/in the compiler.  How true is that?  How future proof is it?

Anyway, maybe my subconscious will figure out what's really bugging me while I sleep.  More tomorrow... if anything comes to me.

-- 
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 #14 from Don <clugdbug@yahoo.com.au> 2010-06-08 04:13:39 PDT ---
(In reply to comment #13)
> Something about that code bugs me, but I'm having trouble deciding exactly what it is.
> 
> Part of it is that there's redundant work.  Move the new code inside the else block?

Although the code in the else block is the same, it's for a very different
reason. But I'm not sure it's correct. For example,
int x;
x, ++x;
doesn't raise an error. Yet the first x has no effect!
Shouldn't the part in the else clause be e1->sideeffect() && e2->sideeffect() ?


> Part of it is also that it presumes a good bit about the structure of the tree inside a comma expression.  The comment suggests that it can only come from generated code w/in the compiler.  How true is that?  How future proof is it?

Declarations are not legal inside comma expressions. But the compiler generates
them in several places. They are also used in implementing struct constructors
and postblit, for example.
There's definitely something a bit weird about the compiler generating code
that couldn't get past the parsing stage.

-- 
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
----------------------------------------------------------------------------
                 CC|                            |bearophile_hugs@eml.cc


--- Comment #15 from Don <clugdbug@yahoo.com.au> 2010-06-09 07:57:14 PDT ---
*** Issue 3966 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: -------

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


Walter Bright <bugzilla@digitalmars.com> changed:

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


--- Comment #16 from Walter Bright <bugzilla@digitalmars.com> 2010-06-09 17:08:15 PDT ---
http://www.dsource.org/projects/dmd/changeset/530

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