Fix rdar://8139785 "implement warning on dead expression in comma operator"

As a bonus, fix the warning for || and && operators; it was emitted even if one of the operands had side effects, e.g:

x || test_logical_foo1();

emitted a bogus "expression result unused" for 'x'.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@107274 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
index 4a8068629e9f97271530611d5de906ae97aceafb..e84f73a1aa0d9fecec52d6d9fa6d13364ffa7410 100644 (file)
--- a/lib/AST/Expr.cpp
+++ b/lib/AST/Expr.cpp
@@ -974,7 +974,8 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
     switch (BO->getOpcode()) {
       default:
         break;
-      // Consider ',', '||', '&&' to have side effects if the LHS or RHS does.
+      // Consider the RHS of comma for side effects. LHS was checked by
+      // Sema::CheckCommaOperands.
       case BinaryOperator::Comma:
         // ((foo = <blah>), 0) is an idiom for hiding the result (and
         // lvalue-ness) of an assignment written in a macro.
@@ -982,10 +983,14 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
               dyn_cast<IntegerLiteral>(BO->getRHS()->IgnoreParens()))
           if (IE->getValue() == 0)
             return false;
+        return BO->getRHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx);
+      // Consider '||', '&&' to have side effects if the LHS or RHS does.
       case BinaryOperator::LAnd:
       case BinaryOperator::LOr:
-        return (BO->getLHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx) ||
-                BO->getRHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx));
+        if (!BO->getLHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx) ||
+            !BO->getRHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx))
+          return false;
+        break;
     }
     if (BO->isAssignmentOp())
       return false;

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index ae16d57870f80e23926af27dfa1e75f8987899e6..c6d5c6590d869bc5af97ef7306177f3f1095f36e 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -5875,6 +5875,8 @@ QualType Sema::CheckAssignmentOperands(Expr *LHS, Expr *&RHS,
 
 // C99 6.5.17
 QualType Sema::CheckCommaOperands(Expr *LHS, Expr *&RHS, SourceLocation Loc) {
+  DiagnoseUnusedExprResult(LHS);
+
   // Comma performs lvalue conversion (C99 6.3.2.1), but not unary conversions.
   // C++ does not perform this conversion (C++ [expr.comma]p1).
   if (!getLangOptions().CPlusPlus)

diff --git a/test/Analysis/dead-stores.c b/test/Analysis/dead-stores.c
index 1c600276ba43b34e84975fe76bcb086abea88bdc..defd7e0b7bdf9922bd2b62828522807a5b21dd7f 100644 (file)
--- a/test/Analysis/dead-stores.c
+++ b/test/Analysis/dead-stores.c
@@ -300,11 +300,11 @@ void f22() {
   case 7:
     (void)(0 && x);
     (void)y7;
-    (void)(0 || (y8, ({ return; }), 1));
+    (void)(0 || (y8, ({ return; }), 1));  // expected-warning {{expression result unused}}
     (void)x;
     break;
   case 8:
-    (void)(1 && (y9, ({ return; }), 1));
+    (void)(1 && (y9, ({ return; }), 1));  // expected-warning {{expression result unused}}
     (void)x;
     break;
   case 9:

diff --git a/test/Parser/objc-try-catch-1.m b/test/Parser/objc-try-catch-1.m
index 1934cbd3b83d040bdd5539c89c70b617dc5b0d7b..719369124e5cf4d380a40f3c3d40c63e53c1f954 100644 (file)
--- a/test/Parser/objc-try-catch-1.m
+++ b/test/Parser/objc-try-catch-1.m
@@ -27,13 +27,15 @@ void * foo()
       return proc();
     }
     @catch (Frob* ex) {
-      @throw 1,2; // expected-error {{@throw requires an Objective-C object type ('int' invalid)}}
+      @throw 1,2; // expected-error {{@throw requires an Objective-C object type ('int' invalid)}} \
+                                 // expected-warning {{expression result unused}}
     }
     @catch (float x) {  // expected-error {{@catch parameter is not a pointer to an interface type}}
       
     }
     @catch(...) {
-      @throw (4,3,proc());
+      @throw (4,3,proc()); // expected-warning {{expression result unused}} \
+                                                  // expected-warning {{expression result unused}}
     }
   }
 

diff --git a/test/Sema/const-eval.c b/test/Sema/const-eval.c
index 9c537250a93a067e284370692b01825b6f526aa9..c132b347d7b2dc8d7b0935cfc7b52cd881348590 100644 (file)
--- a/test/Sema/const-eval.c
+++ b/test/Sema/const-eval.c
@@ -74,5 +74,5 @@ const _Bool constbool = 0;
 EVAL_EXPR(35, constbool)
 EVAL_EXPR(36, constbool)
 
-EVAL_EXPR(37, (1,2.0) == 2.0)
+EVAL_EXPR(37, (1,2.0) == 2.0)  // expected-warning {{expression result unused}}
 EVAL_EXPR(38, __builtin_expect(1,1) == 1)

diff --git a/test/Sema/expr-comma-c89.c b/test/Sema/expr-comma-c89.c
index d0883ba202f9b4309cdf8f177ee392bd2d0db6f7..dc427028d0d1bfdecfb067de334b56611d32dc98 100644 (file)
--- a/test/Sema/expr-comma-c89.c
+++ b/test/Sema/expr-comma-c89.c
@@ -11,7 +11,7 @@ int B[sizeof((a.c)) == 17 ? 1 : -1];
 
 
 // comma does array/function promotion in c99.
-int X[sizeof(0, (foo().c)) == sizeof(char*) ? 1 : -1];
-int Y[sizeof(0, (a,b).c) == sizeof(char*) ? 1 : -1];
-int Z[sizeof(0, (a=b).c) == sizeof(char*) ? 1 : -1];
+int X[sizeof(0, (foo().c)) == sizeof(char*) ? 1 : -1]; // expected-warning {{expression result unused}}
+int Y[sizeof(0, (a,b).c) == sizeof(char*) ? 1 : -1];   // expected-warning {{expression result unused}} expected-warning {{expression result unused}}
+int Z[sizeof(0, (a=b).c) == sizeof(char*) ? 1 : -1];  // expected-warning {{expression result unused}}
 

diff --git a/test/Sema/expr-comma.c b/test/Sema/expr-comma.c
index d3e4020af637f850a5c9032f4138ef9caf064cfd..b004fc1ba3e29547c860302b9fcbd2c8b5dbf64a 100644 (file)
--- a/test/Sema/expr-comma.c
+++ b/test/Sema/expr-comma.c
@@ -11,7 +11,7 @@ int B[sizeof((a.c)) == 17 ? 1 : -1];
 
 
 // comma does not promote array/function in c90 unless they are lvalues.
-int W[sizeof(0, a.c) == sizeof(char*) ? 1 : -1];
-int X[sizeof(0, (foo().c)) == 17 ? 1 : -1];
-int Y[sizeof(0, (a,b).c) == 17 ? 1 : -1];
-int Z[sizeof(0, (a=b).c) == 17 ? 1 : -1];
+int W[sizeof(0, a.c) == sizeof(char*) ? 1 : -1];  // expected-warning {{expression result unused}}
+int X[sizeof(0, (foo().c)) == 17 ? 1 : -1];              // expected-warning {{expression result unused}}
+int Y[sizeof(0, (a,b).c) == 17 ? 1 : -1];   // expected-warning {{expression result unused}} // expected-warning {{expression result unused}}
+int Z[sizeof(0, (a=b).c) == 17 ? 1 : -1];   // expected-warning {{expression result unused}}

diff --git a/test/Sema/i-c-e.c b/test/Sema/i-c-e.c
index 97d9f43cfba68b891285481830129d11500dc47d..c86a93fed82988300562c48813498b0ae426f350 100644 (file)
--- a/test/Sema/i-c-e.c
+++ b/test/Sema/i-c-e.c
@@ -50,9 +50,10 @@ char y[__builtin_constant_p(expr) ? -1 : 1];
 char z[__builtin_constant_p(4) ? 1 : -1];
 
 // Comma tests
-int comma1[0?1,2:3];
-int comma2[1||(1,2)];
-int comma3[(1,2)]; // expected-warning {{size of static array must be an integer constant expression}}
+int comma1[0?1,2:3];  // expected-warning {{expression result unused}}
+int comma2[1||(1,2)]; // expected-warning {{expression result unused}}
+int comma3[(1,2)]; // expected-warning {{size of static array must be an integer constant expression}} \
+                                       // expected-warning {{expression result unused}}
 
 // Pointer + __builtin_constant_p
 char pbcp[__builtin_constant_p(4) ? (intptr_t)&expr : 0]; // expected-error {{variable length array declaration not allowed at file scope}}

diff --git a/test/Sema/warn-unused-value.c b/test/Sema/warn-unused-value.c
index 16b787f774411a8ddab2e54f7fa44d227765ece1..1a7e745785b365cd070692a15c09ccac99c12369 100644 (file)
--- a/test/Sema/warn-unused-value.c
+++ b/test/Sema/warn-unused-value.c
@@ -18,9 +18,9 @@ void pr4806() {
   i,foo();          // expected-warning {{expression result unused}}
   foo(),i;          // expected-warning {{expression result unused}}
 
-  i,j,foo();        // expected-warning {{expression result unused}}
-  i,foo(),j;        // expected-warning {{expression result unused}}
-  foo(),i,j;        // expected-warning {{expression result unused}}
+  i,j,foo();        // expected-warning {{expression result unused}} expected-warning {{expression result unused}}
+  i,foo(),j;        // expected-warning {{expression result unused}} expected-warning {{expression result unused}}
+  foo(),i,j;        // expected-warning {{expression result unused}} expected-warning {{expression result unused}}
 
   i++;
 
@@ -63,6 +63,16 @@ int test_logical_bar() {
   (x = test_logical_foo1()) ||  // no-warning
   (x = test_logical_foo2()) ||  // no-warning
   (x = test_logical_foo3());    // no-warning
+
+  x || test_logical_foo1();     // no-warning
+
   return x;
 }
 
+struct s0 { int f0; };
+
+void f0(int a);
+void f1(struct s0 *a) {
+  // rdar://8139785
+  f0((int)(a->f0 + 1, 10)); // expected-warning {{expression result unused}}
+}

diff --git a/test/SemaCXX/overloaded-operator.cpp b/test/SemaCXX/overloaded-operator.cpp
index b9e0786739b18f4f98533283e50c3fb0bcbe3bd1..24f7f66129ebd9b4564297e632701a8bf56b48ae 100644 (file)
--- a/test/SemaCXX/overloaded-operator.cpp
+++ b/test/SemaCXX/overloaded-operator.cpp
@@ -157,7 +157,7 @@ bool& operator,(X, Y);
 
 void test_comma(X x, Y y) {
   bool& b1 = (x, y);
-  X& xr = (x, x);
+  X& xr = (x, x); // expected-warning {{expression result unused}}
 }
 
 struct Callable {