[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