[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