[PATCH] Improved checking for repeated macro argument with side effects when macro contains ?:

[Alexander Kornienko]

LG with a couple of comments.


================
Comment at: clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp:93
@@ -83,1 +92,3 @@
   for (const auto &T : MI->tokens()) {
+    // The result from __builtin_constant_p(x) is 0 if x is a macro argument
+    // with side effects. If we have seen a __builtin_constant_p(x) and then
----------------
nits: 
  * s/result from/result of/
  * "If we have seen a `__builtin_constant_p(x)` and then there is a ..." - "If we see `__builtin_constant_p(x)` followed by a ..."
  * add a comma before "then we need to reason about ..."
  * s/bailout/bail out/

================
Comment at: clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp:127
@@ -99,4 +126,3 @@
 
-    // If a builtin is found within the macro definition, skip next
-    // parenthesis.
-    if (TII->getBuiltinID() != 0) {
+    // If a __builtin_constant_p is found within the macro definition, dont
+    // count argument inside the parentheses and remember that it has been seen
----------------
nits:
  * dont -> don't
  * argument -> arguments

http://reviews.llvm.org/D10653

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