[clang-tidy] Avoid bugprone-macro-parentheses warnings after goto argument

[jubnzv]

Summary

clang-tidy should not generate bugprone-macro-parentheses warnings for the goto argument without parentheses, because it would be a syntax error.

The only valid case where an argument can be enclosed in parentheses is "Labels as Values" gcc extension: https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html.
But using this extension generates a syntax error in clang, so it cannot be checked with clang-tidy.

This commit fixes the following bugzilla issue: https://bugs.llvm.org/show_bug.cgi?id=49634

[jubnzv]

Ping. Could somebody review this, please?

[aaron.ballman]

LGTM, thank you!

[aaron.ballman]

Adding the CFE mailing list, which was not on the review previously.

[njames93]

In D99924#2706181, @aaron.ballman wrote:

Adding the CFE mailing list, which was not on the review previously.

I think H744 needs updating to add cfe-commits. This used to be handled by the rule that adds clang to projects in clang-tools-extra but that has since been removed for clang-tools-extra.

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

 }
 
 void MacroParenthesesPPCallbacks::argument(const Token &MacroNameTok,
                                            const MacroInfo *MI) {
 
   // Skip variable declaration.
   bool VarDecl = possibleVarDecl(MI, MI->tokens_begin());
 
+  // Skip the goto argument with an arbitrary number of subsequent stars.
+  bool FoundGoto = false;
+
   for (auto TI = MI->tokens_begin(), TE = MI->tokens_end(); TI != TE; ++TI) {
     // First token.
     if (TI == MI->tokens_begin())
       continue;
 
     // Last token.
     if ((TI + 1) == MI->tokens_end())
       continue;
 
     const Token &Prev = *(TI - 1);
     const Token &Next = *(TI + 1);
 
     const Token &Tok = *TI;
 
     // There should not be extra parentheses in possible variable declaration.
     if (VarDecl) {
       if (Tok.isOneOf(tok::equal, tok::semi, tok::l_square, tok::l_paren))
         VarDecl = false;
       continue;
     }
 
+    // There should not be extra parentheses for the goto argument.
+    if (Tok.is(tok::kw_goto)) {
+      FoundGoto = true;
+      continue;
+    }
+
     // Only interested in identifiers.
-    if (!Tok.isOneOf(tok::identifier, tok::raw_identifier))
+    if (!Tok.isOneOf(tok::identifier, tok::raw_identifier)) {
+      FoundGoto = false;
       continue;
+    }
 
     // Only interested in macro arguments.
     if (MI->getParameterNum(Tok.getIdentifierInfo()) < 0)
       continue;
 
     // Argument is surrounded with parentheses/squares/braces/commas.
     if (isSurroundedLeft(Prev) && isSurroundedRight(Next))
       continue;
for (auto TI = MI->tokens_begin(), TE = MI->tokens_end(); TI != TE; ++TI) {
     // Namespaces.
     if (Prev.is(tok::kw_namespace))
       continue;
 
     // Variadic templates
     if (MI->isVariadic())
       continue;
 
+    if (!FoundGoto) {
       Check->diag(Tok.getLocation(), "macro argument should be enclosed in "
                                      "parentheses")
           << FixItHint::CreateInsertion(Tok.getLocation(), "(")
           << FixItHint::CreateInsertion(Tok.getLocation().getLocWithOffset(
                                             PP->getSpelling(Tok).length()),
                                         ")");
     }
+
+    FoundGoto = false;
+  }
 }
 
 void MacroParenthesesCheck::registerPPCallbacks(
     const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
   PP->addPPCallbacks(std::make_unique<MacroParenthesesPPCallbacks>(PP, this));
 }
 
 } // namespace bugprone
 } // namespace tidy
 } // namespace clang

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

 // RUN: %check_clang_tidy %s bugprone-macro-parentheses %t
 
 #define BAD1              -1
 // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: macro replacement list should be enclosed in parentheses [bugprone-macro-parentheses]
 #define BAD2              1+2
 // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: macro replacement list should be enclosed in parentheses [bugprone-macro-parentheses]
 #define BAD3(A)           (A+1)
 // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: macro argument should be enclosed in parentheses [bugprone-macro-parentheses]
 #define BAD4(x)           ((unsigned char)(x & 0xff))
 // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: macro argument should be enclosed in parentheses [bugprone-macro-parentheses]
 #define BAD5(X)           A*B=(C*)X+2
 // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: macro argument should be enclosed in parentheses [bugprone-macro-parentheses]
+#define BAD6(x)           goto *x;
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: macro argument should be enclosed in parentheses [bugprone-macro-parentheses]
+#define BAD7(x, y)        if (x) goto y; else x;
+// CHECK-MESSAGES: :[[@LINE-1]]:47: warning: macro argument should be enclosed in parentheses [bugprone-macro-parentheses]
 
 #define GOOD1             1
 #define GOOD2             (1+2)
 #define GOOD3(A)          #A
 #define GOOD4(A,B)        A ## B
 #define GOOD5(T)          ((T*)0)
 #define GOOD6(B)          "A" B "C"
 #define GOOD7(b)          A b

 #define GOOD26(x)         (a->*x)
 #define GOOD27(x)         (a.*x)
 #define GOOD28(x)         namespace x {int b;}
 #define GOOD29(...)       std::cout << __VA_ARGS__;
 #define GOOD30(args...)   std::cout << args;
 #define GOOD31(X)         A*X=2
 #define GOOD32(X)         std::vector<X>
 #define GOOD33(x)         if (!a__##x) a_##x = &f(#x)
+#define GOOD34(x, y)      if (x) goto y;
+#define GOOD35(x, y)      if (x) goto *(y);
 
 // These are allowed for now..
 #define MAYBE1            *12.34
 #define MAYBE2            <<3