fixed comments from previous revision.
http://reviews.llvm.org/D9496
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp
clang-tidy/misc/MacroRepeatedSideEffectsCheck.h
clang-tidy/misc/MiscTidyModule.cpp
test/clang-tidy/misc-repeated-side-effects-in-macro.c
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
Index: test/clang-tidy/misc-repeated-side-effects-in-macro.c
===================================================================
--- test/clang-tidy/misc-repeated-side-effects-in-macro.c
+++ test/clang-tidy/misc-repeated-side-effects-in-macro.c
@@ -0,0 +1,80 @@
+// 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 first macro argument 'x' is repeated in macro expansion [misc-macro-repeated-side-effects]
+ ret = badA(++a, b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: side effects in the first
+ ret = badA(a--, b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: side effects in the first
+ ret = badA(--a, b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: side effects in the first
+ ret = badA(a, b++);
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: side effects in the second
+ ret = badA(a, ++b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: side effects in the second
+ ret = badA(a, b--);
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: side effects in the second
+ ret = badA(a, --b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: side effects in the second
+}
+
+
+// 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 first macro argument 'A' is repeated in macro expansion [misc-macro-repeated-side-effects]
+}
+
+// do not produce a false positive on a strchr() macro, currently the '?'
+// triggers the test to bail, 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 argument (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]]:45: warning: side effects in the nineteenth macro argument 's' is repeated in macro expansion [misc-macro-repeated-side-effects]
+ 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]]:47: warning: side effects in the twentieth macro argument 't' is repeated in macro expansion [misc-macro-repeated-side-effects]
+ 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]]:49: warning: side effects in the macro argument 'u' is repeated in macro expansion [misc-macro-repeated-side-effects]
+}
+
+// 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]]:18: warning: side effects in the first macro argument 'x' is repeated in macro expansion [misc-macro-repeated-side-effects]
+ ret += macroA(a++);
+ ret += macroB(a++);
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: side effects in the first
+}
+
+// bailout 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++);
+}
+
Index: clang-tidy/misc/MacroRepeatedSideEffectsCheck.h
===================================================================
--- clang-tidy/misc/MacroRepeatedSideEffectsCheck.h
+++ clang-tidy/misc/MacroRepeatedSideEffectsCheck.h
@@ -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 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
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -16,6 +16,7 @@
#include "BoolPointerImplicitConversionCheck.h"
#include "InaccurateEraseCheck.h"
#include "InefficientAlgorithmCheck.h"
+#include "MacroRepeatedSideEffectsCheck.h"
#include "NoexceptMoveConstructorCheck.h"
#include "StaticAssertCheck.h"
#include "SwappedArgumentsCheck.h"
@@ -42,6 +43,8 @@
"misc-inaccurate-erase");
CheckFactories.registerCheck<InefficientAlgorithmCheck>(
"misc-inefficient-algorithm");
+ CheckFactories.registerCheck<MacroRepeatedSideEffectsCheck>(
+ "misc-macro-repeated-side-effects");
CheckFactories.registerCheck<NoexceptMoveConstructorCheck>(
"misc-noexcept-move-constructor");
CheckFactories.registerCheck<StaticAssertCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -7,6 +7,7 @@
BoolPointerImplicitConversionCheck.cpp
InaccurateEraseCheck.cpp
InefficientAlgorithmCheck.cpp
+ MacroRepeatedSideEffectsCheck.cpp
MiscTidyModule.cpp
NoexceptMoveConstructorCheck.cpp
StaticAssertCheck.cpp
Index: clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp
===================================================================
--- clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp
+++ clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp
@@ -0,0 +1,144 @@
+//===--- 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;
+ const static StringRef TiersStr[];
+ StringRef getTiersStr(int i);
+};
+} // namespace
+
+const StringRef MacroRepeatedPPCallbacks::TiersStr[] = {
+ "", "first ", "second ", "third ", "fourth ",
+ "fifth ", "sixth ", "seventh ", "eighth ", "ninth ",
+ "tenth ", "eleventh ", "twelfth ", "thirteenth ", "fourteenth ",
+ "fifteenth ", "sixteenth ", "seventeenth ", "eightteenth ", "nineteenth ",
+ "twentieth "};
+
+StringRef MacroRepeatedPPCallbacks::getTiersStr(int i) {
+ if (i <= 0)
+ return TiersStr[0];
+ if (i <= 20)
+ return TiersStr[i];
+ return TiersStr[0];
+}
+
+void MacroRepeatedPPCallbacks::MacroExpands(const Token &MacroNameTok,
+ const MacroDefinition &MD,
+ SourceRange Range,
+ const MacroArgs *Args) {
+ SourceLocation Loc = Range.getBegin();
+ // Ignore macro argument expansions.
+ if (!Loc.isFileID())
+ return;
+
+ const MacroInfo *MI = MD.getMacroInfo();
+ for (const IdentifierInfo *Arg : MI->args()) {
+ const Token *ResultArgToks =
+ Args->getUnexpArgument(MI->getArgumentNum(Arg));
+
+ int CountInMacro = 0;
+ bool SkipParen = false;
+ int SkipParenCount = 0;
+ for (const auto &T : MI->tokens()) {
+ // Bail if contents of Macro is containing keywords that are making the
+ // macro to complex.
+ if (T.is(tok::question) || T.is(tok::kw_if) || T.is(tok::kw_else) ||
+ T.is(tok::kw_switch) || T.is(tok::kw_case) || T.is(tok::kw_break) ||
+ T.is(tok::kw_while) || T.is(tok::kw_do) || T.is(tok::kw_for) ||
+ T.is(tok::kw_continue) || T.is(tok::kw_goto) || T.is(tok::kw_return))
+ return;
+ // Skip the whole paranthesis that is starting at the current position. If
+ // none is starting then do not skip anything.
+ 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 == NULL)
+ continue;
+ // If another macro or a builtin is found within the macro definition,
+ // skip next parenthesis.
+ if (TII->hasMacroDefinition() || TII->getBuiltinID() != 0) {
+ SkipParen = true;
+ continue;
+ }
+ int ArgNo = MI->getArgumentNum(TII);
+ if (ArgNo == -1) {
+ // This isn't an argument, continue.
+ continue;
+ }
+ const Token *Res = Args->getUnexpArgument(ArgNo);
+ if (ResultArgToks == Res)
+ CountInMacro++;
+ }
+
+ if (CountInMacro < 2)
+ continue;
+
+ // If the arg token didn't expand into anything, ignore it.
+ if (ResultArgToks->is(tok::eof))
+ continue;
+ unsigned NumToks = MacroArgs::getArgLength(ResultArgToks);
+
+ // Check for side effects in the argument expansions.
+ for (unsigned ArgumentIndex = 0; ArgumentIndex < NumToks; ++ArgumentIndex) {
+ const Token &AT = ResultArgToks[ArgumentIndex];
+
+ if (AT.is(tok::plusplus) || AT.is(tok::minusminus)) {
+ Check.diag(ResultArgToks->getLocation(), "side effects in the %0macro "
+ "argument '%1' is repeated in "
+ "macro expansion")
+ << getTiersStr(MI->getArgumentNum(Arg) + 1) << Arg->getName();
+ Check.diag(MI->getDefinitionLoc(), "%0 macro defined here",
+ DiagnosticIDs::Note)
+ << MacroNameTok.getIdentifierInfo();
+ }
+ }
+ }
+}
+
+void MacroRepeatedSideEffectsCheck::registerPPCallbacks(
+ CompilerInstance &Compiler) {
+ Compiler.getPreprocessor().addPPCallbacks(
+ ::llvm::make_unique<MacroRepeatedPPCallbacks>(
+ *this, Compiler.getSourceManager(), Compiler.getPreprocessor()));
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
