[PATCH] [clang-tidy] Add a checker for swapped literal arguments.
Alexander Kornienko
alexfh at google.com
Mon Jul 14 05:40:48 PDT 2014
================
Comment at: clang-tidy/misc/SwappedArgumentsCheck.cpp:54
@@ +53,3 @@
+ // a fixit for.
+ if (!SM.isWrittenInSameFile(R.getBegin(), R.getEnd()))
+ return StringRef();
----------------
IIUC, isWrittenInSameFile(l1, l2) will return true if l1 and l2 are macro locations. Am I missing something?
================
Comment at: clang-tidy/misc/SwappedArgumentsCheck.cpp:77
@@ +76,3 @@
+
+ auto *LHSCast = dyn_cast<ImplicitCastExpr>(ignoreNoOpCasts(LHS));
+ auto *RHSCast = dyn_cast<ImplicitCastExpr>(ignoreNoOpCasts(RHS));
----------------
I'd prefer "const auto *" (or just "auto", if you want to save a few keystrokes).
================
Comment at: clang-tidy/misc/SwappedArgumentsCheck.cpp:90
@@ +89,3 @@
+ // TODO: We could make use of the edit distance between the argument name
+ // and the name of the passed variable to get additional confidence.
+ if (LHS->getType() == RHS->getType() ||
----------------
Name-based approach may be used independently of argument types. I'd expect x/y, raw/column and other argument pairs using the same type to be swapped occasionally.
================
Comment at: clang-tidy/misc/SwappedArgumentsCheck.cpp:98
@@ +97,3 @@
+ auto D = diag(Call->getLocStart(),
+ "two matching implicit casts, potentially swapped arguments");
+
----------------
"Two matching implicit casts" isn't enough to understand what's wrong here. Maybe something like this
warning: potentially swapped arguments for parameter "x" of type double and parameter "y" of type int
note: argument "0" has type int, argument "10e6" has type double
?
================
Comment at: test/clang-tidy/misc-swapped-arguments.cpp:1
@@ +1,2 @@
+// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s misc-swapped-arguments %t
+// REQUIRES: shell
----------------
Please add a test where the invocations are inside a template with a couple of instantiations.
http://reviews.llvm.org/D4457
More information about the cfe-commits
mailing list