Author: ioeric Date: Fri Oct 14 04:32:06 2016 New Revision: 284219 URL: http://llvm.org/viewvc/llvm-project?rev=284219&view=rev Log: Deduplicate sets of replacements by file names.
Summary: If there are multiple <File, Replacements> pairs with the same file path after removing dots, we only keep one pair (with path after dots being removed) and discard the rest. Reviewers: djasper Subscribers: klimek, hokein, bkramer, cfe-commits Differential Revision: https://reviews.llvm.org/D25565 Modified: cfe/trunk/include/clang/Tooling/Core/Replacement.h cfe/trunk/include/clang/Tooling/Refactoring.h cfe/trunk/lib/Tooling/Core/Replacement.cpp cfe/trunk/lib/Tooling/Refactoring.cpp cfe/trunk/unittests/Tooling/RefactoringTest.cpp Modified: cfe/trunk/include/clang/Tooling/Core/Replacement.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Core/Replacement.h?rev=284219&r1=284218&r2=284219&view=diff ============================================================================== --- cfe/trunk/include/clang/Tooling/Core/Replacement.h (original) +++ cfe/trunk/include/clang/Tooling/Core/Replacement.h Fri Oct 14 04:32:06 2016 @@ -230,8 +230,6 @@ private: Replacements(const_iterator Begin, const_iterator End) : Replaces(Begin, End) {} - Replacements mergeReplacements(const ReplacementsImpl &Second) const; - // Returns `R` with new range that refers to code after `Replaces` being // applied. Replacement getReplacementInChangedCode(const Replacement &R) const; @@ -294,10 +292,11 @@ std::vector<Range> calculateRangesAfterReplacements(const Replacements &Replaces, const std::vector<Range> &Ranges); -/// \brief Groups a random set of replacements by file path. Replacements -/// related to the same file entry are put into the same vector. -std::map<std::string, Replacements> -groupReplacementsByFile(const Replacements &Replaces); +/// \brief If there are multiple <File, Replacements> pairs with the same file +/// path after removing dots, we only keep one pair (with path after dots being +/// removed) and discard the rest. +std::map<std::string, Replacements> groupReplacementsByFile( + const std::map<std::string, Replacements> &FileToReplaces); template <typename Node> Replacement::Replacement(const SourceManager &Sources, Modified: cfe/trunk/include/clang/Tooling/Refactoring.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring.h?rev=284219&r1=284218&r2=284219&view=diff ============================================================================== --- cfe/trunk/include/clang/Tooling/Refactoring.h (original) +++ cfe/trunk/include/clang/Tooling/Refactoring.h Fri Oct 14 04:32:06 2016 @@ -55,6 +55,9 @@ public: /// \brief Apply all stored replacements to the given Rewriter. /// + /// FileToReplaces will be deduplicated with `groupReplacementsByFile` before + /// application. + /// /// Replacement applications happen independently of the success of other /// applications. /// @@ -75,6 +78,9 @@ private: /// /// \pre Replacements must be conflict-free. /// +/// FileToReplaces will be deduplicated with `groupReplacementsByFile` before +/// application. +/// /// Replacement applications happen independently of the success of other /// applications. /// Modified: cfe/trunk/lib/Tooling/Core/Replacement.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Replacement.cpp?rev=284219&r1=284218&r2=284219&view=diff ============================================================================== --- cfe/trunk/lib/Tooling/Core/Replacement.cpp (original) +++ cfe/trunk/lib/Tooling/Core/Replacement.cpp Fri Oct 14 04:32:06 2016 @@ -21,6 +21,7 @@ #include "clang/Lex/Lexer.h" #include "clang/Rewrite/Core/Rewriter.h" #include "llvm/Support/FileSystem.h" +#include "llvm/Support/Path.h" #include "llvm/Support/raw_os_ostream.h" namespace clang { @@ -564,13 +565,15 @@ llvm::Expected<std::string> applyAllRepl return Result; } -std::map<std::string, Replacements> -groupReplacementsByFile(const Replacements &Replaces) { - std::map<std::string, Replacements> FileToReplaces; - for (const auto &Replace : Replaces) - // We can ignore the Error here since \p Replaces is already conflict-free. - FileToReplaces[Replace.getFilePath()].add(Replace); - return FileToReplaces; +std::map<std::string, Replacements> groupReplacementsByFile( + const std::map<std::string, Replacements> &FileToReplaces) { + std::map<std::string, Replacements> Result; + for (const auto &Entry : FileToReplaces) { + llvm::SmallString<256> CleanPath(Entry.first.data()); + llvm::sys::path::remove_dots(CleanPath, /*remove_dot_dot=*/true); + Result[CleanPath.str()] = std::move(Entry.second); + } + return Result; } } // end namespace tooling Modified: cfe/trunk/lib/Tooling/Refactoring.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring.cpp?rev=284219&r1=284218&r2=284219&view=diff ============================================================================== --- cfe/trunk/lib/Tooling/Refactoring.cpp (original) +++ cfe/trunk/lib/Tooling/Refactoring.cpp Fri Oct 14 04:32:06 2016 @@ -57,7 +57,7 @@ int RefactoringTool::runAndSave(Frontend bool RefactoringTool::applyAllReplacements(Rewriter &Rewrite) { bool Result = true; - for (const auto &Entry : FileToReplaces) + for (const auto &Entry : groupReplacementsByFile(FileToReplaces)) Result = tooling::applyAllReplacements(Entry.second, Rewrite) && Result; return Result; } @@ -73,7 +73,7 @@ bool formatAndApplyAllReplacements( FileManager &Files = SM.getFileManager(); bool Result = true; - for (const auto &FileAndReplaces : FileToReplaces) { + for (const auto &FileAndReplaces : groupReplacementsByFile(FileToReplaces)) { const std::string &FilePath = FileAndReplaces.first; auto &CurReplaces = FileAndReplaces.second; Modified: cfe/trunk/unittests/Tooling/RefactoringTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringTest.cpp?rev=284219&r1=284218&r2=284219&view=diff ============================================================================== --- cfe/trunk/unittests/Tooling/RefactoringTest.cpp (original) +++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp Fri Oct 14 04:32:06 2016 @@ -972,5 +972,23 @@ TEST_F(MergeReplacementsTest, Overlappin toReplacements({{"", 0, 3, "cc"}, {"", 3, 3, "dd"}})); } +TEST(DeduplicateByFileTest, LeaveLeadingDotDot) { + std::map<std::string, Replacements> FileToReplaces; + FileToReplaces["../../a/b/.././c.h"] = Replacements(); + FileToReplaces["../../a/c.h"] = Replacements(); + FileToReplaces = groupReplacementsByFile(FileToReplaces); + EXPECT_EQ(1u, FileToReplaces.size()); + EXPECT_EQ("../../a/c.h", FileToReplaces.begin()->first); +} + +TEST(DeduplicateByFileTest, RemoveDotSlash) { + std::map<std::string, Replacements> FileToReplaces; + FileToReplaces["./a/b/.././c.h"] = Replacements(); + FileToReplaces["a/c.h"] = Replacements(); + FileToReplaces = groupReplacementsByFile(FileToReplaces); + EXPECT_EQ(1u, FileToReplaces.size()); + EXPECT_EQ("a/c.h", FileToReplaces.begin()->first); +} + } // end namespace tooling } // end namespace clang _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
