clang-tidy: Add checker that warn when macro argument with side effects is repeated in the macro

llvm-svn: 239909
This commit is contained in:
Daniel Marjamaki 2015-06-17 14:19:35 +00:00
parent b19f5cfee6
commit 71c9257f07
5 changed files with 263 additions and 0 deletions

View File

@ -8,6 +8,7 @@ add_clang_library(clangTidyMiscModule
InaccurateEraseCheck.cpp
InefficientAlgorithmCheck.cpp
MacroParenthesesCheck.cpp
MacroRepeatedSideEffectsCheck.cpp
MiscTidyModule.cpp
NoexceptMoveConstructorCheck.cpp
StaticAssertCheck.cpp

View File

@ -0,0 +1,143 @@
//===--- MacroRepeatedSideEffectsCheck.cpp - clang-tidy--------------------===//
//
// The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
//===----------------------------------------------------------------------===//
#include "MacroRepeatedSideEffectsCheck.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Lex/PPCallbacks.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Lex/MacroArgs.h"
namespace clang {
namespace tidy {
namespace misc {
namespace {
class MacroRepeatedPPCallbacks : public PPCallbacks {
public:
MacroRepeatedPPCallbacks(ClangTidyCheck &Check, SourceManager &SM,
Preprocessor &PP)
: Check(Check), SM(SM), PP(PP) {}
void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD,
SourceRange Range, const MacroArgs *Args) override;
private:
ClangTidyCheck &Check;
SourceManager &SM;
Preprocessor &PP;
unsigned CountArgumentExpansions(const MacroInfo *MI,
const IdentifierInfo *Arg) const;
bool HasSideEffects(const Token *ResultArgToks) const;
};
} // namespace
void MacroRepeatedPPCallbacks::MacroExpands(const Token &MacroNameTok,
const MacroDefinition &MD,
SourceRange Range,
const MacroArgs *Args) {
// Ignore macro argument expansions.
if (!Range.getBegin().isFileID())
return;
const MacroInfo *MI = MD.getMacroInfo();
// Bail out if the contents of the macro are containing keywords that are
// 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);
}) != MI->tokens().end())
return;
for (unsigned ArgNo = 0U; ArgNo < MI->getNumArgs(); ++ArgNo) {
const IdentifierInfo *Arg = *(MI->arg_begin() + ArgNo);
const Token *ResultArgToks = Args->getUnexpArgument(ArgNo);
if (HasSideEffects(ResultArgToks) &&
CountArgumentExpansions(MI, Arg) >= 2) {
Check.diag(ResultArgToks->getLocation(),
"side effects in the %ordinal0 macro argument '%1' are "
"repeated in macro expansion")
<< (ArgNo + 1) << Arg->getName();
Check.diag(MI->getDefinitionLoc(), "macro %0 defined here",
DiagnosticIDs::Note)
<< MacroNameTok.getIdentifierInfo();
}
}
}
unsigned MacroRepeatedPPCallbacks::CountArgumentExpansions(
const MacroInfo *MI, const IdentifierInfo *Arg) const {
unsigned CountInMacro = 0;
bool SkipParen = false;
int SkipParenCount = 0;
for (const auto &T : MI->tokens()) {
// If current token is a parenthesis, skip it.
if (SkipParen) {
if (T.is(tok::l_paren))
SkipParenCount++;
else if (T.is(tok::r_paren))
SkipParenCount--;
SkipParen = (SkipParenCount != 0);
if (SkipParen)
continue;
}
IdentifierInfo *TII = T.getIdentifierInfo();
// If not existent, skip it.
if (TII == nullptr)
continue;
// If a builtin is found within the macro definition, skip next
// parenthesis.
if (TII->getBuiltinID() != 0) {
SkipParen = true;
continue;
}
// If another macro is found within the macro definition, skip the macro
// and the eventual arguments.
if (TII->hasMacroDefinition()) {
const MacroInfo *M = PP.getMacroDefinition(TII).getMacroInfo();
if (M != nullptr && M->isFunctionLike())
SkipParen = true;
continue;
}
// Count argument.
if (TII == Arg)
CountInMacro++;
}
return CountInMacro;
}
bool MacroRepeatedPPCallbacks::HasSideEffects(
const Token *ResultArgToks) const {
for (; ResultArgToks->isNot(tok::eof); ++ResultArgToks) {
if (ResultArgToks->isOneOf(tok::plusplus, tok::minusminus))
return true;
}
return false;
}
void MacroRepeatedSideEffectsCheck::registerPPCallbacks(
CompilerInstance &Compiler) {
Compiler.getPreprocessor().addPPCallbacks(
::llvm::make_unique<MacroRepeatedPPCallbacks>(
*this, Compiler.getSourceManager(), Compiler.getPreprocessor()));
}
} // namespace misc
} // namespace tidy
} // namespace clang

View File

@ -0,0 +1,32 @@
//===--- MacroRepeatedSideEffectsCheck.h - clang-tidy -----------*- C++ -*-===//
//
// The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
//===----------------------------------------------------------------------===//
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MACRO_REPEATED_SIDE_EFFECTS_CHECK_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MACRO_REPEATED_SIDE_EFFECTS_CHECK_H
#include "../ClangTidy.h"
namespace clang {
namespace tidy {
namespace misc {
/// \brief Checks for repeated argument with side effects in macros.
///
class MacroRepeatedSideEffectsCheck : public ClangTidyCheck {
public:
MacroRepeatedSideEffectsCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
void registerPPCallbacks(CompilerInstance &Compiler) override;
};
} // namespace misc
} // namespace tidy
} // namespace clang
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MACRO_REPEATED_SIDE_EFFECTS_CHECK_H

View File

@ -17,6 +17,7 @@
#include "InaccurateEraseCheck.h"
#include "InefficientAlgorithmCheck.h"
#include "MacroParenthesesCheck.h"
#include "MacroRepeatedSideEffectsCheck.h"
#include "NoexceptMoveConstructorCheck.h"
#include "StaticAssertCheck.h"
#include "SwappedArgumentsCheck.h"
@ -45,6 +46,8 @@ public:
"misc-inefficient-algorithm");
CheckFactories.registerCheck<MacroParenthesesCheck>(
"misc-macro-parentheses");
CheckFactories.registerCheck<MacroRepeatedSideEffectsCheck>(
"misc-macro-repeated-side-effects");
CheckFactories.registerCheck<NoexceptMoveConstructorCheck>(
"misc-noexcept-move-constructor");
CheckFactories.registerCheck<StaticAssertCheck>(

View File

@ -0,0 +1,84 @@
// RUN: $(dirname %s)/check_clang_tidy.sh %s misc-macro-repeated-side-effects %t
// REQUIRES: shell
#define badA(x,y) ((x)+((x)+(y))+(y))
void bad(int ret, int a, int b) {
ret = badA(a++, b);
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: side effects in the 1st macro argument 'x' are repeated in macro expansion [misc-macro-repeated-side-effects]
ret = badA(++a, b);
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: side effects in the 1st macro argument 'x'
ret = badA(a--, b);
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: side effects in the 1st macro argument 'x'
ret = badA(--a, b);
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: side effects in the 1st macro argument 'x'
ret = badA(a, b++);
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: side effects in the 2nd macro argument 'y'
ret = badA(a, ++b);
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: side effects in the 2nd macro argument 'y'
ret = badA(a, b--);
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: side effects in the 2nd macro argument 'y'
ret = badA(a, --b);
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: side effects in the 2nd macro argument 'y'
}
// 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}
void fp1(int i) {
UNROLL({ i++; });
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: side effects in the 1st macro argument 'A'
}
// Do not produce a false positive on a strchr() macro. Explanation; Currently the '?'
// triggers the test to bail out, because it cannot evaluate __builtin_constant_p(c).
# define strchrs(s, c) \
(__extension__ (__builtin_constant_p (c) && !__builtin_constant_p (s) \
&& (c) == '\0' \
? (char *) __rawmemchr (s, c) \
: __builtin_strchr (s, c)))
char* __rawmemchr(char* a, char b) {
return a;
}
void pass(char* pstr, char ch) {
strchrs(pstr, ch++); // No error.
}
// Check large arguments (t=20, u=21).
#define largeA(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, x, y, z) \
((a) + (a) + (b) + (b) + (c) + (c) + (d) + (d) + (e) + (e) + (f) + (f) + (g) + (g) + \
(h) + (h) + (i) + (i) + (j) + (j) + (k) + (k) + (l) + (l) + (m) + (m) + (n) + (n) + \
(o) + (o) + (p) + (p) + (q) + (q) + (r) + (r) + (s) + (s) + (t) + (t) + (u) + (u) + \
(v) + (v) + (x) + (x) + (y) + (y) + (z) + (z))
void large(int a) {
largeA(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, a++, 0, 0, 0, 0, 0, 0);
// CHECK-MESSAGES: :[[@LINE-1]]:64: warning: side effects in the 19th macro argument 's'
largeA(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, a++, 0, 0, 0, 0, 0);
// CHECK-MESSAGES: :[[@LINE-1]]:67: warning: side effects in the 20th macro argument 't'
largeA(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, a++, 0, 0, 0, 0);
// 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))
void builtins(int ret, int a) {
ret += builtinA(a++);
ret += builtinB(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'
}
// 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)
{
ret += condA(a++, b++);
condB(a, b++);
}