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

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 250054.
njames93 added a comment.

- Added test cases


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76054/new/

https://reviews.llvm.org/D76054

Files:
  clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-tools-extra/test/clang-apply-replacements/Inputs/identical2/file1.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/identical2/file2.yaml
  
clang-tools-extra/test/clang-apply-replacements/Inputs/identical2/identical2.cpp
  clang-tools-extra/test/clang-apply-replacements/identical2.cpp


Index: clang-tools-extra/test/clang-apply-replacements/identical2.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-apply-replacements/identical2.cpp
@@ -0,0 +1,10 @@
+// RUN: mkdir -p %T/Inputs/identical2
+// RUN: grep -Ev "// *[A-Z-]+:" %S/Inputs/identical2/identical2.cpp > %T/Inputs/identical2/identical2.cpp
+// RUN: sed "s#\$(path)#%/T/Inputs/identical2#" %S/Inputs/identical2/file1.yaml > %T/Inputs/identical2/file1.yaml
+// RUN: sed "s#\$(path)#%/T/Inputs/identical2#" %S/Inputs/identical2/file2.yaml > %T/Inputs/identical2/file2.yaml
+// RUN: clang-apply-replacements %T/Inputs/identical2
+// RUN: FileCheck -input-file=%T/Inputs/identical2/identical2.cpp %S/Inputs/identical2/identical2.cpp
+
+// Similar to identical test but each yaml file contains the same fix twice. 
+// This check ensures that only the duplicated replacements in a single yaml 
+// file are applied twice. Addresses PR45150.
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/identical2/identical2.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/identical2/identical2.cpp
@@ -0,0 +1,2 @@
+class MyType {};
+// CHECK: class MyType00 {};
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/identical2/file2.yaml
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/identical2/file2.yaml
@@ -0,0 +1,19 @@
+---
+MainSourceFile:     identical2.cpp
+Diagnostics:
+  - DiagnosticName: test-identical-insertion
+    DiagnosticMessage:
+      Message: Fix
+      FilePath: $(path)/identical2.cpp
+      FileOffset: 12
+      Replacements:
+        - FilePath:        $(path)/identical2.cpp
+          Offset:          12
+          Length:          0
+          ReplacementText: '0'
+        - FilePath:        $(path)/identical2.cpp
+          Offset:          12
+          Length:          0
+          ReplacementText: '0'
+...
+
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/identical2/file1.yaml
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/identical2/file1.yaml
@@ -0,0 +1,19 @@
+---
+MainSourceFile:     identical2.cpp
+Diagnostics:
+  - DiagnosticName: test-identical-insertion
+    DiagnosticMessage:
+      Message: Fix
+      FilePath: $(path)/identical2.cpp
+      FileOffset: 12
+      Replacements:
+        - FilePath:        $(path)/identical2.cpp
+          Offset:          12
+          Length:          0
+          ReplacementText: '0'
+        - FilePath:        $(path)/identical2.cpp
+          Offset:          12
+          Length:          0
+          ReplacementText: '0'
+...
+
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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits