[PATCH] Templatize tooling::deduplicate()

[Edwin Vane]

Hi klimek, silvas, djasper,

To facilitate deduplication of Replacements with extra data attached,
deduplicate() has been generalized to work on vectors of type T.
tooling::Replacement already satisfies all requirements of type T so no
changes to existing code are necessary.

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

Files:
  include/clang/Tooling/Refactoring.h
  lib/Tooling/Refactoring.cpp

Index: include/clang/Tooling/Refactoring.h
===================================================================
--- include/clang/Tooling/Refactoring.h
+++ include/clang/Tooling/Refactoring.h
@@ -114,14 +114,6 @@
   /// \brief Returns a human readable string representation.
   std::string toString() const;
 
-  /// \brief Comparator to be able to use Replacement in std::set for uniquing.
-  class Less {
-  public:
-    bool operator()(const Replacement &R1, const Replacement &R2) const;
-  };
-
-  bool operator==(const Replacement &Other) const;
-
  private:
   void setFromSourceLocation(SourceManager &Sources, SourceLocation Start,
                              unsigned Length, StringRef ReplacementText);
@@ -133,9 +125,15 @@
   std::string ReplacementText;
 };
 
+/// \brief Less-than operator between two Replacements.
+bool operator<(const Replacement &LHS, const Replacement &RHS);
+
+/// \brief Equal-to operator between two Replacements.
+bool operator==(const Replacement &LHS, const Replacement &RHS);
+
 /// \brief A set of Replacements.
 /// FIXME: Change to a vector and deduplicate in the RefactoringTool.
-typedef std::set<Replacement, Replacement::Less> Replacements;
+typedef std::set<Replacement> Replacements;
 
 /// \brief Apply all replacements in \p Replaces to the Rewriter \p Rewrite.
 ///
@@ -164,14 +162,6 @@
 /// applied.
 unsigned shiftedCodePosition(const Replacements& Replaces, unsigned Position);
 
-/// \brief Removes duplicate Replacements and reports if Replacements conflict
-/// with one another.
-///
-/// This function will sort \p Replaces so that conflicts can be reported simply
-/// by offset into \p Replaces and number of elements in the conflict.
-void deduplicate(std::vector<Replacement> &Replaces,
-                 std::vector<Range> &Conflicts);
-
 /// \brief A tool to run refactorings.
 ///
 /// This is a refactoring specific version of \see ClangTool. FrontendActions
@@ -217,6 +207,56 @@
   setFromSourceRange(Sources, Range, ReplacementText);
 }
 
+/// \brief Removes duplicates and reports conflicts between Replacement-like
+/// objects. If T == Replacement, no extra work is necessary to call this
+/// function. Otherwise, T must satisfy these requirements:
+/// \li operator== must exist to compare objects of type T
+/// \li operator< must exist to compare objects of type T
+/// \li T.getOffset() must exist and returns a file offset
+/// \li T.getLength() must exist and returns a source range length
+///
+/// This function will sort \p Replaces so that conflicts can be reported simply
+/// by offset into \p Replaces and number of elements in the conflict.
+template <typename T>
+void deduplicate(std::vector<T> &Replaces,
+                 std::vector<Range> &Conflicts) {
+  if (Replaces.empty())
+    return;
+
+  // Deduplicate
+  std::sort(Replaces.begin(), Replaces.end());
+  std::vector<Replacement>::iterator End =
+      std::unique(Replaces.begin(), Replaces.end());
+  Replaces.erase(End, Replaces.end());
+
+  // Detect conflicts
+  Range ConflictRange(Replaces.front().getOffset(),
+                      Replaces.front().getLength());
+  unsigned ConflictStart = 0;
+  unsigned ConflictLength = 1;
+  for (unsigned i = 1; i < Replaces.size(); ++i) {
+    Range Current(Replaces[i].getOffset(), Replaces[i].getLength());
+    if (ConflictRange.overlapsWith(Current)) {
+      // Extend conflicted range
+      ConflictRange = Range(ConflictRange.getOffset(),
+                            std::max(ConflictRange.getLength(),
+                                     Current.getOffset() + Current.getLength() -
+                                         ConflictRange.getOffset()));
+      ++ConflictLength;
+    } else {
+      if (ConflictLength > 1)
+        Conflicts.push_back(Range(ConflictStart, ConflictLength));
+      ConflictRange = Current;
+      ConflictStart = i;
+      ConflictLength = 1;
+    }
+  }
+
+  if (ConflictLength > 1)
+    Conflicts.push_back(Range(ConflictStart, ConflictLength));
+}
+
+
 } // end namespace tooling
 } // end namespace clang
 
Index: lib/Tooling/Refactoring.cpp
===================================================================
--- lib/Tooling/Refactoring.cpp
+++ lib/Tooling/Refactoring.cpp
@@ -80,20 +80,21 @@
   return result;
 }
 
-bool Replacement::Less::operator()(const Replacement &R1,
-                                   const Replacement &R2) const {
-  if (R1.FilePath != R2.FilePath) return R1.FilePath < R2.FilePath;
-  if (R1.ReplacementRange.getOffset() != R2.ReplacementRange.getOffset())
-    return R1.ReplacementRange.getOffset() < R2.ReplacementRange.getOffset();
-  if (R1.ReplacementRange.getLength() != R2.ReplacementRange.getLength())
-    return R1.ReplacementRange.getLength() < R2.ReplacementRange.getLength();
-  return R1.ReplacementText < R2.ReplacementText;
+bool operator==(const Replacement &LHS, const Replacement &RHS) {
+  return LHS.getOffset() == RHS.getOffset() &&
+         LHS.getLength() == RHS.getLength() &&
+         LHS.getFilePath() == RHS.getFilePath() &&
+         LHS.getReplacementText() == RHS.getReplacementText();
 }
 
-bool Replacement::operator==(const Replacement &Other) const {
-  return ReplacementRange.getOffset() == Other.ReplacementRange.getOffset() &&
-         ReplacementRange.getLength() == Other.ReplacementRange.getLength() &&
-         FilePath == Other.FilePath && ReplacementText == Other.ReplacementText;
+bool operator<(const Replacement &LHS, const Replacement &RHS) {
+  if (LHS.getFilePath() != RHS.getFilePath())
+    return LHS.getFilePath() < RHS.getFilePath();
+  if (LHS.getOffset() != RHS.getOffset())
+    return LHS.getOffset() < RHS.getOffset();
+  if (LHS.getLength() != RHS.getLength())
+    return LHS.getLength() < RHS.getLength();
+  return LHS.getReplacementText() < RHS.getReplacementText();
 }
 
 void Replacement::setFromSourceLocation(SourceManager &Sources,
@@ -202,45 +203,6 @@
   return NewPosition;
 }
 
-void deduplicate(std::vector<Replacement> &Replaces,
-                 std::vector<Range> &Conflicts) {
-  if (Replaces.empty())
-    return;
-
-  // Deduplicate
-  std::sort(Replaces.begin(), Replaces.end(), Replacement::Less());
-  std::vector<Replacement>::iterator End =
-      std::unique(Replaces.begin(), Replaces.end());
-  Replaces.erase(End, Replaces.end());
-
-  // Detect conflicts
-  Range ConflictRange(Replaces.front().getOffset(),
-                      Replaces.front().getLength());
-  unsigned ConflictStart = 0;
-  unsigned ConflictLength = 1;
-  for (unsigned i = 1; i < Replaces.size(); ++i) {
-    Range Current(Replaces[i].getOffset(), Replaces[i].getLength());
-    if (ConflictRange.overlapsWith(Current)) {
-      // Extend conflicted range
-      ConflictRange = Range(ConflictRange.getOffset(),
-                            std::max(ConflictRange.getLength(),
-                                     Current.getOffset() + Current.getLength() -
-                                         ConflictRange.getOffset()));
-      ++ConflictLength;
-    } else {
-      if (ConflictLength > 1)
-        Conflicts.push_back(Range(ConflictStart, ConflictLength));
-      ConflictRange = Current;
-      ConflictStart = i;
-      ConflictLength = 1;
-    }
-  }
-
-  if (ConflictLength > 1)
-    Conflicts.push_back(Range(ConflictStart, ConflictLength));
-}
-
-
 RefactoringTool::RefactoringTool(const CompilationDatabase &Compilations,
                                  ArrayRef<std::string> SourcePaths)
   : ClangTool(Compilations, SourcePaths) {}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1415.1.patch
Type: text/x-patch
Size: 7506 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130815/4ecb2749/attachment.bin>