ioeric created this revision.
ioeric added a reviewer: bkramer.
Herald added a subscriber: cfe-commits.
After r329813, clang-apply-replacements stopped deduplicating identical
replacements; however, tools like clang-tidy relies on the deduplication of
identical dignostics replacements from different TUs to apply fixes correctly.
This change partially roll back the behavior by deduplicating changes from
diagnostics. Ideally, we should deduplicate on diagnostics level, but we need to
figure out an effecient way.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52264
Files:
clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
test/clang-apply-replacements/Inputs/identical/file1.yaml
test/clang-apply-replacements/Inputs/identical/file2.yaml
test/clang-apply-replacements/Inputs/identical/identical.cpp
test/clang-apply-replacements/identical.cpp
Index: test/clang-apply-replacements/identical.cpp
===================================================================
--- test/clang-apply-replacements/identical.cpp
+++ test/clang-apply-replacements/identical.cpp
@@ -1,5 +1,6 @@
// RUN: mkdir -p %T/Inputs/identical
// RUN: grep -Ev "// *[A-Z-]+:" %S/Inputs/identical/identical.cpp >
%T/Inputs/identical/identical.cpp
// RUN: sed "s#\$(path)#%/T/Inputs/identical#" %S/Inputs/identical/file1.yaml
> %T/Inputs/identical/file1.yaml
+// RUN: sed "s#\$(path)#%/T/Inputs/identical#" %S/Inputs/identical/file2.yaml
> %T/Inputs/identical/file2.yaml
// RUN: clang-apply-replacements %T/Inputs/identical
// RUN: FileCheck -input-file=%T/Inputs/identical/identical.cpp
%S/Inputs/identical/identical.cpp
Index: test/clang-apply-replacements/Inputs/identical/identical.cpp
===================================================================
--- test/clang-apply-replacements/Inputs/identical/identical.cpp
+++ test/clang-apply-replacements/Inputs/identical/identical.cpp
@@ -1,2 +1,2 @@
class MyType {};
-// CHECK: class MyType00 {};
+// CHECK: class MyType0 {};
Index: test/clang-apply-replacements/Inputs/identical/file2.yaml
===================================================================
--- test/clang-apply-replacements/Inputs/identical/file2.yaml
+++ test/clang-apply-replacements/Inputs/identical/file2.yaml
@@ -10,9 +10,5 @@
Offset: 12
Length: 0
ReplacementText: '0'
- - FilePath: $(path)/identical.cpp
- Offset: 12
- Length: 0
- ReplacementText: '0'
...
Index: test/clang-apply-replacements/Inputs/identical/file1.yaml
===================================================================
--- test/clang-apply-replacements/Inputs/identical/file1.yaml
+++ test/clang-apply-replacements/Inputs/identical/file1.yaml
@@ -10,9 +10,5 @@
Offset: 12
Length: 0
ReplacementText: '0'
- - FilePath: $(path)/identical.cpp
- Offset: 12
- Length: 0
- ReplacementText: '0'
...
Index: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===================================================================
--- clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -125,7 +125,8 @@
}
/// \brief Extract replacements from collected TranslationUnitReplacements and
-/// TranslationUnitDiagnostics and group them per file.
+/// TranslationUnitDiagnostics and group them per file. Identical replacements
+/// from diagnostics are deduplicated.
///
/// \param[in] TUs Collection of all found and deserialized
/// TranslationUnitReplacements.
@@ -142,10 +143,20 @@
llvm::DenseMap<const FileEntry *, std::vector<tooling::Replacement>>
GroupedReplacements;
- auto AddToGroup = [&](const tooling::Replacement &R) {
+ // Deduplicate identical replacements in diagnostics.
+ // FIXME: Find an efficient way to deduplicate on diagnostics level.
+ llvm::DenseMap<const FileEntry *, std::set<tooling::Replacement>>
+ DiagReplacements;
+
+ auto AddToGroup = [&](const tooling::Replacement &R, bool FromDiag) {
// Use the file manager to deduplicate paths. FileEntries are
// automatically canonicalized.
if (const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath()))
{
+ if (FromDiag) {
+ auto &Replaces = DiagReplacements[Entry];
+ if (!Replaces.insert(R).second)
+ return;
+ }
GroupedReplacements[Entry].push_back(R);
} else if (Warned.insert(R.getFilePath()).second) {
errs() << "Described file '" << R.getFilePath()
@@ -155,13 +166,13 @@
for (const auto &TU : TUs)
for (const tooling::Replacement &R : TU.Replacements)
- AddToGroup(R);
+ AddToGroup(R, false);
for (const auto &TU : TUDs)
for (const auto &D : TU.Diagnostics)
for (const auto &Fix : D.Fix)
for (const tooling::Replacement &R : Fix.second)
- AddToGroup(R);
+ AddToGroup(R, true);
// Sort replacements per file to keep consistent behavior when
// clang-apply-replacements run on differents machine.
Index: test/clang-apply-replacements/identical.cpp
===================================================================
--- test/clang-apply-replacements/identical.cpp
+++ test/clang-apply-replacements/identical.cpp
@@ -1,5 +1,6 @@
// RUN: mkdir -p %T/Inputs/identical
// RUN: grep -Ev "// *[A-Z-]+:" %S/Inputs/identical/identical.cpp > %T/Inputs/identical/identical.cpp
// RUN: sed "s#\$(path)#%/T/Inputs/identical#" %S/Inputs/identical/file1.yaml > %T/Inputs/identical/file1.yaml
+// RUN: sed "s#\$(path)#%/T/Inputs/identical#" %S/Inputs/identical/file2.yaml > %T/Inputs/identical/file2.yaml
// RUN: clang-apply-replacements %T/Inputs/identical
// RUN: FileCheck -input-file=%T/Inputs/identical/identical.cpp %S/Inputs/identical/identical.cpp
Index: test/clang-apply-replacements/Inputs/identical/identical.cpp
===================================================================
--- test/clang-apply-replacements/Inputs/identical/identical.cpp
+++ test/clang-apply-replacements/Inputs/identical/identical.cpp
@@ -1,2 +1,2 @@
class MyType {};
-// CHECK: class MyType00 {};
+// CHECK: class MyType0 {};
Index: test/clang-apply-replacements/Inputs/identical/file2.yaml
===================================================================
--- test/clang-apply-replacements/Inputs/identical/file2.yaml
+++ test/clang-apply-replacements/Inputs/identical/file2.yaml
@@ -10,9 +10,5 @@
Offset: 12
Length: 0
ReplacementText: '0'
- - FilePath: $(path)/identical.cpp
- Offset: 12
- Length: 0
- ReplacementText: '0'
...
Index: test/clang-apply-replacements/Inputs/identical/file1.yaml
===================================================================
--- test/clang-apply-replacements/Inputs/identical/file1.yaml
+++ test/clang-apply-replacements/Inputs/identical/file1.yaml
@@ -10,9 +10,5 @@
Offset: 12
Length: 0
ReplacementText: '0'
- - FilePath: $(path)/identical.cpp
- Offset: 12
- Length: 0
- ReplacementText: '0'
...
Index: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===================================================================
--- clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -125,7 +125,8 @@
}
/// \brief Extract replacements from collected TranslationUnitReplacements and
-/// TranslationUnitDiagnostics and group them per file.
+/// TranslationUnitDiagnostics and group them per file. Identical replacements
+/// from diagnostics are deduplicated.
///
/// \param[in] TUs Collection of all found and deserialized
/// TranslationUnitReplacements.
@@ -142,10 +143,20 @@
llvm::DenseMap<const FileEntry *, std::vector<tooling::Replacement>>
GroupedReplacements;
- auto AddToGroup = [&](const tooling::Replacement &R) {
+ // Deduplicate identical replacements in diagnostics.
+ // FIXME: Find an efficient way to deduplicate on diagnostics level.
+ llvm::DenseMap<const FileEntry *, std::set<tooling::Replacement>>
+ DiagReplacements;
+
+ auto AddToGroup = [&](const tooling::Replacement &R, bool FromDiag) {
// Use the file manager to deduplicate paths. FileEntries are
// automatically canonicalized.
if (const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath())) {
+ if (FromDiag) {
+ auto &Replaces = DiagReplacements[Entry];
+ if (!Replaces.insert(R).second)
+ return;
+ }
GroupedReplacements[Entry].push_back(R);
} else if (Warned.insert(R.getFilePath()).second) {
errs() << "Described file '" << R.getFilePath()
@@ -155,13 +166,13 @@
for (const auto &TU : TUs)
for (const tooling::Replacement &R : TU.Replacements)
- AddToGroup(R);
+ AddToGroup(R, false);
for (const auto &TU : TUDs)
for (const auto &D : TU.Diagnostics)
for (const auto &Fix : D.Fix)
for (const tooling::Replacement &R : Fix.second)
- AddToGroup(R);
+ AddToGroup(R, true);
// Sort replacements per file to keep consistent behavior when
// clang-apply-replacements run on differents machine.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
