[clang-tidy] fix false positives for bugprone-macro-parentheses in C++ templates

clang-tools-extra/clang-tidy/bugprone/MacroParenthesesCheck.cpp

@@ -52,7 +52,8 @@ static bool isSurroundedRight(const Token &T) {
 /// Is given TokenKind a keyword?
 static bool isKeyword(const Token &T) {
   // FIXME: better matching of keywords to avoid false positives.
-  return T.isOneOf(tok::kw_if, tok::kw_case, tok::kw_const, tok::kw_struct);
+  return T.isOneOf(tok::kw_if, tok::kw_case, tok::kw_const, tok::kw_volatile,
+                   tok::kw_struct);
 }
 
 /// Warning is written when one of these operators are not within parentheses.
@@ -129,19 +130,19 @@ void MacroParenthesesPPCallbacks::replacementList(const Token &MacroNameTok,
       // Heuristic for macros that are clearly not intended to be enclosed in
       // parentheses, macro starts with operator. For example:
       // #define X     *10
-      if (TI == MI->tokens_begin() && (TI + 1) != TE &&
+      if (TI == MI->tokens_begin() && std::next(TI) != TE &&
           !Tok.isOneOf(tok::plus, tok::minus))
         return;
       // Don't warn about this macro if the last token is a star. For example:
       // #define X    void *
-      if ((TE - 1)->is(tok::star))
+      if (std::prev(TE)->is(tok::star))
         return;
 
       Loc = Tok.getLocation();
     }
   }
   if (Loc.isValid()) {
-    const Token &Last = *(MI->tokens_end() - 1);
+    const Token &Last = *std::prev(MI->tokens_end());
     Check->diag(Loc, "macro replacement list should be enclosed in parentheses")
         << FixItHint::CreateInsertion(MI->tokens_begin()->getLocation(), "(")
         << FixItHint::CreateInsertion(Last.getLocation().getLocWithOffset(
@@ -164,11 +165,11 @@ void MacroParenthesesPPCallbacks::argument(const Token &MacroNameTok,
       continue;
 
     // Last token.
-    if ((TI + 1) == MI->tokens_end())
+    if (std::next(TI) == MI->tokens_end())
       continue;
 
-    const Token &Prev = *(TI - 1);
-    const Token &Next = *(TI + 1);
+    const Token &Prev = *std::prev(TI);
+    const Token &Next = *std::next(TI);
 
     const Token &Tok = *TI;
 
@@ -227,17 +228,26 @@ void MacroParenthesesPPCallbacks::argument(const Token &MacroNameTok,
 
     // Cast.
     if (Prev.is(tok::l_paren) && Next.is(tok::star) &&
-        TI + 2 != MI->tokens_end() && (TI + 2)->is(tok::r_paren))
+        std::next(TI, 2) != MI->tokens_end() &&
+        std::next(TI, 2)->is(tok::r_paren))
       continue;
 
     // Assignment/return, i.e. '=x;' or 'return x;'.
     if (Prev.isOneOf(tok::equal, tok::kw_return) && Next.is(tok::semi))
       continue;
 
     // C++ template parameters.
-    if (PP->getLangOpts().CPlusPlus && Prev.isOneOf(tok::comma, tok::less) &&
-        Next.isOneOf(tok::comma, tok::greater))
-      continue;
+    if (PP->getLangOpts().CPlusPlus && Prev.isOneOf(tok::comma, tok::less)) {
+      const auto *NextIt =
+          std::find_if_not(std::next(TI), MI->tokens_end(), [](const Token &T) {
+            return T.isOneOf(tok::star, tok::amp, tok::ampamp, tok::kw_const,
+                             tok::kw_volatile);
+          });
+
+      if (NextIt != MI->tokens_end() &&
+          NextIt->isOneOf(tok::comma, tok::greater))
+        continue;
+    }
 
     // Namespaces.
     if (Prev.is(tok::kw_namespace))

clang-tools-extra/docs/ReleaseNotes.rst

@@ -111,10 +111,10 @@ Hover
 Code completion
 ^^^^^^^^^^^^^^^
 
-- Added a new ``MacroFilter`` configuration option to ``Completion`` to 
-  allow fuzzy-matching with the ``FuzzyMatch`` option when suggesting 
-  macros. ``ExactPrefix`` is the default, which retains previous 
-  behavior of suggesting macros which match the prefix exactly.  
+- Added a new ``MacroFilter`` configuration option to ``Completion`` to
+  allow fuzzy-matching with the ``FuzzyMatch`` option when suggesting
+  macros. ``ExactPrefix`` is the default, which retains previous
+  behavior of suggesting macros which match the prefix exactly.
 
 Code actions
 ^^^^^^^^^^^^
@@ -205,7 +205,7 @@ Improvements to clang-tidy
 
 - Improved :program:`clang-tidy` by adding the `--removed-arg` option to remove
   arguments sent to the compiler when invoking Clang-Tidy. This option was also
-  added to :program:`run-clang-tidy.py` and :program:`clang-tidy-diff.py` and 
+  added to :program:`run-clang-tidy.py` and :program:`clang-tidy-diff.py` and
   can be configured in the config file through the `RemovedArgs` option.
 
 - Deprecated the :program:`clang-tidy` ``zircon`` module. All checks have been
@@ -392,6 +392,10 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/invalid-enum-default-initialization>` with new
   `IgnoredEnums` option to ignore specified enums during analysis.
 
+- Improved :doc:`bugprone-macro-parentheses
+  <clang-tidy/checks/bugprone/macro-parentheses>` check by fixing false
+  positives when using C++ template parameters.
+
 - Improved :doc:`bugprone-narrowing-conversions
   <clang-tidy/checks/bugprone/narrowing-conversions>` check by fixing
   false positive from analysis of a conditional expression in C.

clang-tools-extra/test/clang-tidy/checkers/bugprone/macro-parentheses.cpp

@@ -54,3 +54,38 @@
 // These are allowed for now..
 #define MAYBE1            *12.34
 #define MAYBE2            <<3
+#define MAYBE3            a < b * c
+
+#define CAST1(type, p)    (reinterpret_cast<type*>(p))
+#define CAST2(type, p)    (static_cast<type*>(p))
+#define CAST3(type, p)    (const_cast<type*>(p))
+#define CAST4(type, p)    (dynamic_cast<type*>(p))
+#define CAST5(type, p)    (static_cast<type&>(p))
+#define CAST6(type, p)    (static_cast<type&&>(p))
+#define CAST7(type, p)    (static_cast<const type*>(p))
+#define CAST8(type, p)    (static_cast<volatile type*>(p))
+#define CAST9(type, p)    (static_cast<type const*>(p))
+#define CAST10(type, p)   (reinterpret_cast<type * const &>(p))
+
+#define TEMPLATE1(T)      (std::vector<T*>)
+#define TEMPLATE2(T)      (std::vector<T&>)
+#define TEMPLATE3(T)      (std::vector<const T*>)
+#define TEMPLATE4(T)      (std::map<T*, T*>)
+
+#define BAD_TEMPLATE1(T)  (std::vector<T*2>)
+// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: macro argument should be enclosed in parentheses [bugprone-macro-parentheses]
+#define BAD_TEMPLATE2(T)  (std::map<T*2, int>)
+// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: macro argument should be enclosed in parentheses [bugprone-macro-parentheses]
+#define BAD_TEMPLATE3(T)  (std::map<int, T*2>)
+// CHECK-MESSAGES: :[[@LINE-1]]:42: warning: macro argument should be enclosed in parentheses [bugprone-macro-parentheses]
+#define BAD_TEMPLATE4(T)  (std::vector<T+1>)
+// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: macro argument should be enclosed in parentheses [bugprone-macro-parentheses]
+#define BAD_TEMPLATE5(T)  (std::vector<T*T>)
+// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: macro argument should be enclosed in parentheses [bugprone-macro-parentheses]
+// CHECK-MESSAGES: :[[@LINE-2]]:42: warning: macro argument should be enclosed in parentheses [bugprone-macro-parentheses]
+#define BAD_TEMPLATE6(T)  (std::vector<2*T>)
+// CHECK-MESSAGES: :[[@LINE-1]]:42: warning: macro argument should be enclosed in parentheses [bugprone-macro-parentheses]
+#define BAD_CAST1(T)      (reinterpret_cast<T*2>(0))
+// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: macro argument should be enclosed in parentheses [bugprone-macro-parentheses]
+#define BAD_CAST2(T)      (reinterpret_cast<T+1>(0))
+// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: macro argument should be enclosed in parentheses [bugprone-macro-parentheses]