[PATCH] [clang-tidy] Add a checker for swapped literal arguments.
Daniel Jasper
djasper at google.com
Thu Jul 10 03:20:17 PDT 2014
================
Comment at: clang-tidy/misc/CMakeLists.txt:6
@@ -5,2 +5,3 @@
BoolPointerImplicitConversion.cpp
+ ImplicitArgumentConversion.cpp
MiscTidyModule.cpp
----------------
ImplicitArgumentConversion is not exactly what this does, I think. Maybe SwappedArgumentsCheck?
================
Comment at: clang-tidy/misc/ImplicitArgumentConversion.cpp:33
@@ +32,3 @@
+ for (auto *Parm : Call->arguments()) {
+ // We only care about literals.
+ const Expr *PP = Parm->IgnoreImpCasts();
----------------
Why do we only care about literals?
================
Comment at: clang-tidy/misc/ImplicitArgumentConversion.cpp:34
@@ +33,3 @@
+ // We only care about literals.
+ const Expr *PP = Parm->IgnoreImpCasts();
+ if (!isa<CharacterLiteral>(PP) && !isa<CXXBoolLiteralExpr>(PP) &&
----------------
Maybe IgnoreParenImpCasts()?
================
Comment at: clang-tidy/misc/ImplicitArgumentConversion.cpp:52
@@ +51,3 @@
+ // most likely a swap.
+ if (NumConversions >= 2 && DeclTypes == CallTypes) {
+ diag(Call->getLocStart(),
----------------
Mhmm. This seems like a very crude way of doing this. Might be ok for a first iteration, but it seems like we could actually be much smarter and check if there is a re-ordering that would fit better (and even extend the analysis to more than two parameters, no)?
================
Comment at: clang-tidy/misc/ImplicitArgumentConversion.h:18
@@ +17,3 @@
+
+/// \brief
+class ImplicitArgumentConversion : public ClangTidyCheck {
----------------
This is VERY brief ;-)
http://reviews.llvm.org/D4457
More information about the cfe-commits
mailing list