зеркало из https://github.com/microsoft/clang.git
Add -Wcomma warning to Clang.
-Wcomma will detect and warn on most uses of the builtin comma operator. It currently whitelists the first and third statements of the for-loop. For other cases, the warning can be silenced by casting the first operand of the comma operator to void. Differential Revision: http://reviews.llvm.org/D3976 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@261278 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Richard Trieu
Родитель
d4539e33f0
Коммит
ba3bb7db1d
|
|
@ -74,6 +74,10 @@ def warn_infinite_recursive_function : Warning<
|
|||
"all paths through this function will call itself">,
|
||||
InGroup<InfiniteRecursion>, DefaultIgnore;
|
||||
|
||||
def warn_comma_operator : Warning<"possible misuse of comma operator here">,
|
||||
InGroup<DiagGroup<"comma">>, DefaultIgnore;
|
||||
def note_cast_to_void : Note<"cast expression to void to silence warning">;
|
||||
|
||||
// Constant expressions
|
||||
def err_expr_not_ice : Error<
|
||||
"expression is not an %select{integer|integral}0 constant expression">;
|
||||
|
|
|
|||
|
|
@ -3971,6 +3971,8 @@ public:
|
|||
ExprResult CreateBuiltinBinOp(SourceLocation OpLoc, BinaryOperatorKind Opc,
|
||||
Expr *LHSExpr, Expr *RHSExpr);
|
||||
|
||||
void DiagnoseCommaOperator(const Expr *LHS, SourceLocation Loc);
|
||||
|
||||
/// ActOnConditionalOp - Parse a ?: operation. Note that 'LHS' may be null
|
||||
/// in the case of a the GNU conditional expr extension.
|
||||
ExprResult ActOnConditionalOp(SourceLocation QuestionLoc,
|
||||
|
|
|
|||
|
|
@ -9867,6 +9867,67 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS,
|
|||
? LHSType : LHSType.getUnqualifiedType());
|
||||
}
|
||||
|
||||
// Only ignore explicit casts to void.
|
||||
static bool IgnoreCommaOperand(const Expr *E) {
|
||||
E = E->IgnoreParens();
|
||||
|
||||
if (const CastExpr *CE = dyn_cast<CastExpr>(E)) {
|
||||
if (CE->getCastKind() == CK_ToVoid) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
// Look for instances where it is likely the comma operator is confused with
|
||||
// another operator. There is a whitelist of acceptable expressions for the
|
||||
// left hand side of the comma operator, otherwise emit a warning.
|
||||
void Sema::DiagnoseCommaOperator(const Expr *LHS, SourceLocation Loc) {
|
||||
// No warnings in macros
|
||||
if (Loc.isMacroID())
|
||||
return;
|
||||
|
||||
// Don't warn in template instantiations.
|
||||
if (!ActiveTemplateInstantiations.empty())
|
||||
return;
|
||||
|
||||
// Scope isn't fine-grained enough to whitelist the specific cases, so
|
||||
// instead, skip more than needed, then call back into here with the
|
||||
// CommaVisitor in SemaStmt.cpp.
|
||||
// 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.
|
||||
const unsigned ForIncreamentFlags =
|
||||
Scope::ControlScope | Scope::ContinueScope | Scope::BreakScope;
|
||||
const unsigned ForInitFlags = Scope::ControlScope | Scope::DeclScope;
|
||||
const unsigned ScopeFlags = getCurScope()->getFlags();
|
||||
if ((ScopeFlags & ForIncrementFlags) == ForIncrementFlags ||
|
||||
(ScopeFlags & ForInitFlags) == ForInitFlags)
|
||||
return;
|
||||
|
||||
// If there are multiple comma operators used together, get the RHS of the
|
||||
// of the comma operator as the LHS.
|
||||
while (const BinaryOperator *BO = dyn_cast<BinaryOperator>(LHS)) {
|
||||
if (BO->getOpcode() != BO_Comma)
|
||||
break;
|
||||
LHS = BO->getRHS();
|
||||
}
|
||||
|
||||
// Only allow some expressions on LHS to not warn.
|
||||
if (IgnoreCommaOperand(LHS))
|
||||
return;
|
||||
|
||||
Diag(Loc, diag::warn_comma_operator);
|
||||
Diag(LHS->getLocStart(), diag::note_cast_to_void)
|
||||
<< LHS->getSourceRange()
|
||||
<< FixItHint::CreateInsertion(LHS->getLocStart(),
|
||||
LangOpts.CPlusPlus ? "static_cast<void>("
|
||||
: "(void)(")
|
||||
<< FixItHint::CreateInsertion(PP.getLocForEndOfToken(LHS->getLocEnd()),
|
||||
")");
|
||||
}
|
||||
|
||||
// C99 6.5.17
|
||||
static QualType CheckCommaOperands(Sema &S, ExprResult &LHS, ExprResult &RHS,
|
||||
SourceLocation Loc) {
|
||||
|
|
@ -9896,6 +9957,9 @@ static QualType CheckCommaOperands(Sema &S, ExprResult &LHS, ExprResult &RHS,
|
|||
diag::err_incomplete_type);
|
||||
}
|
||||
|
||||
if (!S.getDiagnostics().isIgnored(diag::warn_comma_operator, Loc))
|
||||
S.DiagnoseCommaOperator(LHS.get(), Loc);
|
||||
|
||||
return RHS.get()->getType();
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -488,6 +488,20 @@ StmtResult Sema::ActOnAttributedStmt(SourceLocation AttrLoc,
|
|||
return LS;
|
||||
}
|
||||
|
||||
namespace {
|
||||
class CommaVisitor : public EvaluatedExprVisitor<CommaVisitor> {
|
||||
typedef EvaluatedExprVisitor<CommaVisitor> Inherited;
|
||||
Sema &SemaRef;
|
||||
public:
|
||||
CommaVisitor(Sema &SemaRef) : Inherited(SemaRef.Context), SemaRef(SemaRef) {}
|
||||
void VisitBinaryOperator(BinaryOperator *E) {
|
||||
if (E->getOpcode() == BO_Comma)
|
||||
SemaRef.DiagnoseCommaOperator(E->getLHS(), E->getExprLoc());
|
||||
EvaluatedExprVisitor<CommaVisitor>::VisitBinaryOperator(E);
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
StmtResult
|
||||
Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, Decl *CondVar,
|
||||
Stmt *thenStmt, SourceLocation ElseLoc,
|
||||
|
|
@ -502,6 +516,11 @@ Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, Decl *CondVar,
|
|||
}
|
||||
Expr *ConditionExpr = CondResult.getAs<Expr>();
|
||||
if (ConditionExpr) {
|
||||
|
||||
if (!Diags.isIgnored(diag::warn_comma_operator,
|
||||
ConditionExpr->getExprLoc()))
|
||||
CommaVisitor(*this).Visit(ConditionExpr);
|
||||
|
||||
DiagnoseUnusedExprResult(thenStmt);
|
||||
|
||||
if (!elseStmt) {
|
||||
|
|
@ -1240,6 +1259,10 @@ Sema::ActOnWhileStmt(SourceLocation WhileLoc, FullExprArg Cond,
|
|||
return StmtError();
|
||||
CheckBreakContinueBinding(ConditionExpr);
|
||||
|
||||
if (ConditionExpr &&
|
||||
!Diags.isIgnored(diag::warn_comma_operator, ConditionExpr->getExprLoc()))
|
||||
CommaVisitor(*this).Visit(ConditionExpr);
|
||||
|
||||
DiagnoseUnusedExprResult(Body);
|
||||
|
||||
if (isa<NullStmt>(Body))
|
||||
|
|
@ -1642,6 +1665,11 @@ Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc,
|
|||
return StmtError();
|
||||
}
|
||||
|
||||
if (SecondResult.get() &&
|
||||
!Diags.isIgnored(diag::warn_comma_operator,
|
||||
SecondResult.get()->getExprLoc()))
|
||||
CommaVisitor(*this).Visit(SecondResult.get());
|
||||
|
||||
Expr *Third = third.release().getAs<Expr>();
|
||||
|
||||
DiagnoseUnusedExprResult(First);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,278 @@
|
|||
// 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
|
||||
|
||||
// Test builtin operators
|
||||
void test1() {
|
||||
int x = 0, y = 0;
|
||||
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 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() {
|
||||
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() {
|
||||
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}:")"
|
||||
}
|
||||
|
||||
// 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() {
|
||||
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() {
|
||||
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:
|
||||
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() {
|
||||
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 {
|
||||
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) {
|
||||
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}:")"
|
||||
}
|
||||
Загрузка…
Ссылка в новой задаче