[PATCH] Templatize tooling::deduplicate()

[Edwin Vane]

  I do actually have a case for this. The replacement applicator tool, D1382, is being turned into a more general-purpose applicator called `clang-replace`. See my response to @silvas for an explanation of that case.

  However, I submitted this patch in support of my work on clang-replace before I actually started using this function. As you'll see from D1382, I've split the patch up so that replacement application isn't included yet. You're right about having to templatize other functions too and now this solution seems far from ideal as the requirements on T are starting to pile up. Also, even with just deduplication I found that I actually wanted to accumulate the context of duplicate replacements instead of losing context as part of the `std::unique` so clang-replace has to do extra work anyway to collect it.

  FWIW, if Context were added as a string to Replacement we should exclude it from Replacement comparison I would think. It's mean to be an annotation. I don't really like the idea but it was suggested as a way for the main source file, as provided by the migrator, to be included in this general-purpose replacement serialization file. I would prefer something a little more concrete.


================
Comment at: lib/Tooling/Refactoring.cpp:91
@@ +90,3 @@
+bool operator<(const Replacement &LHS, const Replacement &RHS) {
+  if (LHS.getFilePath() != RHS.getFilePath())
+    return LHS.getFilePath() < RHS.getFilePath();
----------------
Daniel Jasper wrote:
> I'd keep the original order of comparisons. String comparisons are relatively expensive. Thus, I'd do that only if offset and length are both equal.
Thisi **is** the original order, I just copied it. However, I agree with fixing up the comparison order.

================
Comment at: include/clang/Tooling/Refactoring.h:129
@@ +128,3 @@
+/// \brief Less-than operator between two Replacements.
+bool operator<(const Replacement &LHS, const Replacement &RHS);
+
----------------
Daniel Jasper wrote:
> Is there a particular reason to move these out of the replacement class?
Effective C++ 3rd Ed Item 23.


http://llvm-reviews.chandlerc.com/D1415