================
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/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
