[PATCH] clang-tidy: check for repeated side effects in macro

[Alexander Kornienko]

================
Comment at: clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp:48
@@ +47,3 @@
+
+  // Don't write warnings for macros defined in system headers
+  if (SM.isInSystemHeader(MI->getDefinitionLoc()))
----------------
nit: Please add a trailing period.

================
Comment at: clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp:49
@@ +48,3 @@
+  // Don't write warnings for macros defined in system headers
+  if (SM.isInSystemHeader(MI->getDefinitionLoc()))
+    return;
----------------
What system macros did get flagged here that you wouldn't want? Combining MIN/MAX with a side effect seems like a bad idea, for example.

================
Comment at: clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp:53
@@ +52,3 @@
+  typedef MacroInfo::arg_iterator ArgIter;
+  for (ArgIter AI = MI->arg_begin(), AE = MI->arg_end(); AI != AE; ++AI) {
+    const Token *ResultArgToks =
----------------
So, will a range-based for loop work here?

Also, when I send patches for review, I find it convenient to answer each reviewer's comment with "Done" or a reason why the comment is not applicable. This helps keeping track of the issues I addressed.

================
Comment at: clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp:77
@@ +76,3 @@
+
+    // Check for sideeffects in the argument expansions.
+    for (unsigned ArgumentIndex = 0; ArgumentIndex < NumToks; ++ArgumentIndex) {
----------------
s/sideeffects/side effects/

================
Comment at: clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp:84
@@ +83,3 @@
+            ResultArgToks->getLocation(),
+            "side effects in macro argument is repeated in macro expansion");
+        Check.diag(MI->getDefinitionLoc(), "%0 macro defined here",
----------------
I'd make the message more specific by adding at least the index of the argument that is repeated. Maybe also add the name of the parameter in the note.

http://reviews.llvm.org/D9496

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