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'.

llvm-svn: 107274

clang/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;

clang/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)

clang/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:

clang/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}}
     }
   }
 

clang/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)

clang/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}}
 

clang/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}}

clang/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}}

clang/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}}
+}

clang/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 {