clang-apply-replacements: Deduplicate paths with FileManager.

Bucket replacements by FileEntry instead of path. The same file with
different paths is very common, relative #include paths and symlinks can
easily create them. When that occurs we would apply the fix twice.

git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@217440 91177308-0d34-0410-b5e6-96231b3b80d8

3 files changed, 41 insertions, 46 deletions

diff --git a/clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h b/clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
index af8c5258..979b112d 100644
--- a/clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
+++ b/clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
@@ -45,8 +45,9 @@ TUReplacements;
 typedef std::vector<std::string> TUReplacementFiles;
 
 /// \brief Map mapping file name to Replacements targeting that file.
-typedef llvm::StringMap<std::vector<clang::tooling::Replacement> >
-FileToReplacementsMap;
+typedef llvm::DenseMap<const clang::FileEntry *,
+                       std::vector<clang::tooling::Replacement>>
+    FileToReplacementsMap;
 
 /// \brief Recursively descends through a directory structure rooted at \p
 /// Directory and attempts to deserialize *.yaml files as
diff --git a/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp b/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
index f90e29a8..958734de 100644
--- a/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ b/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -140,33 +140,24 @@ static bool deduplicateAndDetectConflicts(FileToReplacementsMap &Replacements,
                                           SourceManager &SM) {
   bool conflictsFound = false;
 
-  for (FileToReplacementsMap::iterator I = Replacements.begin(),
-                                       E = Replacements.end();
-       I != E; ++I) {
-
-    const FileEntry *Entry = SM.getFileManager().getFile(I->getKey());
-    if (!Entry) {
-      errs() << "Described file '" << I->getKey()
-             << "' doesn't exist. Ignoring...\n";
-      continue;
-    }
+  for (auto &FileAndReplacements : Replacements) {
+    const FileEntry *Entry = FileAndReplacements.first;
+    auto &Replacements = FileAndReplacements.second;
+    assert(Entry != nullptr && "No file entry!");
 
     std::vector<tooling::Range> Conflicts;
-    tooling::deduplicate(I->getValue(), Conflicts);
+    tooling::deduplicate(FileAndReplacements.second, Conflicts);
 
     if (Conflicts.empty())
       continue;
 
     conflictsFound = true;
 
-    errs() << "There are conflicting changes to " << I->getKey() << ":\n";
+    errs() << "There are conflicting changes to " << Entry->getName() << ":\n";
 
-    for (std::vector<tooling::Range>::const_iterator
-             ConflictI = Conflicts.begin(),
-             ConflictE = Conflicts.end();
-         ConflictI != ConflictE; ++ConflictI) {
-      ArrayRef<tooling::Replacement> ConflictingReplacements(
-          &I->getValue()[ConflictI->getOffset()], ConflictI->getLength());
+    for (const tooling::Range &Conflict : Conflicts) {
+      auto ConflictingReplacements = llvm::makeArrayRef(
+          &Replacements[Conflict.getOffset()], Conflict.getLength());
       reportConflict(Entry, ConflictingReplacements, SM);
     }
   }
@@ -179,14 +170,20 @@ bool mergeAndDeduplicate(const TUReplacements &TUs,
                          clang::SourceManager &SM) {
 
   // Group all replacements by target file.
-  for (TUReplacements::const_iterator TUI = TUs.begin(), TUE = TUs.end();
-       TUI != TUE; ++TUI)
-    for (std::vector<tooling::Replacement>::const_iterator
-             RI = TUI->Replacements.begin(),
-             RE = TUI->Replacements.end();
-         RI != RE; ++RI)
-      GroupedReplacements[RI->getFilePath()].push_back(*RI);
-
+  std::set<StringRef> Warned;
+  for (const auto &TU : TUs) {
+    for (const tooling::Replacement &R : TU.Replacements) {
+      // Use the file manager to deduplicate paths. FileEntries are
+      // automatically canonicalized.
+      const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath());
+      if (!Entry && Warned.insert(R.getFilePath()).second) {
+        errs() << "Described file '" << R.getFilePath()
+               << "' doesn't exist. Ignoring...\n";
+        continue;
+      }
+      GroupedReplacements[Entry].push_back(R);
+    }
+  }
 
   // Ask clang to deduplicate and report conflicts.
   if (deduplicateAndDetectConflicts(GroupedReplacements, SM))
@@ -204,10 +201,8 @@ bool applyReplacements(const FileToReplacementsMap &GroupedReplacements,
   // data structure for applying replacements. Rewriter certainly doesn't care.
   // However, until we nail down the design of ReplacementGroups, might as well
   // leave this as is.
-  for (FileToReplacementsMap::const_iterator I = GroupedReplacements.begin(),
-                                             E = GroupedReplacements.end();
-       I != E; ++I) {
-    if (!tooling::applyAllReplacements(I->getValue(), Rewrites))
+  for (const auto &FileAndReplacements : GroupedReplacements) {
+    if (!tooling::applyAllReplacements(FileAndReplacements.second, Rewrites))
       return false;
   }
 
diff --git a/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp b/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
index 4559a26a..94ad4638 100644
--- a/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
+++ b/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
@@ -246,35 +246,34 @@ int main(int argc, char **argv) {
 
   Rewriter ReplacementsRewriter(SM, LangOptions());
 
-  for (FileToReplacementsMap::const_iterator I = GroupedReplacements.begin(),
-                                             E = GroupedReplacements.end();
-       I != E; ++I) {
-
-    std::string NewFileData;
-
+  for (const auto &FileAndReplacements : GroupedReplacements) {
     // This shouldn't happen but if a file somehow has no replacements skip to
     // next file.
-    if (I->getValue().empty())
+    if (FileAndReplacements.second.empty())
       continue;
 
-    if (!applyReplacements(I->getValue(), NewFileData, Diagnostics)) {
-      errs() << "Failed to apply replacements to " << I->getKey() << "\n";
+    std::string NewFileData;
+    const char *FileName = FileAndReplacements.first->getName();
+    if (!applyReplacements(FileAndReplacements.second, NewFileData,
+                           Diagnostics)) {
+      errs() << "Failed to apply replacements to " << FileName << "\n";
       continue;
     }
 
     // Apply formatting if requested.
-    if (DoFormat && !applyFormatting(I->getValue(), NewFileData, NewFileData,
-                                     FormatStyle, Diagnostics)) {
-      errs() << "Failed to apply reformatting replacements for " << I->getKey()
+    if (DoFormat &&
+        !applyFormatting(FileAndReplacements.second, NewFileData, NewFileData,
+                         FormatStyle, Diagnostics)) {
+      errs() << "Failed to apply reformatting replacements for " << FileName
              << "\n";
       continue;
     }
 
     // Write new file to disk
     std::error_code EC;
-    llvm::raw_fd_ostream FileStream(I->getKey(), EC, llvm::sys::fs::F_Text);
+    llvm::raw_fd_ostream FileStream(FileName, EC, llvm::sys::fs::F_Text);
     if (EC) {
-      llvm::errs() << "Could not open " << I->getKey() << " for writing\n";
+      llvm::errs() << "Could not open " << FileName << " for writing\n";
       continue;
     }