[Sema] Fix -Wcomma for C89

There is a small difference in the scope flags for C89 versus the other C/C++
dialects.  This change ensures that the -Wcomma warning won't be duplicated or
issued in the wrong location.  Also, the test case is refactored into C and C++
parts, with the C++ parts guarded by a #ifdef to allow the test to run in both
modes.

https://bugs.llvm.org/show_bug.cgi?id=32370


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

3 files changed, 139 insertions, 119 deletions

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 98025ca27c..66194c6a07 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -11309,8 +11309,11 @@ void Sema::DiagnoseCommaOperator(const Expr *LHS, SourceLocation Loc) {
   // The whitelisted locations are the initialization and increment portions
   // of a for loop.  The additional checks are on the condition of
   // if statements, do/while loops, and for loops.
+  // Differences in scope flags for C89 mode requires the extra logic.
   const unsigned ForIncrementFlags =
-      Scope::ControlScope | Scope::ContinueScope | Scope::BreakScope;
+      getLangOpts().C99 || getLangOpts().CPlusPlus
+          ? Scope::ControlScope | Scope::ContinueScope | Scope::BreakScope
+          : Scope::ContinueScope | Scope::BreakScope;
   const unsigned ForInitFlags = Scope::ControlScope | Scope::DeclScope;
   const unsigned ScopeFlags = getCurScope()->getFlags();
   if ((ScopeFlags & ForIncrementFlags) == ForIncrementFlags ||
diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp
index 16ed9ed767..7fc89bfd69 100644
--- a/lib/Sema/SemaStmt.cpp
+++ b/lib/Sema/SemaStmt.cpp
@@ -551,8 +551,9 @@ Sema::ActOnIfStmt(SourceLocation IfLoc, bool IsConstexpr, Stmt *InitStmt,
         false);
 
   Expr *CondExpr = Cond.get().second;
-  if (!Diags.isIgnored(diag::warn_comma_operator,
-                       CondExpr->getExprLoc()))
+  // Only call the CommaVisitor when not C89 due to differences in scope flags.
+  if ((getLangOpts().C99 || getLangOpts().CPlusPlus) &&
+      !Diags.isIgnored(diag::warn_comma_operator, CondExpr->getExprLoc()))
     CommaVisitor(*this).Visit(CondExpr);
 
   if (!elseStmt)
@@ -1328,6 +1329,11 @@ Sema::ActOnDoStmt(SourceLocation DoLoc, Stmt *Body,
     return StmtError();
   Cond = CondResult.get();
 
+  // Only call the CommaVisitor for C89 due to differences in scope flags.
+  if (Cond && !getLangOpts().C99 && !getLangOpts().CPlusPlus &&
+      !Diags.isIgnored(diag::warn_comma_operator, Cond->getExprLoc()))
+    CommaVisitor(*this).Visit(Cond);
+
   DiagnoseUnusedExprResult(Body);
 
   return new (Context) DoStmt(Body, Cond, DoLoc, WhileLoc, CondRParen);
diff --git a/test/SemaCXX/warn-comma-operator.cpp b/test/SemaCXX/warn-comma-operator.cpp
index 75ef4521cb..0ed127b943 100644
--- a/test/SemaCXX/warn-comma-operator.cpp
+++ b/test/SemaCXX/warn-comma-operator.cpp
@@ -1,8 +1,16 @@
 // RUN: %clang_cc1 -fsyntax-only -Wcomma -std=c++11 -verify %s
 // RUN: %clang_cc1 -fsyntax-only -Wcomma -std=c++11 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
+// RUN: %clang_cc1 -fsyntax-only -Wcomma -x c -std=c89 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wcomma -x c -std=c99 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wcomma -x c -std=c11 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wcomma -x c -std=c17 -verify %s
+
+// int returning function
+int return_four() { return 5; }
+
 // Test builtin operators
-void test1() {
+void test_builtin() {
   int x = 0, y = 0;
   for (; y < 10; x++, y++) {}
   for (; y < 10; ++x, y++) {}
@@ -23,6 +31,116 @@ void test1() {
   for (; y < 10; x ^= 5, ++y) {}
 }
 
+// Test nested comma operators
+void test_nested() {
+  int x1, x2, x3;
+  int y1, *y2 = 0, y3 = 5;
+
+#if __STDC_VERSION >= 199901L
+  for (int z1 = 5, z2 = 4, z3 = 3; x1 <4; ++x1) {}
+#endif
+}
+
+// Confusing "," for "=="
+void test_compare() {
+  if (return_four(), 5) {}
+  // expected-warning@-1{{comma operator}}
+  // expected-note@-2{{cast expression to void}}
+  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:7-[[@LINE-3]]:7}:"static_cast<void>("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:20-[[@LINE-4]]:20}:")"
+
+  if (return_four() == 5) {}
+}
+
+// Confusing "," for "+"
+int test_plus() {
+  return return_four(), return_four();
+  // expected-warning@-1{{comma operator}}
+  // expected-note@-2{{cast expression to void}}
+  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:10-[[@LINE-3]]:10}:"static_cast<void>("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:23-[[@LINE-4]]:23}:")"
+
+  return return_four() + return_four();
+}
+
+// Be sure to look through parentheses
+void test_parentheses() {
+  int x, y;
+  for (x = 0; return_four(), x;) {}
+  // expected-warning@-1{{comma operator}}
+  // expected-note@-2{{cast expression to void}}
+  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:15-[[@LINE-3]]:15}:"static_cast<void>("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:28-[[@LINE-4]]:28}:")"
+
+  for (x = 0; (return_four()), (x) ;) {}
+  // expected-warning@-1{{comma operator}}
+  // expected-note@-2{{cast expression to void}}
+  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:15-[[@LINE-3]]:15}:"static_cast<void>("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:30-[[@LINE-4]]:30}:")"
+}
+
+void test_increment() {
+  int x, y;
+  ++x, ++y;
+  // expected-warning@-1{{comma operator}}
+  // expected-note@-2{{cast expression to void}}
+  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:3-[[@LINE-3]]:3}:"static_cast<void>("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:6-[[@LINE-4]]:6}:")"
+}
+
+// Check for comma operator in conditions.
+void test_conditions(int x) {
+  x = (return_four(), x);
+  // expected-warning@-1{{comma operator}}
+  // expected-note@-2{{cast expression to void}}
+  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:8-[[@LINE-3]]:8}:"static_cast<void>("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:21-[[@LINE-4]]:21}:")"
+
+  int y = (return_four(), x);
+  // expected-warning@-1{{comma operator}}
+  // expected-note@-2{{cast expression to void}}
+  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:12-[[@LINE-3]]:12}:"static_cast<void>("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:25-[[@LINE-4]]:25}:")"
+
+  for (; return_four(), x;) {}
+  // expected-warning@-1{{comma operator}}
+  // expected-note@-2{{cast expression to void}}
+  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:10-[[@LINE-3]]:10}:"static_cast<void>("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:23-[[@LINE-4]]:23}:")"
+
+  while (return_four(), x) {}
+  // expected-warning@-1{{comma operator}}
+  // expected-note@-2{{cast expression to void}}
+  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:10-[[@LINE-3]]:10}:"static_cast<void>("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:23-[[@LINE-4]]:23}:")"
+
+  if (return_four(), x) {}
+  // expected-warning@-1{{comma operator}}
+  // expected-note@-2{{cast expression to void}}
+  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:7-[[@LINE-3]]:7}:"static_cast<void>("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:20-[[@LINE-4]]:20}:")"
+
+  do { } while (return_four(), x);
+  // expected-warning@-1{{comma operator}}
+  // expected-note@-2{{cast expression to void}}
+  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:17-[[@LINE-3]]:17}:"static_cast<void>("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:30-[[@LINE-4]]:30}:")"
+}
+
+// Nested comma operator with fix-its.
+void test_nested_fixits() {
+  return_four(), return_four(), return_four(), return_four();
+  // expected-warning@-1 3{{comma operator}}
+  // expected-note@-2 3{{cast expression to void}}
+  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:3-[[@LINE-3]]:3}:"static_cast<void>("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:16-[[@LINE-4]]:16}:")"
+  // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:18-[[@LINE-5]]:18}:"static_cast<void>("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:31-[[@LINE-6]]:31}:")"
+  // CHECK: fix-it:{{.*}}:{[[@LINE-7]]:33-[[@LINE-7]]:33}:"static_cast<void>("
+  // CHECK: fix-it:{{.*}}:{[[@LINE-8]]:46-[[@LINE-8]]:46}:")"
+}
+
+#ifdef __cplusplus
 class S2 {
 public:
   void advance();
@@ -45,7 +163,7 @@ public:
 };
 
 // Test overloaded operators
-void test2() {
+void test_overloaded_operator() {
   S2 x;
   int y;
   for (; y < 10; x++, y++) {}
@@ -67,22 +185,13 @@ void test2() {
   for (; y < 10; x ^= 5, ++y) {}
 }
 
-// Test nested comma operators
-void test3() {
-  int x1, x2, x3;
-  int y1, *y2 = 0, y3 = 5;
-  for (int z1 = 5, z2 = 4, z3 = 3; x1 <4; ++x1) {}
-}
-
 class Stream {
  public:
   Stream& operator<<(int);
 } cout;
 
-int return_four() { return 5; }
-
 // Confusing "," for "<<"
-void test4() {
+void test_stream() {
   cout << 5 << return_four();
   cout << 5, return_four();
   // expected-warning@-1{{comma operator}}
@@ -91,33 +200,11 @@ void test4() {
   // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:12-[[@LINE-4]]:12}:")"
 }
 
-// Confusing "," for "=="
-void test5() {
-  if (return_four(), 5) {}
-  // expected-warning@-1{{comma operator}}
-  // expected-note@-2{{cast expression to void}}
-  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:7-[[@LINE-3]]:7}:"static_cast<void>("
-  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:20-[[@LINE-4]]:20}:")"
-
-  if (return_four() == 5) {}
-}
-
-// Confusing "," for "+"
-int test6() {
-  return return_four(), return_four();
-  // expected-warning@-1{{comma operator}}
-  // expected-note@-2{{cast expression to void}}
-  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:10-[[@LINE-3]]:10}:"static_cast<void>("
-  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:23-[[@LINE-4]]:23}:")"
-
-  return return_four() + return_four();
-}
-
 void Concat(int);
 void Concat(int, int);
 
 // Testing extra parentheses in function call
-void test7() {
+void test_overloaded_function() {
   Concat((return_four() , 5));
   // expected-warning@-1{{comma operator}}
   // expected-note@-2{{cast expression to void}}
@@ -127,22 +214,6 @@ void test7() {
   Concat(return_four() , 5);
 }
 
-// Be sure to look through parentheses
-void test8() {
-  int x, y;
-  for (x = 0; return_four(), x;) {}
-  // expected-warning@-1{{comma operator}}
-  // expected-note@-2{{cast expression to void}}
-  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:15-[[@LINE-3]]:15}:"static_cast<void>("
-  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:28-[[@LINE-4]]:28}:")"
-
-  for (x = 0; (return_four()), (x) ;) {}
-  // expected-warning@-1{{comma operator}}
-  // expected-note@-2{{cast expression to void}}
-  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:15-[[@LINE-3]]:15}:"static_cast<void>("
-  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:30-[[@LINE-4]]:30}:")"
-}
-
 bool DoStuff();
 class S9 {
 public:
@@ -151,24 +222,15 @@ public:
 };
 
 // Ignore comma operator in for-loop initializations and increments.
-void test9() {
+void test_for_loop() {
   int x, y;
   for (x = 0, y = 5; x < y; ++x) {}
   for (x = 0; x < 10; DoStuff(), ++x) {}
   for (S9 s; s.More(); s.Advance(), ++x) {}
 }
 
-void test10() {
-  int x, y;
-  ++x, ++y;
-  // expected-warning@-1{{comma operator}}
-  // expected-note@-2{{cast expression to void}}
-  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:3-[[@LINE-3]]:3}:"static_cast<void>("
-  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:6-[[@LINE-4]]:6}:")"
-}
-
 // Ignore comma operator in templates.
-namespace test11 {
+namespace test_template {
 template <bool T>
 struct B { static const bool value = T; };
 
@@ -188,7 +250,7 @@ class Foo {
 const auto X = Foo<true_type>();
 }
 
-namespace test12 {
+namespace test_mutex {
 class Mutex {
  public:
   Mutex();
@@ -225,64 +287,13 @@ bool get_status() {
 }
 }
 
-// Check for comma operator in conditions.
-void test13(int x) {
-  x = (return_four(), x);
-  // expected-warning@-1{{comma operator}}
-  // expected-note@-2{{cast expression to void}}
-  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:8-[[@LINE-3]]:8}:"static_cast<void>("
-  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:21-[[@LINE-4]]:21}:")"
-
-  int y = (return_four(), x);
-  // expected-warning@-1{{comma operator}}
-  // expected-note@-2{{cast expression to void}}
-  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:12-[[@LINE-3]]:12}:"static_cast<void>("
-  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:25-[[@LINE-4]]:25}:")"
-
-  for (; return_four(), x;) {}
-  // expected-warning@-1{{comma operator}}
-  // expected-note@-2{{cast expression to void}}
-  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:10-[[@LINE-3]]:10}:"static_cast<void>("
-  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:23-[[@LINE-4]]:23}:")"
-
-  while (return_four(), x) {}
-  // expected-warning@-1{{comma operator}}
-  // expected-note@-2{{cast expression to void}}
-  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:10-[[@LINE-3]]:10}:"static_cast<void>("
-  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:23-[[@LINE-4]]:23}:")"
-
-  if (return_four(), x) {}
-  // expected-warning@-1{{comma operator}}
-  // expected-note@-2{{cast expression to void}}
-  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:7-[[@LINE-3]]:7}:"static_cast<void>("
-  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:20-[[@LINE-4]]:20}:")"
-
-  do { } while (return_four(), x);
-  // expected-warning@-1{{comma operator}}
-  // expected-note@-2{{cast expression to void}}
-  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:17-[[@LINE-3]]:17}:"static_cast<void>("
-  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:30-[[@LINE-4]]:30}:")"
-}
-
-// Nested comma operator with fix-its.
-void test14() {
-  return_four(), return_four(), return_four(), return_four();
-  // expected-warning@-1 3{{comma operator}}
-  // expected-note@-2 3{{cast expression to void}}
-  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:3-[[@LINE-3]]:3}:"static_cast<void>("
-  // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:16-[[@LINE-4]]:16}:")"
-  // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:18-[[@LINE-5]]:18}:"static_cast<void>("
-  // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:31-[[@LINE-6]]:31}:")"
-  // CHECK: fix-it:{{.*}}:{[[@LINE-7]]:33-[[@LINE-7]]:33}:"static_cast<void>("
-  // CHECK: fix-it:{{.*}}:{[[@LINE-8]]:46-[[@LINE-8]]:46}:")"
-}
-
 // PR39375 - test cast to void to silence warnings
 template <typename T>
-void test15() {
+void test_dependent_cast() {
   (void)42, 0;
   static_cast<void>(42), 0;
 
   (void)T{}, 0;
   static_cast<void>(T{}), 0;
 }
+#endif  // ifdef __cplusplus