[PATCH] clang-tidy checker that enforce proper parentheses in macros

[Alexander Kornienko]

I agree with most Aaron's comments on the list. The main concern is the noisiness of the check: it makes sense to look at a bigger sample of results and see whether the warning should be silenced in more cases. Another possibility to make the check more usable would be to detect cases where automated fixes would be possible. Otherwise I doubt anyone will be willing to fix hundreds of warnings in their projects manually.

A few more comments inline.


================
Comment at: clang-tidy/misc/MacroParenthesesCheck.cpp:68
@@ +67,3 @@
+    } else if (Indent <= 0 &&
+               (Tok.is(tok::plus) || Tok.is(tok::minus) || Tok.is(tok::star) ||
+                Tok.is(tok::slash) || Tok.is(tok::percent) ||
----------------
This looks like a reason to move isOneOf from clang::format::FormatToken (tools/clang/lib/Format/FormatToken.h) to clang::Token. It would greatly simplify this and similar conditions.

================
Comment at: clang-tidy/misc/MacroParenthesesCheck.cpp:77
@@ +76,3 @@
+          !Tok.is(tok::minus) && (TI + 1) != TE &&
+          (TI + 1)->is(tok::numeric_constant) && (TI + 2) == TE) {
+        return;
----------------
`TI + 2 == TE` implies `TI + 1 != TE`. I'd also put it next to `TI == MI->tokens_begin()`.

================
Comment at: clang-tidy/misc/MacroParenthesesCheck.cpp:86
@@ +85,3 @@
+  if (Err) {
+    SourceLocation Loc = MI->tokens_begin()->getLocation();
+    Check->diag(Loc,
----------------
nit: No need for a variable here, just use the expression below.

http://reviews.llvm.org/D9528

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/