[PATCH] D12446: [PATCH] Enable clang-tidy misc-static-assert for C11

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

> 1. If <assert.h> is included, then static_assert() is also fine to use. Presumably, in order for the assert() check to trigger, <assert.h> has to be included, but I don't feel particularly comfortable with that assumption.


Do you have any C11 code that uses `assert()`, but doesn't include `assert.h`? If no, I'd suggest relying on this assumption until someone stumbles upon this and files a bug (or fixes this). One more option is to ensure assert.h is always included in the file being fixed (there's `IncludeInserter` class that can do this).

> If there's a way to access a Preprocessor instance from check(),


You can subscribe to `PPCallbacks` and find out if static_assert is defined after the last include. Or if it's defined and not undefined. But I'd probably just skip this until you see real code that would benefit from this.

> we can use getMacroDefinitionAtLoc() to determine whether static_assert() is defined and a valid replacement for assert(). For right now, I'm 

>  always using _Static_assert() in C11 mode to be on the safe side.




> 2. My Python skills are rudimentary at best. If there's a better way to update check_clang_tidy.py to handle file extensions, that would be great. This change is required or else the script fails on .c test cases that check fixes because of writing out to a .tmp.cpp file instead of a .tmp.c file conflicts with the language options specified on the RUN line.


Alternatively, you could add "-x c" to the test's RUN: line, but your solution is an improvement over this hack. Thanks!


================
Comment at: test/clang-tidy/check_clang_tidy.py:50
@@ -45,3 +49,3 @@
   if len(clang_tidy_extra_args) == 0:
-    clang_tidy_extra_args = ['--', '--std=c++11']
+    clang_tidy_extra_args = ['--', '--std=c++11' if extension == '.cpp' else '-std=c99']
 
----------------
I'm not sure that c99 is a good default for C code here. The existing .c tests don't rely on this, and the tests you're adding override `-std` anyway. I'd change this to `['--', '--std=c++11'] if extension == '.cpp' else ['--']`.

================
Comment at: test/clang-tidy/misc-static-assert-c99.c:1
@@ +1,2 @@
+// RUN: %python %S/check_clang_tidy.py %s misc-static-assert %t -- -std=c99
+
----------------
Instead of duplicating the test, you could add a second run line in the other test and just verify that clang-tidy doesn't generate any warnings when run with `-std=c99`. The invariant "no warnings == no fixes" is pretty basic for clang-tidy, so you can rely on it in the tests.


http://reviews.llvm.org/D12446