aaron.ballman created this revision.
aaron.ballman added reviewers: alexfh, djasper.
aaron.ballman added a subscriber: cfe-commits.
This patch adds support for the C11 _Static_assert macro to clang-tidy's
misc-static-assert checker. There are a few things I'm not overly satisfied
with, and welcome suggestions on how to improve, if desired.
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. If
there's a way to access a Preprocessor instance from check(), 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.
~Aaron
http://reviews.llvm.org/D12446
Files:
clang-tidy/misc/StaticAssertCheck.cpp
test/clang-tidy/check_clang_tidy.py
test/clang-tidy/misc-static-assert-c11.c
test/clang-tidy/misc-static-assert-c99.c
Index: test/clang-tidy/misc-static-assert-c99.c
===================================================================
--- test/clang-tidy/misc-static-assert-c99.c
+++ test/clang-tidy/misc-static-assert-c99.c
@@ -0,0 +1,24 @@
+// RUN: %python %S/check_clang_tidy.py %s misc-static-assert %t -- -std=c99
+
+void abort() {}
+#ifdef NDEBUG
+#define assert(x) 1
+#else
+#define assert(x) \
+ if (!(x)) \
+ abort()
+#endif
+
+void f(void) {
+ int x = 1;
+ assert(x == 0);
+ // CHECK-FIXES: {{^ }}assert(x == 0);
+
+ #define static_assert(x, msg) _Static_assert(x, msg)
+ assert(11 == 5 + 6);
+ // CHECK-FIXES: {{^ }}assert(11 == 5 + 6);
+ #undef static_assert
+
+ assert(10 == 5 + 5);
+ // CHECK-FIXES: {{^ }}assert(10 == 5 + 5);
+}
Index: test/clang-tidy/misc-static-assert-c11.c
===================================================================
--- test/clang-tidy/misc-static-assert-c11.c
+++ test/clang-tidy/misc-static-assert-c11.c
@@ -0,0 +1,26 @@
+// RUN: %python %S/check_clang_tidy.py %s misc-static-assert %t -- -std=c11
+
+void abort() {}
+#ifdef NDEBUG
+#define assert(x) 1
+#else
+#define assert(x) \
+ if (!(x)) \
+ abort()
+#endif
+
+void f(void) {
+ int x = 1;
+ assert(x == 0);
+ // CHECK-FIXES: {{^ }}assert(x == 0);
+
+ #define static_assert(x, msg) _Static_assert(x, msg)
+ assert(11 == 5 + 6);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be
+ // CHECK-FIXES: {{^ }}_Static_assert(11 == 5 + 6, "");
+ #undef static_assert
+
+ assert(10 == 5 + 5);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be
+ // CHECK-FIXES: {{^ }}_Static_assert(10 == 5 + 5, "");
+}
Index: test/clang-tidy/check_clang_tidy.py
===================================================================
--- test/clang-tidy/check_clang_tidy.py
+++ test/clang-tidy/check_clang_tidy.py
@@ -38,12 +38,16 @@
sys.exit('Not enough arguments.')
input_file_name = sys.argv[1]
+ extension = '.cpp'
+ if (input_file_name.endswith('.c')):
+ extension = '.c'
+
check_name = sys.argv[2]
- temp_file_name = sys.argv[3] + '.cpp'
+ temp_file_name = sys.argv[3] + extension
clang_tidy_extra_args = sys.argv[4:]
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']
with open(input_file_name, 'r') as input_file:
input_text = input_file.read()
Index: clang-tidy/misc/StaticAssertCheck.cpp
===================================================================
--- clang-tidy/misc/StaticAssertCheck.cpp
+++ clang-tidy/misc/StaticAssertCheck.cpp
@@ -27,10 +27,9 @@
: ClangTidyCheck(Name, Context) {}
void StaticAssertCheck::registerMatchers(MatchFinder *Finder) {
- // FIXME: I don't see why this checker couldn't also be interesting for
- // _Static_assert in C11, or static_assert if <assert.h> has been included,
- // but it is currently only enabled for C++11. Investigate.
- if (getLangOpts().CPlusPlus11) {
+ // This checker only makes sense for languages that have static assertion
+ // capabilities: C++11 and C11.
+ if (getLangOpts().CPlusPlus11 || getLangOpts().C11) {
auto IsAlwaysFalse = expr(ignoringParenImpCasts(
expr(anyOf(boolLiteral(equals(false)), integerLiteral(equals(0)),
nullPtrLiteralExpr(), gnuNullExpr()))
@@ -108,6 +107,14 @@
return;
}
+ // FIXME: static_assert is fine in C11 if the macro is defined, otherwise we
+ // should suggest _Static_assert. However, there is no way to determine
+ // whether the macro is defined at the location of the assert without access
+ // to the Preprocessor instance. For right now, always suggest _Static_assert
+ // in C11 mode.
+ StringRef Replacement =
+ getLangOpts().C11 ? "_Static_assert" : "static_assert";
+
SourceLocation AssertLoc = SM.getImmediateMacroCallerLoc(AssertExpansionLoc);
SmallVector<FixItHint, 4> FixItHints;
@@ -115,7 +122,7 @@
if (AssertLoc.isValid() && !AssertLoc.isMacroID() &&
(LastParenLoc = getLastParenLoc(ASTCtx, AssertLoc)).isValid()) {
FixItHints.push_back(
- FixItHint::CreateReplacement(SourceRange(AssertLoc), "static_assert"));
+ FixItHint::CreateReplacement(SourceRange(AssertLoc), Replacement));
std::string StaticAssertMSG = ", \"\"";
if (AssertExprRoot) {
@@ -130,8 +137,8 @@
FixItHint::CreateInsertion(LastParenLoc, StaticAssertMSG));
}
- diag(AssertLoc, "found assert() that could be replaced by static_assert()")
- << FixItHints;
+ diag(AssertLoc, "found assert() that could be replaced by %0()")
+ << Replacement << FixItHints;
}
SourceLocation StaticAssertCheck::getLastParenLoc(const ASTContext *ASTCtx,
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
