[PATCH] [clang-tidy] Assert related checkers

Alexander Kornienko alexfh at google.com
Wed Feb 25 10:43:28 PST 2015


================
Comment at: clang-tidy/misc/StaticAssertCheck.cpp:54
@@ +53,3 @@
+void StaticAssertCheck::check(const MatchFinder::MatchResult &Result) {
+  ASTContext *ASTCtx = Result.Context;
+  const LangOptions &Opts = ASTCtx->getLangOpts();
----------------
nit: Do you need ASTCtx to point to a non-const ASTContext?

================
Comment at: clang-tidy/misc/StaticAssertCheck.cpp:72
@@ +71,3 @@
+
+  if (MacroName != "assert" || !Condition->isEvaluatable(*ASTCtx))
+    return;
----------------
I wonder if it's intentional that the other check has an option for assert macro names and this one doesn't. 

================
Comment at: clang-tidy/misc/StaticAssertCheck.cpp:108
@@ +107,3 @@
+const SourceLocation
+StaticAssertCheck::getLastParenLoc(ASTContext *ASTCtx,
+                                   const SourceLocation AssertLoc) {
----------------
Do you need ASTCtx to point to a non-const ASTContext? I only dislike top-level const ;)

Also, please remove the "const" on the second parameter.

================
Comment at: test/clang-tidy/misc-assert-side-effect.cpp:45
@@ +44,3 @@
+
+bool freeFunction() { // a non-member function
+  return true;
----------------
This comment doesn't seem to be very useful.

http://reviews.llvm.org/D7375

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






More information about the cfe-commits mailing list