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

[Daniel Marjamäki]

> > Here are some results:

> 

> >  https://drive.google.com/file/d/0BykPmWrCOxt2YTdOU0Y3M1RmdGM/view?usp=sharing

> 

> 

> Nice, but the results are from a different check: misc-macro-parentheses.


I see.. I dont have the results anymore. I am rerunning the checker and will probably post results tomorrow.


================
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()))
----------------
alexfh wrote:
> nit: Please add a trailing period.
Will be fixed in next patch

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

================
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) {
----------------
alexfh wrote:
> danielmarjamaki wrote:
> > alexfh wrote:
> > > ditto
> > Impossible.
> > 
> > Fortunately I have a patch pending (http://reviews.llvm.org/D9079) that makes it possible to do this. If it gets the OK and I can apply it I will of course use the range-based for loop here too.
> > 
> Phabricator says that you have committed that patch 10 days ago:
> 
> > Closed by commit rL236975: Refactor MacroInfo so range for loops can be used to iterate its tokens. (authored by danielmarjamaki)
Yes I will of course use this. I totally agree range for loops are better.

http://reviews.llvm.org/D9496

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