fix Issue 12390 - "has no effect in expression" diagnostic regression

When value discarding check on statements, the side-effect check should not be done deeply.
dlang · Apr 7, 2014 · 89ca0a1 · 89ca0a1
1 parent 6d10394
commit 89ca0a1
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 69 deletions.
diff --git a/src/sideeffect.c b/src/sideeffect.c
@@ -24,7 +24,7 @@
 #include "attrib.h"
 
 bool walkPostorder(Expression *e, StoppableVisitor *v);
-void discardValue(Expression *e);
+bool lambdaHasSideEffect(Expression *e);
 
 /********************************************
  * Determine if Expression has any side effects.
@@ -39,81 +39,86 @@ bool hasSideEffect(Expression *e)
 
         void visit(Expression *e)
         {
-            switch (e->op)
-            {
-            // Sort the cases by most frequently used first
-            case TOKassign:
-            case TOKplusplus:
-            case TOKminusminus:
-            case TOKdeclaration:
-            case TOKconstruct:
-            case TOKblit:
-            case TOKaddass:
-            case TOKminass:
-            case TOKcatass:
-            case TOKmulass:
-            case TOKdivass:
-            case TOKmodass:
-            case TOKshlass:
-            case TOKshrass:
-            case TOKushrass:
-            case TOKandass:
-            case TOKorass:
-            case TOKxorass:
-            case TOKpowass:
-            case TOKin:
-            case TOKremove:
-            case TOKassert:
-            case TOKhalt:
-            case TOKdelete:
-            case TOKnew:
-            case TOKnewanonclass:
-                // stop walking if we determine this expression has side effects
-                stop = true;
-                break;
+            // stop walking if we determine this expression has side effects
+            stop = lambdaHasSideEffect(e);
+        }
+    };
+
+    LambdaHasSideEffect v;
+    return walkPostorder(e, &v);
+}
 
-            case TOKcall:
+bool lambdaHasSideEffect(Expression *e)
+{
+    switch (e->op)
+    {
+        // Sort the cases by most frequently used first
+        case TOKassign:
+        case TOKplusplus:
+        case TOKminusminus:
+        case TOKdeclaration:
+        case TOKconstruct:
+        case TOKblit:
+        case TOKaddass:
+        case TOKminass:
+        case TOKcatass:
+        case TOKmulass:
+        case TOKdivass:
+        case TOKmodass:
+        case TOKshlass:
+        case TOKshrass:
+        case TOKushrass:
+        case TOKandass:
+        case TOKorass:
+        case TOKxorass:
+        case TOKpowass:
+        case TOKin:
+        case TOKremove:
+        case TOKassert:
+        case TOKhalt:
+        case TOKdelete:
+        case TOKnew:
+        case TOKnewanonclass:
+            return true;
+
+        case TOKcall:
+        {
+            CallExp *ce = (CallExp *)e;
+            /* Calling a function or delegate that is pure nothrow
+             * has no side effects.
+             */
+            if (ce->e1->type)
             {
-                CallExp *ce = (CallExp *)e;
-                /* Calling a function or delegate that is pure nothrow
-                 * has no side effects.
-                 */
-                if (ce->e1->type)
+                Type *t = ce->e1->type->toBasetype();
+                if ((t->ty == Tfunction && ((TypeFunction *)t)->purity > PUREweak &&
+                                           ((TypeFunction *)t)->isnothrow)
+                    ||
+                    (t->ty == Tdelegate && ((TypeFunction *)((TypeDelegate *)t)->next)->purity > PUREweak &&
+                                           ((TypeFunction *)((TypeDelegate *)t)->next)->isnothrow)
+                   )
                 {
-                    Type *t = ce->e1->type->toBasetype();
-                    if ((t->ty == Tfunction && ((TypeFunction *)t)->purity > PUREweak &&
-                                               ((TypeFunction *)t)->isnothrow)
-                        ||
-                        (t->ty == Tdelegate && ((TypeFunction *)((TypeDelegate *)t)->next)->purity > PUREweak &&
-                                               ((TypeFunction *)((TypeDelegate *)t)->next)->isnothrow)
-                       )
-                    {
-                    }
-                    else
-                        stop = true;
                 }
-                break;
-            }
-
-            case TOKcast:
-            {
-                CastExp *ce = (CastExp *)e;
-                /* if:
-                 *  cast(classtype)func()  // because it may throw
-                 */
-                if (ce->to->ty == Tclass && ce->e1->op == TOKcall && ce->e1->type->ty == Tclass)
-                    stop = true;
-                break;
+                else
+                    return true;
             }
+            break;
+        }
 
-            default:
-                break;
-            }
+        case TOKcast:
+        {
+            CastExp *ce = (CastExp *)e;
+            /* if:
+             *  cast(classtype)func()  // because it may throw
+             */
+            if (ce->to->ty == Tclass && ce->e1->op == TOKcall && ce->e1->type->ty == Tclass)
+                return true;
+            break;
         }
-    };
 
-    LambdaHasSideEffect v;
-    return walkPostorder(e, &v);
+        default:
+            break;
+    }
+    return false;
 }
 
 
@@ -123,7 +128,7 @@ bool hasSideEffect(Expression *e)
  */
 void discardValue(Expression *e)
 {
-    if (hasSideEffect(e))
+    if (lambdaHasSideEffect(e))     // check side-effect shallowly
         return;
     switch (e->op)
     {

diff --git a/test/fail_compilation/fail12390.d b/test/fail_compilation/fail12390.d
@@ -0,0 +1,15 @@
+/*
+TEST_OUTPUT:
+---
+fail_compilation/fail12390.d(14): Error: == has no effect in expression (fun().i == 4)
+---
+*/
+
+struct S { int i; }
+
+S fun() { return S(42); }
+
+void main()
+{
+    fun().i == 4;
+}