r345228 - [Sema] Fix -Wcomma for C89
Richard Trieu via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 24 18:08:01 PDT 2018
Author: rtrieu
Date: Wed Oct 24 18:08:00 2018
New Revision: 345228
URL: http://llvm.org/viewvc/llvm-project?rev=345228&view=rev
Log:
[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
Modified:
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/test/SemaCXX/warn-comma-operator.cpp
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=345228&r1=345227&r2=345228&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Oct 24 18:08:00 2018
@@ -11309,8 +11309,11 @@ void Sema::DiagnoseCommaOperator(const E
// 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 ||
Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=345228&r1=345227&r2=345228&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Wed Oct 24 18:08:00 2018
@@ -551,8 +551,9 @@ Sema::ActOnIfStmt(SourceLocation IfLoc,
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,
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);
Modified: cfe/trunk/test/SemaCXX/warn-comma-operator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-comma-operator.cpp?rev=345228&r1=345227&r2=345228&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-comma-operator.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-comma-operator.cpp Wed Oct 24 18:08:00 2018
@@ -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 at -1{{comma operator}}
+ // expected-note at -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 at -1{{comma operator}}
+ // expected-note at -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 at -1{{comma operator}}
+ // expected-note at -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 at -1{{comma operator}}
+ // expected-note at -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 at -1{{comma operator}}
+ // expected-note at -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 at -1{{comma operator}}
+ // expected-note at -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 at -1{{comma operator}}
+ // expected-note at -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 at -1{{comma operator}}
+ // expected-note at -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 at -1{{comma operator}}
+ // expected-note at -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 at -1{{comma operator}}
+ // expected-note at -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 at -1{{comma operator}}
+ // expected-note at -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 at -1 3{{comma operator}}
+ // expected-note at -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 at -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 at -1{{comma operator}}
- // expected-note at -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 at -1{{comma operator}}
- // expected-note at -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 at -1{{comma operator}}
// expected-note at -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 at -1{{comma operator}}
- // expected-note at -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 at -1{{comma operator}}
- // expected-note at -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 at -1{{comma operator}}
- // expected-note at -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 at -1{{comma operator}}
- // expected-note at -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 at -1{{comma operator}}
- // expected-note at -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 at -1{{comma operator}}
- // expected-note at -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 at -1{{comma operator}}
- // expected-note at -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 at -1{{comma operator}}
- // expected-note at -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 at -1{{comma operator}}
- // expected-note at -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 at -1 3{{comma operator}}
- // expected-note at -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
More information about the cfe-commits
mailing list