[PATCH] D15121: A new clang-tidy module to find calls to `std::swap`, and change them to use ADL

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 2 08:17:53 PST 2015


aaron.ballman added inline comments.

================
Comment at: clang-tidy/misc/StdSwapCheck.cpp:86
@@ +85,3 @@
+	  SourceLocation semiLoc = findSemiAfterLocation(swapSourceRange.getEnd(), *Result.Context, false);
+	  assert(semiLoc != SourceLocation() && "Can't find the terminating semicolon" );
+        
----------------
Extra space at the end of the assert before the closing paren.

================
Comment at: clang-tidy/misc/StdSwapCheck.h:19
@@ +18,3 @@
+/// A checker that finds ns-qualified calls to std::swap, and replaces them
+///    with calls that use ADL instead.
+///
----------------
Strange indentation on the second line of the comment.

================
Comment at: test/clang-tidy/misc-StdSwap.cpp:4
@@ +3,3 @@
+#include <utility>
+
+// FIXME: Add something that triggers the check here.
----------------
> The reason that <utility> is included here is that that is where swap is declared.

The usual approach we take in our tests is to declare the STL functionality inside of the source file being tested. For instance, see misc-move-constructor-init.cpp. Another approach, if you require the declaration to be in a header file (which you don't appear to from the checker) is to create a local test file and #include  it with "header" instead of <header>. Relying on whatever STL happens to be found by header search logic makes the tests highly fragile (and bots will happily chirp at you when you do this).



http://reviews.llvm.org/D15121





More information about the cfe-commits mailing list