Do you have an actual use case for this? I can't come up with a good one
where extra information would actually help. Also, does it make sense to make
the deduplication generic and leave all the other functions unchanged? How
would you apply a vector<SomeOtherReplacement>?
On the other hand, if there is a good use case to use the "context" and the
serialization format supports it, we could also consider just adding that
string. After all and empty string should have a low overhead (compared to
FilePath and ReplacementText).
Another thing that we need to consider eventually is the concept of a
replacement group. What I mean by that is that several replacements need to be
applied atomically, i.e. either all of them or none of them. Imagine you want
to migrate a for-loop. Now, you have one replacement for the actual "for (...)"
and possibly one replacement for each usage of "something[i]". Now, one of the
latter might conflict, e.g. due to macro usage. Now, we need to abandon the
entire for-rewrite. I don't have firm grasp on how we are going to implement
this, but I have the suspicion that dealing with the extra information would
make things harder.
================
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();
----------------
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.
================
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);
+
----------------
Is there a particular reason to move these out of the replacement class?
http://llvm-reviews.chandlerc.com/D1415
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
