[PATCH] D76054: [clang-apply-replacements] No longer deduplucates replacements from the same TU

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: aaron.ballman, gribozavr2, AlexanderLanin.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
njames93 added a comment.

Test cases will follow, just time constrained for now.


clang-apply-replacements currently deduplicates all diagnostic replacements. However if you get a duplicated replacement from one TU then its expected that it should not be deduplicated. This goes some way to solving export-fixes to yaml adds extra newlines and breaks offsets. <https://bugs.llvm.org/show_bug.cgi?id=45150>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76054

Files:
  clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp


Index: clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===================================================================
--- clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -145,16 +145,21 @@
 
   // Deduplicate identical replacements in diagnostics.
   // FIXME: Find an efficient way to deduplicate on diagnostics level.
-  llvm::DenseMap<const FileEntry *, std::set<tooling::Replacement>>
+  llvm::DenseMap<const FileEntry *,
+                 std::map<tooling::Replacement,
+                          const tooling::TranslationUnitDiagnostics *>>
       DiagReplacements;
 
-  auto AddToGroup = [&](const tooling::Replacement &R, bool FromDiag) {
+  auto AddToGroup = [&](const tooling::Replacement &R,
+                        const tooling::TranslationUnitDiagnostics *FromTU) {
     // Use the file manager to deduplicate paths. FileEntries are
     // automatically canonicalized.
     if (auto Entry = SM.getFileManager().getFile(R.getFilePath())) {
-      if (FromDiag) {
+      if (FromTU) {
         auto &Replaces = DiagReplacements[*Entry];
-        if (!Replaces.insert(R).second)
+        if (Replaces.find(R) == Replaces.end())
+          Replaces.emplace(R, FromTU);
+        else if (Replaces.at(R) != FromTU)
           return;
       }
       GroupedReplacements[*Entry].push_back(R);
@@ -166,14 +171,14 @@
 
   for (const auto &TU : TUs)
     for (const tooling::Replacement &R : TU.Replacements)
-      AddToGroup(R, false);
+      AddToGroup(R, nullptr);
 
   for (const auto &TU : TUDs)
     for (const auto &D : TU.Diagnostics)
       if (const auto *ChoosenFix = tooling::selectFirstFix(D)) {
         for (const auto &Fix : *ChoosenFix)
           for (const tooling::Replacement &R : Fix.second)
-            AddToGroup(R, true);
+            AddToGroup(R, &TU);
       }
 
   // Sort replacements per file to keep consistent behavior when


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76054.249879.patch
Type: text/x-patch
Size: 2013 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200312/43536f1c/attachment-0001.bin>