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