From 6ec86b3e06a28fa6531dfa2594e35ee157d08bba Mon Sep 17 00:00:00 2001 From: Eric Liu Date: Tue, 25 Sep 2018 08:24:07 +0000 Subject: Deduplicate replacements from diagnostics. Summary: 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. Reviewers: bkramer Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D52264 --- .../lib/Tooling/ApplyReplacements.cpp | 19 +++++++++++++++---- .../Inputs/identical/file1.yaml | 4 ---- .../Inputs/identical/file2.yaml | 14 ++++++++++++++ .../Inputs/identical/identical.cpp | 2 +- .../test/clang-apply-replacements/identical.cpp | 1 + 5 files changed, 31 insertions(+), 9 deletions(-) create mode 100644 clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file2.yaml (limited to 'clang-tools-extra') diff --git a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp index 1970d05c66f..b4799225116 100644 --- a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp +++ b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp @@ -125,7 +125,8 @@ std::error_code collectReplacementsFromDirectory( } /// \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 @@ groupReplacements(const TUReplacements &TUs, const TUDiagnostics &TUDs, llvm::DenseMap> GroupedReplacements; - auto AddToGroup = [&](const tooling::Replacement &R) { + // Deduplicate identical replacements in diagnostics. + // FIXME: Find an efficient way to deduplicate on diagnostics level. + llvm::DenseMap> + 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 @@ groupReplacements(const TUReplacements &TUs, const TUDiagnostics &TUDs, 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. diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file1.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file1.yaml index 1ef5651c4d0..cf273c4820d 100644 --- a/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file1.yaml +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file1.yaml @@ -10,9 +10,5 @@ Diagnostics: Offset: 12 Length: 0 ReplacementText: '0' - - FilePath: $(path)/identical.cpp - Offset: 12 - Length: 0 - ReplacementText: '0' ... diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file2.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file2.yaml new file mode 100644 index 00000000000..cf273c4820d --- /dev/null +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file2.yaml @@ -0,0 +1,14 @@ +--- +MainSourceFile: identical.cpp +Diagnostics: + - DiagnosticName: test-identical-insertion + Message: Fix + FilePath: $(path)/identical.cpp + FileOffset: 12 + Replacements: + - FilePath: $(path)/identical.cpp + Offset: 12 + Length: 0 + ReplacementText: '0' +... + diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/identical.cpp b/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/identical.cpp index bdaab4fc823..bc740fa12f5 100644 --- a/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/identical.cpp +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/identical.cpp @@ -1,2 +1,2 @@ class MyType {}; -// CHECK: class MyType00 {}; +// CHECK: class MyType0 {}; diff --git a/clang-tools-extra/test/clang-apply-replacements/identical.cpp b/clang-tools-extra/test/clang-apply-replacements/identical.cpp index b513f3e63ba..ffbf2e37390 100644 --- a/clang-tools-extra/test/clang-apply-replacements/identical.cpp +++ b/clang-tools-extra/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 -- cgit v1.2.3