fix false positives in clang-tidy macro parentheses checker

[danielmarjamaki]

Summary

I got email from Robert Reif about false positive(s) for such code:

struct Fred
{
    enum Foo { First, Second };
};

const char * to_string(Fred::Foo foo, bool verbose)
{
    switch (foo)
    {
#define CASE(a, b)    case Fred::a: if (verbose) return b; else return #a
    CASE(First, "This is the first one");
    CASE(Second, "This is the second one");
#undef CASE
    }
    return "unknown";
}

The macro-parentheses checker currently warns about both macro arguments a and b. The warning about a is wrong. The warning about b is annoying.

This patch fixes so no warning is shown for that example code.

[alexfh]

LG

clang-tidy/misc/MacroParenthesesCheck.cpp

172

nit: "Assignment/return, i.e. '= x;' or 'return x;'.

[danielmarjamaki]

Committed with 240399.

[alexfh]

BTW, I found a couple of related issues:

1. When a parameter is used as a type or a namespace qualifier: `#define

X(type) type::Field`

2. When a parameter is used as a type parameter of a template: `#define

X(t) std::set<t> s; ... or #define X(t) some_template<..., t, ...>`

[danielmarjamaki]

In D10644#193393, @alexfh wrote:

BTW, I found a couple of related issues:

1. When a parameter is used as a type or a namespace qualifier: `#define

X(type) type::Field`

2. When a parameter is used as a type parameter of a template: `#define

X(t) std::set<t> s; ... or #define X(t) some_template<..., t, ...>`

thanks!

I'll look at those asap..

clang-tidy/misc/MacroParenthesesCheck.cpp

for (auto TI = MI->tokens_begin(), TE = MI->tokens_end(); TI != TE; ++TI) {
     if (isSurroundedLeft(Prev) && isSurroundedRight(Next))
       continue;
 
     // Don't warn after hash/hashhash or before hashhash.
     if (Prev.isOneOf(tok::hash, tok::hashhash) || Next.is(tok::hashhash))
       continue;
 
     // Argument is a struct member.
-    if (Prev.isOneOf(tok::period, tok::arrow))
+    if (Prev.isOneOf(tok::period, tok::arrow, tok::coloncolon))
       continue;
 
     // String concatenation.
     if (isStringLiteral(Prev.getKind()) || isStringLiteral(Next.getKind()))
       continue;
 
     // Type/Var.
     if (isAnyIdentifier(Prev.getKind()) || isKeyword(Prev) ||
         isAnyIdentifier(Next.getKind()) || isKeyword(Next))
       continue;
 
     // Initialization.
     if (Next.is(tok::l_paren))
       continue;
 
     // Cast.
     if (Prev.is(tok::l_paren) && Next.is(tok::star) &&
         TI + 2 != MI->tokens_end() && (TI + 2)->is(tok::r_paren))
       continue;
 
-    // Assignment.
-    if (Prev.is(tok::equal) && Next.is(tok::semi))
+    // Assignment/Return, i.e. '=x;' or 'return x;'.
+    if (Prev.isOneOf(tok::equal, tok::kw_return) && Next.is(tok::semi))
       continue;
 
     Check->diag(Tok.getLocation(), "macro argument should be enclosed in "
                                    "parentheses")
         << FixItHint::CreateInsertion(Tok.getLocation(), "(")
         << FixItHint::CreateInsertion(Tok.getLocation().getLocWithOffset(
                                           PP->getSpelling(Tok).length()),
                                       ")");

test/clang-tidy/misc-macro-parentheses.cpp

 #define GOOD13(x)         a.x
 #define GOOD14(x)         a->x
 #define GOOD15(x)         ({ int a = x; a+4; })
 #define GOOD16(x)         a_ ## x, b_ ## x = c_ ## x - 1,
 #define GOOD17            case 123: x=4+5; break;
 #define GOOD18(x)         ;x;
 #define GOOD19            ;-2;
 #define GOOD20            void*
+#define GOOD21(a)         case Fred::a:
+#define GOOD22(a)         if (verbose) return a;
 
 // These are allowed for now..
 #define MAYBE1            *12.34
 #define MAYBE2            <<3