[clang-tidy] Enhance clang-tidy misc-macro-repeated-side-effects...
Enhance clang-tidy misc-macro-repeated-side-effects to handle ? and : better. When ? is used in a macro, there are 2 possible control flow paths through the macro. These paths are tracked separately so problems can be detected properly. http://reviews.llvm.org/D10653 git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@241245 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Родитель
4763f24973
Коммит
d947af800e
|
@ -30,12 +30,12 @@ private:
|
|||
ClangTidyCheck &Check;
|
||||
Preprocessor &PP;
|
||||
|
||||
unsigned CountArgumentExpansions(const MacroInfo *MI,
|
||||
unsigned countArgumentExpansions(const MacroInfo *MI,
|
||||
const IdentifierInfo *Arg) const;
|
||||
|
||||
bool HasSideEffects(const Token *ResultArgToks) const;
|
||||
bool hasSideEffects(const Token *ResultArgToks) const;
|
||||
};
|
||||
} // namespace
|
||||
} // End of anonymous namespace.
|
||||
|
||||
void MacroRepeatedPPCallbacks::MacroExpands(const Token &MacroNameTok,
|
||||
const MacroDefinition &MD,
|
||||
|
@ -51,10 +51,10 @@ void MacroRepeatedPPCallbacks::MacroExpands(const Token &MacroNameTok,
|
|||
// making the macro too complex.
|
||||
if (std::find_if(
|
||||
MI->tokens().begin(), MI->tokens().end(), [](const Token &T) {
|
||||
return T.isOneOf(tok::question, tok::kw_if, tok::kw_else,
|
||||
tok::kw_switch, tok::kw_case, tok::kw_break,
|
||||
tok::kw_while, tok::kw_do, tok::kw_for,
|
||||
tok::kw_continue, tok::kw_goto, tok::kw_return);
|
||||
return T.isOneOf(tok::kw_if, tok::kw_else, tok::kw_switch,
|
||||
tok::kw_case, tok::kw_break, tok::kw_while,
|
||||
tok::kw_do, tok::kw_for, tok::kw_continue,
|
||||
tok::kw_goto, tok::kw_return);
|
||||
}) != MI->tokens().end())
|
||||
return;
|
||||
|
||||
|
@ -62,8 +62,8 @@ void MacroRepeatedPPCallbacks::MacroExpands(const Token &MacroNameTok,
|
|||
const IdentifierInfo *Arg = *(MI->arg_begin() + ArgNo);
|
||||
const Token *ResultArgToks = Args->getUnexpArgument(ArgNo);
|
||||
|
||||
if (HasSideEffects(ResultArgToks) &&
|
||||
CountArgumentExpansions(MI, Arg) >= 2) {
|
||||
if (hasSideEffects(ResultArgToks) &&
|
||||
countArgumentExpansions(MI, Arg) >= 2) {
|
||||
Check.diag(ResultArgToks->getLocation(),
|
||||
"side effects in the %ordinal0 macro argument '%1' are "
|
||||
"repeated in macro expansion")
|
||||
|
@ -75,12 +75,39 @@ void MacroRepeatedPPCallbacks::MacroExpands(const Token &MacroNameTok,
|
|||
}
|
||||
}
|
||||
|
||||
unsigned MacroRepeatedPPCallbacks::CountArgumentExpansions(
|
||||
unsigned MacroRepeatedPPCallbacks::countArgumentExpansions(
|
||||
const MacroInfo *MI, const IdentifierInfo *Arg) const {
|
||||
unsigned CountInMacro = 0;
|
||||
// Current argument count. When moving forward to a different control-flow
|
||||
// path this can decrease.
|
||||
unsigned Current = 0;
|
||||
// Max argument count.
|
||||
unsigned Max = 0;
|
||||
bool SkipParen = false;
|
||||
int SkipParenCount = 0;
|
||||
// Has a __builtin_constant_p been found?
|
||||
bool FoundBuiltin = false;
|
||||
// Count when "?" is reached. The "Current" will get this value when the ":"
|
||||
// is reached.
|
||||
std::stack<unsigned, SmallVector<unsigned, 8>> CountAtQuestion;
|
||||
for (const auto &T : MI->tokens()) {
|
||||
// The result of __builtin_constant_p(x) is 0 if x is a macro argument
|
||||
// with side effects. If we see a __builtin_constant_p(x) followed by a
|
||||
// "?" "&&" or "||", then we need to reason about control flow to report
|
||||
// warnings correctly. Until such reasoning is added, bail out when this
|
||||
// happens.
|
||||
if (FoundBuiltin && T.isOneOf(tok::question, tok::ampamp, tok::pipepipe))
|
||||
return Max;
|
||||
|
||||
// Handling of ? and :.
|
||||
if (T.is(tok::question)) {
|
||||
CountAtQuestion.push(Current);
|
||||
} else if (T.is(tok::colon)) {
|
||||
if (CountAtQuestion.empty())
|
||||
return 0;
|
||||
Current = CountAtQuestion.top();
|
||||
CountAtQuestion.pop();
|
||||
}
|
||||
|
||||
// If current token is a parenthesis, skip it.
|
||||
if (SkipParen) {
|
||||
if (T.is(tok::l_paren))
|
||||
|
@ -97,9 +124,11 @@ unsigned MacroRepeatedPPCallbacks::CountArgumentExpansions(
|
|||
if (TII == nullptr)
|
||||
continue;
|
||||
|
||||
// If a builtin is found within the macro definition, skip next
|
||||
// parenthesis.
|
||||
if (TII->getBuiltinID() != 0) {
|
||||
// If a __builtin_constant_p is found within the macro definition, don't
|
||||
// count arguments inside the parentheses and remember that it has been
|
||||
// seen in case there are "?", "&&" or "||" operators later.
|
||||
if (TII->getBuiltinID() == Builtin::BI__builtin_constant_p) {
|
||||
FoundBuiltin = true;
|
||||
SkipParen = true;
|
||||
continue;
|
||||
}
|
||||
|
@ -114,13 +143,16 @@ unsigned MacroRepeatedPPCallbacks::CountArgumentExpansions(
|
|||
}
|
||||
|
||||
// Count argument.
|
||||
if (TII == Arg)
|
||||
CountInMacro++;
|
||||
if (TII == Arg) {
|
||||
Current++;
|
||||
if (Current > Max)
|
||||
Max = Current;
|
||||
}
|
||||
}
|
||||
return CountInMacro;
|
||||
return Max;
|
||||
}
|
||||
|
||||
bool MacroRepeatedPPCallbacks::HasSideEffects(
|
||||
bool MacroRepeatedPPCallbacks::hasSideEffects(
|
||||
const Token *ResultArgToks) const {
|
||||
for (; ResultArgToks->isNot(tok::eof); ++ResultArgToks) {
|
||||
if (ResultArgToks->isOneOf(tok::plusplus, tok::minusminus))
|
||||
|
|
|
@ -22,6 +22,21 @@ void bad(int ret, int a, int b) {
|
|||
}
|
||||
|
||||
|
||||
#define MIN(A,B) ((A) < (B) ? (A) : (B)) // single ?:
|
||||
#define LIMIT(X,A,B) ((X) < (A) ? (A) : ((X) > (B) ? (B) : (X))) // two ?:
|
||||
void question(int x) {
|
||||
MIN(x++, 12);
|
||||
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: side effects in the 1st macro argument 'A'
|
||||
MIN(34, x++);
|
||||
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: side effects in the 2nd macro argument 'B'
|
||||
LIMIT(x++, 0, 100);
|
||||
// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: side effects in the 1st macro argument 'X'
|
||||
LIMIT(20, x++, 100);
|
||||
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: side effects in the 2nd macro argument 'A'
|
||||
LIMIT(20, 0, x++);
|
||||
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: side effects in the 3rd macro argument 'B'
|
||||
}
|
||||
|
||||
// False positive: Repeated side effects is intentional.
|
||||
// It is hard to know when it's done by intention so right now we warn.
|
||||
#define UNROLL(A) {A A}
|
||||
|
@ -59,26 +74,29 @@ void large(int a) {
|
|||
// CHECK-MESSAGES: :[[@LINE-1]]:70: warning: side effects in the 21st macro argument 'u'
|
||||
}
|
||||
|
||||
// Builtins and macros should not be counted.
|
||||
#define builtinA(x) (__builtin_constant_p(x) + (x))
|
||||
#define builtinB(x) (__builtin_constant_p(x) + (x) + (x))
|
||||
#define macroA(x) (builtinA(x) + (x))
|
||||
#define macroB(x) (builtinA(x) + (x) + (x))
|
||||
// Passing macro argument as argument to __builtin_constant_p and macros.
|
||||
#define builtinbad(x) (__builtin_constant_p(x) + (x) + (x))
|
||||
#define builtingood1(x) (__builtin_constant_p(x) + (x))
|
||||
#define builtingood2(x) ((__builtin_constant_p(x) && (x)) || (x))
|
||||
#define macrobad(x) (builtingood1(x) + (x) + (x))
|
||||
#define macrogood(x) (builtingood1(x) + (x))
|
||||
void builtins(int ret, int a) {
|
||||
ret += builtinA(a++);
|
||||
ret += builtinB(a++);
|
||||
ret += builtinbad(a++);
|
||||
// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: side effects in the 1st macro argument 'x'
|
||||
|
||||
ret += builtingood1(a++);
|
||||
ret += builtingood2(a++);
|
||||
|
||||
ret += macrobad(a++);
|
||||
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: side effects in the 1st macro argument 'x'
|
||||
ret += macroA(a++);
|
||||
ret += macroB(a++);
|
||||
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: side effects in the 1st macro argument 'x'
|
||||
|
||||
ret += macrogood(a++);
|
||||
}
|
||||
|
||||
// Bail out for conditionals.
|
||||
#define condA(x,y) ((x)>(y)?(x):(y))
|
||||
#define condB(x,y) if(x) {x=y;} else {x=y + 1;}
|
||||
void conditionals(int ret, int a, int b)
|
||||
void conditionals(int a, int b)
|
||||
{
|
||||
ret += condA(a++, b++);
|
||||
condB(a, b++);
|
||||
}
|
||||
|
||||
|
|
Загрузка…
Ссылка в новой задаче