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

[Alexander Kornienko]

A few nits, but looks good in general. I would also like to clear a couple of questions before you submit this.

Did you encounter any cases when the code broke after applying fixes? How many instances of the warning does the latest version of the check produce on the same set of projects where you saw 47k warnings initially? I would also like to see an estimate of the false positive rate (from a random sample of 100 warnings).


================
Comment at: clang-tidy/misc/MacroParenthesesCheck.cpp:32
@@ +31,3 @@
+private:
+  /// Replacement list with calculations should be enclosed in parentheses
+  void replacementList(const Token &MacroNameTok, const MacroInfo *MI);
----------------
nit: Missing trailing period.

================
Comment at: clang-tidy/misc/MacroParenthesesCheck.cpp:35
@@ +34,3 @@
+
+  /// arguments should be enclosed in parentheses
+  void argument(const Token &MacroNameTok, const MacroInfo *MI);
----------------
nit: Missing trailing period, no capitalization.

================
Comment at: clang-tidy/misc/MacroParenthesesCheck.cpp:60
@@ +59,3 @@
+
+/// Is given TokenKind a calculation operator?
+bool isCalcOp(tok::TokenKind K) {
----------------
What is a "calculation operator"? How is it different from other binary operators in this context?

================
Comment at: clang-tidy/misc/MacroParenthesesCheck.cpp:89
@@ +88,3 @@
+      --Count;
+    } else if (Count <= 0 && isCalcOp(Tok.getKind())) {
+      // Heuristic for macros that are clearly not intended to be enclosed in
----------------
I'd make this even stricter: if the replacement list contains unbalanced parentheses/braces/brackets, then it's clearly not meant to be enclosed in parentheses.

================
Comment at: clang-tidy/misc/MacroParenthesesCheck.h:25
@@ +24,3 @@
+///
+/// When the replacement list has an expression it is recommended to surround it
+/// with parentheses. This ensures that the macro result is evaluated
----------------
nit: Missing comma after "expression".

================
Comment at: clang-tidy/misc/MacroParenthesesCheck.h:32
@@ +31,3 @@
+/// properly.
+
+class MacroParenthesesCheck : public ClangTidyCheck {
----------------
nit: Remove the empty line, please.

http://reviews.llvm.org/D9528

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