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

[Alexander Kornienko]

================
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);
----------------
alexfh wrote:
> nit: Missing trailing period.
Seems like you've missed this.

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

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

================
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
----------------
alexfh wrote:
> 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.
In this comment I meant to ask you to handle the `Count < 0` case separately: when it happens, we clearly don't want to complain about missing parentheses or add them.

Sorry if it wasn't clear enough.

================
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
----------------
alexfh wrote:
> nit: Missing comma after "expression".
Seems like you've missed this.

================
Comment at: clang-tidy/misc/MacroParenthesesCheck.h:32
@@ +31,3 @@
+/// properly.
+
+class MacroParenthesesCheck : public ClangTidyCheck {
----------------
alexfh wrote:
> nit: Remove the empty line, please.
Seems like you've missed this.

http://reviews.llvm.org/D9528

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