In http://reviews.llvm.org/D9528#176259, @alexfh wrote:
> A few random issues I noticed on the first couple of pages of results you > posted on a different thread: > > ../src/transmit.c:37:25: warning: macro replacement list should be enclosed > in parentheses [misc-macro-parentheses] > #define MKQUERY_ADDB(b) *rqp++= (b) > ^ > > > It looks like the check is complaining about the `rgp` variable, which is > not a parameter of the macro. > > ../src/addrfam.c:538:3: warning: macro replacement list should be enclosed > in parentheses [misc-macro-parentheses] > const char *p= dgram + rps->labstart[labnum]; \ > ^ > > > Does it complain about `const`? Then it looks like the same issue as the > first one. > > adnstest.c:82:3: warning: macro replacement list should be enclosed in > parentheses [misc-macro-parentheses] > case e: fprintf(stderr,"adns failure: %s: errno=" #e "\n",what); break > ^ > adnstest.c:82:8: warning: macro argument should be enclosed in parentheses > [misc-macro-parentheses] > case e: fprintf(stderr,"adns failure: %s: errno=" #e "\n",what); break > ^ > > > Same. > > It looks like a frequent case, so fixing this would help fixing lots of false > positives. > > ../src/addrfam.c:47:48: warning: macro argument should be enclosed in > parentheses [misc-macro-parentheses] > #define SIN(cnst, sa) ((void)(sa)->sa_family, (cnst struct sockaddr_in > *)(sa)) > ^ > > > `cnst` is a parameter of the macro, but parens are not needed (if acceptable) > in this context. The rule only makes sense for the macro arguments that can > be expressions, so you should try to restrict the check to the cases where > macro argument resembles an expression or when the context where the > parameter is expanded looks like an expression. I'm not sure how exactly this > can be done, and whether it's possible to do this precisely enough, but some > cases can definitely be improved. Hmm.. I think that the results are a bit confusing. When it says "macro replacement list should be enclosed in parentheses" I want that the whole expression is surrounded with parentheses. > It looks like the check is complaining about the `rgp` variable, which is > not a parameter of the macro. Yes it is confusing.. maybe it shouldn't point at the rgp. > Does it complain about `const`? Then it looks like the same issue as the > first one. no.. but I do think it's a confusing message. > `cnst` is a parameter of the macro, but parens are not needed (if acceptable) > in this context. The rule only makes sense for the macro arguments that can > be expressions, so you should try to restrict the check to the cases where > macro argument resembles an expression or when the context where the > parameter is expanded looks like an expression. I'm not sure how exactly this > can be done, and whether it's possible to do this precisely enough, but some > cases can definitely be improved. I agree .. maybe it's acceptable to hide the warning when the next token is a name. ================ 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 MacroDirective *MD); ---------------- alexfh wrote: > Here and below. Please use C++ style comments (`//` or `///` for Doxygen > comments). The comments should be English prose with proper capitalization > and punctuation. ok will try to fix in next patch 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
