[PATCH] clang-tidy: check for repeated side effects in macro
Alexander Kornienko
alexfh at google.com
Fri May 8 07:08:24 PDT 2015
Sorry for the delay. I was on vacation and currently digging out stuff out of my inbox. I'll try to review this thoroughly early next week. A few random comments for now.
In http://reviews.llvm.org/D9496#168953, @danielmarjamaki wrote:
> Now it doesn't warn about macros in system headers.
It looks like the latest patch is exactly the same as the first one.
================
Comment at: clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp:41
@@ +40,3 @@
+ const MacroArgs *Args) {
+ clang::SourceLocation Loc = Range.getBegin();
+ // Ignore macro argument expansions.
----------------
The `clang::` qualifier is not needed here and below.
================
Comment at: clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp:50
@@ +49,3 @@
+ typedef clang::MacroInfo::arg_iterator ArgIter;
+ for (ArgIter AI = MI->arg_begin(), AE = MI->arg_end(); AI != AE; ++AI) {
+ const clang::Token *ResultArgToks =
----------------
Will a range-based for loop work here?
================
Comment at: clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp:54
@@ +53,3 @@
+ int CountInMacro = 0;
+ for (TokIter TI = MI->tokens_begin(), TE = MI->tokens_end(); TI != TE;
+ ++TI) {
----------------
ditto
================
Comment at: test/clang-tidy/misc-repeated-side-effects-in-macro.c:13
@@ +12,3 @@
+
+void plusplus2() {
+ int j = 0;
----------------
Do you need a separate function for each test?
Also, please leave the whole warning message once and truncate all the other instances to fit into 80 columns.
http://reviews.llvm.org/D9496
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the cfe-commits
mailing list