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

[Daniel Marjamäki]

Committed with 239820.


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

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

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

================
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:
> 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.
Fixed

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

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

================
Comment at: clang-tidy/misc/MacroParenthesesCheck.h:18
@@ +17,3 @@
+
+class MacroParenthesesCheck : public ClangTidyCheck {
+public:
----------------
alexfh wrote:
> Please add a class comment describing the purpose of this check, motivation for it and what specific code patterns it triggers on.
Fixed

http://reviews.llvm.org/D9528

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