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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to