clang-tidy: Add checker that warn when macro argument with side effects is repeated...
authorDaniel Marjamaki <daniel.marjamaki@evidente.se>
Wed, 17 Jun 2015 14:19:35 +0000 (14:19 +0000)
committerDaniel Marjamaki <daniel.marjamaki@evidente.se>
Wed, 17 Jun 2015 14:19:35 +0000 (14:19 +0000)
llvm-svn: 239909

clang-tools-extra/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp [new file with mode: 0644]
clang-tools-extra/clang-tidy/misc/MacroRepeatedSideEffectsCheck.h [new file with mode: 0644]
clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/test/clang-tidy/misc-repeated-side-effects-in-macro.c [new file with mode: 0644]

index d4057bf..c2ee923 100644 (file)
@@ -8,6 +8,7 @@ add_clang_library(clangTidyMiscModule
   InaccurateEraseCheck.cpp
   InefficientAlgorithmCheck.cpp
   MacroParenthesesCheck.cpp
+  MacroRepeatedSideEffectsCheck.cpp
   MiscTidyModule.cpp
   NoexceptMoveConstructorCheck.cpp
   StaticAssertCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp b/clang-tools-extra/clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp
new file mode 100644 (file)
index 0000000..4e60164
--- /dev/null
@@ -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
diff --git a/clang-tools-extra/clang-tidy/misc/MacroRepeatedSideEffectsCheck.h b/clang-tools-extra/clang-tidy/misc/MacroRepeatedSideEffectsCheck.h
new file mode 100644 (file)
index 0000000..74c4db1
--- /dev/null
@@ -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
index 91f59f7..4267071 100644 (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>(
diff --git a/clang-tools-extra/test/clang-tidy/misc-repeated-side-effects-in-macro.c b/clang-tools-extra/test/clang-tidy/misc-repeated-side-effects-in-macro.c
new file mode 100644 (file)
index 0000000..8d35935
--- /dev/null
@@ -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++);
+}
+