[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
llvm-svn: 345228
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 98025ca..66194c6 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11309,8 +11309,11 @@
// 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/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 16ed9ed..7fc89bf 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -551,8 +551,9 @@
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 @@
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/clang/test/SemaCXX/warn-comma-operator.cpp b/clang/test/SemaCXX/warn-comma-operator.cpp
index 75ef452..0ed127b 100644
--- a/clang/test/SemaCXX/warn-comma-operator.cpp
+++ b/clang/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,76 +31,18 @@
for (; y < 10; x ^= 5, ++y) {}
}
-class S2 {
-public:
- void advance();
-
- S2 operator++();
- S2 operator++(int);
- S2 operator--();
- S2 operator--(int);
- S2 operator=(int);
- S2 operator*=(int);
- S2 operator/=(int);
- S2 operator%=(int);
- S2 operator+=(int);
- S2 operator-=(int);
- S2 operator<<=(int);
- S2 operator>>=(int);
- S2 operator&=(int);
- S2 operator|=(int);
- S2 operator^=(int);
-};
-
-// Test overloaded operators
-void test2() {
- S2 x;
- int y;
- for (; y < 10; x++, y++) {}
- for (; y < 10; ++x, y++) {}
- for (; y < 10; x++, ++y) {}
- for (; y < 10; ++x, ++y) {}
- for (; y < 10; x--, ++y) {}
- for (; y < 10; --x, ++y) {}
- for (; y < 10; x = 5, ++y) {}
- for (; y < 10; x *= 5, ++y) {}
- for (; y < 10; x /= 5, ++y) {}
- for (; y < 10; x %= 5, ++y) {}
- for (; y < 10; x += 5, ++y) {}
- for (; y < 10; x -= 5, ++y) {}
- for (; y < 10; x <<= 5, ++y) {}
- for (; y < 10; x >>= 5, ++y) {}
- for (; y < 10; x &= 5, ++y) {}
- for (; y < 10; x |= 5, ++y) {}
- for (; y < 10; x ^= 5, ++y) {}
-}
-
// Test nested comma operators
-void test3() {
+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) {}
-}
-
-class Stream {
- public:
- Stream& operator<<(int);
-} cout;
-
-int return_four() { return 5; }
-
-// Confusing "," for "<<"
-void test4() {
- cout << 5 << return_four();
- cout << 5, return_four();
- // 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]]:12-[[@LINE-4]]:12}:")"
+#endif
}
// Confusing "," for "=="
-void test5() {
+void test_compare() {
if (return_four(), 5) {}
// expected-warning@-1{{comma operator}}
// expected-note@-2{{cast expression to void}}
@@ -103,7 +53,7 @@
}
// Confusing "," for "+"
-int test6() {
+int test_plus() {
return return_four(), return_four();
// expected-warning@-1{{comma operator}}
// expected-note@-2{{cast expression to void}}
@@ -113,22 +63,8 @@
return return_four() + return_four();
}
-void Concat(int);
-void Concat(int, int);
-
-// Testing extra parentheses in function call
-void test7() {
- Concat((return_four() , 5));
- // expected-warning@-1{{comma operator}}
- // expected-note@-2{{cast expression to void}}
- // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:11-[[@LINE-3]]:11}:"static_cast<void>("
- // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:24-[[@LINE-4]]:24}:")"
-
- Concat(return_four() , 5);
-}
-
// Be sure to look through parentheses
-void test8() {
+void test_parentheses() {
int x, y;
for (x = 0; return_four(), x;) {}
// expected-warning@-1{{comma operator}}
@@ -143,22 +79,7 @@
// CHECK: fix-it:{{.*}}:{[[@LINE-4]]:30-[[@LINE-4]]:30}:")"
}
-bool DoStuff();
-class S9 {
-public:
- bool Advance();
- bool More();
-};
-
-// Ignore comma operator in for-loop initializations and increments.
-void test9() {
- 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() {
+void test_increment() {
int x, y;
++x, ++y;
// expected-warning@-1{{comma operator}}
@@ -167,66 +88,8 @@
// CHECK: fix-it:{{.*}}:{[[@LINE-4]]:6-[[@LINE-4]]:6}:")"
}
-// Ignore comma operator in templates.
-namespace test11 {
-template <bool T>
-struct B { static const bool value = T; };
-
-typedef B<true> true_type;
-typedef B<false> false_type;
-
-template <bool...>
-struct bool_seq;
-
-template <typename... xs>
-class Foo {
- typedef bool_seq<(xs::value, true)...> all_true;
- typedef bool_seq<(xs::value, false)...> all_false;
- typedef bool_seq<xs::value...> seq;
-};
-
-const auto X = Foo<true_type>();
-}
-
-namespace test12 {
-class Mutex {
- public:
- Mutex();
- ~Mutex();
-};
-class MutexLock {
-public:
- MutexLock(Mutex &);
- MutexLock();
- ~MutexLock();
-};
-class BuiltinMutex {
- Mutex M;
-};
-Mutex StatusMutex;
-bool Status;
-
-bool get_status() {
- return (MutexLock(StatusMutex), Status);
- // expected-warning@-1{{comma operator}}
- // expected-note@-2{{cast expression to void}}
- // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:11-[[@LINE-3]]:11}:"static_cast<void>("
- // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:33-[[@LINE-4]]:33}:")"
- return (MutexLock(), Status);
- // expected-warning@-1{{comma operator}}
- // expected-note@-2{{cast expression to void}}
- // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:11-[[@LINE-3]]:11}:"static_cast<void>("
- // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:22-[[@LINE-4]]:22}:")"
- return (BuiltinMutex(), Status);
- // expected-warning@-1{{comma operator}}
- // expected-note@-2{{cast expression to void}}
- // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:11-[[@LINE-3]]:11}:"static_cast<void>("
- // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:25-[[@LINE-4]]:25}:")"
-}
-}
-
// Check for comma operator in conditions.
-void test13(int x) {
+void test_conditions(int x) {
x = (return_four(), x);
// expected-warning@-1{{comma operator}}
// expected-note@-2{{cast expression to void}}
@@ -265,7 +128,7 @@
}
// Nested comma operator with fix-its.
-void test14() {
+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}}
@@ -277,12 +140,160 @@
// CHECK: fix-it:{{.*}}:{[[@LINE-8]]:46-[[@LINE-8]]:46}:")"
}
+#ifdef __cplusplus
+class S2 {
+public:
+ void advance();
+
+ S2 operator++();
+ S2 operator++(int);
+ S2 operator--();
+ S2 operator--(int);
+ S2 operator=(int);
+ S2 operator*=(int);
+ S2 operator/=(int);
+ S2 operator%=(int);
+ S2 operator+=(int);
+ S2 operator-=(int);
+ S2 operator<<=(int);
+ S2 operator>>=(int);
+ S2 operator&=(int);
+ S2 operator|=(int);
+ S2 operator^=(int);
+};
+
+// Test overloaded operators
+void test_overloaded_operator() {
+ S2 x;
+ int y;
+ for (; y < 10; x++, y++) {}
+ for (; y < 10; ++x, y++) {}
+ for (; y < 10; x++, ++y) {}
+ for (; y < 10; ++x, ++y) {}
+ for (; y < 10; x--, ++y) {}
+ for (; y < 10; --x, ++y) {}
+ for (; y < 10; x = 5, ++y) {}
+ for (; y < 10; x *= 5, ++y) {}
+ for (; y < 10; x /= 5, ++y) {}
+ for (; y < 10; x %= 5, ++y) {}
+ for (; y < 10; x += 5, ++y) {}
+ for (; y < 10; x -= 5, ++y) {}
+ for (; y < 10; x <<= 5, ++y) {}
+ for (; y < 10; x >>= 5, ++y) {}
+ for (; y < 10; x &= 5, ++y) {}
+ for (; y < 10; x |= 5, ++y) {}
+ for (; y < 10; x ^= 5, ++y) {}
+}
+
+class Stream {
+ public:
+ Stream& operator<<(int);
+} cout;
+
+// Confusing "," for "<<"
+void test_stream() {
+ cout << 5 << return_four();
+ cout << 5, return_four();
+ // 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]]:12-[[@LINE-4]]:12}:")"
+}
+
+void Concat(int);
+void Concat(int, int);
+
+// Testing extra parentheses in function call
+void test_overloaded_function() {
+ Concat((return_four() , 5));
+ // expected-warning@-1{{comma operator}}
+ // expected-note@-2{{cast expression to void}}
+ // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:11-[[@LINE-3]]:11}:"static_cast<void>("
+ // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:24-[[@LINE-4]]:24}:")"
+
+ Concat(return_four() , 5);
+}
+
+bool DoStuff();
+class S9 {
+public:
+ bool Advance();
+ bool More();
+};
+
+// Ignore comma operator in for-loop initializations and increments.
+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) {}
+}
+
+// Ignore comma operator in templates.
+namespace test_template {
+template <bool T>
+struct B { static const bool value = T; };
+
+typedef B<true> true_type;
+typedef B<false> false_type;
+
+template <bool...>
+struct bool_seq;
+
+template <typename... xs>
+class Foo {
+ typedef bool_seq<(xs::value, true)...> all_true;
+ typedef bool_seq<(xs::value, false)...> all_false;
+ typedef bool_seq<xs::value...> seq;
+};
+
+const auto X = Foo<true_type>();
+}
+
+namespace test_mutex {
+class Mutex {
+ public:
+ Mutex();
+ ~Mutex();
+};
+class MutexLock {
+public:
+ MutexLock(Mutex &);
+ MutexLock();
+ ~MutexLock();
+};
+class BuiltinMutex {
+ Mutex M;
+};
+Mutex StatusMutex;
+bool Status;
+
+bool get_status() {
+ return (MutexLock(StatusMutex), Status);
+ // expected-warning@-1{{comma operator}}
+ // expected-note@-2{{cast expression to void}}
+ // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:11-[[@LINE-3]]:11}:"static_cast<void>("
+ // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:33-[[@LINE-4]]:33}:")"
+ return (MutexLock(), Status);
+ // expected-warning@-1{{comma operator}}
+ // expected-note@-2{{cast expression to void}}
+ // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:11-[[@LINE-3]]:11}:"static_cast<void>("
+ // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:22-[[@LINE-4]]:22}:")"
+ return (BuiltinMutex(), Status);
+ // expected-warning@-1{{comma operator}}
+ // expected-note@-2{{cast expression to void}}
+ // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:11-[[@LINE-3]]:11}:"static_cast<void>("
+ // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:25-[[@LINE-4]]:25}:")"
+}
+}
+
// 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