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

[Daniel Marjamäki]

The script I use to test Clang on many packages:
https://drive.google.com/file/d/0BykPmWrCOxt2Zy1wVXZQWVV0LVE/view?usp=sharing

Background:
In the Cppcheck project we run Cppcheck on all debian packages continuously with a script. The results are shown here:
http://cppcheck.sourceforge.net/devinfo/daca2-report/daca2.html

I modified the script that runs Cppcheck so it runs Clang instead.

How to use it:

Create a folder:
mkdir ~/daca-clang

Run Clang on debian source code:
python daca-clang.py --folders=abc --cc_path=~/llvm/build/Debug+Asserts/bin1

--folders=abc:

  Select folders to check. This will select the folders "a", "b" and "c".. these contain all packages that start with "a", "b" or "c".

--cc_path=...

  Specify where clang is found

this will only run normal compilation. To run a static analyser checker, add --checker=name-of-checker. To run clang-tidy, use --clang-tidy (however this currently only runs misc-macro* checkers).


================
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;
----------------
alexfh wrote:
> danielmarjamaki wrote:
> > alexfh wrote:
> > > 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.
> > For instance: toupper, strchr. I think these were false positives. :-(
> > 
> > For instance for strchr I assume "__builtin_constant_p (s)" does not use s at runtime and then it's safe to use s again later in the macro.
> > 
> > I don't see how we can teach the checker to not care about any compiletime behaviour. do you know if this is possible?
> > 
> I can find some definitions of these macros, but I'm not sure they would be the same definitions you are talking about. It would be easier if you quoted the definitions here.
Here is the toupper macro:
```
#  define toupper(c)    __tobody (c, toupper, *__ctype_toupper_loc (), (c))
```
The c is repeated but I assume that __tobody doesnt use both argument 1 and 4 at runtime.

Here is the strchr macro:
```
#  define strchr(s, c) \
  (__extension__ (__builtin_constant_p (c) && !__builtin_constant_p (s)	      \
		  && (c) == '\0'					      \
		  ? (char *) __rawmemchr (s, c)				      \
		  : __builtin_strchr (s, c)))
```
The s is used in the condition and then in both conditional expressions. However I assume that the usage in the condition is compile-time only.

http://reviews.llvm.org/D9496

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