Author: ioeric Date: Sat Sep 17 07:26:42 2016 New Revision: 281819 URL: http://llvm.org/viewvc/llvm-project?rev=281819&view=rev Log: When replacements have the same offset, make replacements with smaller length order first in the set.
Summary: No behavioral change intended. The change makes iterating the replacements set more intuitive in Replacements class implementation. Previously, insertion is ordered before an deletion/replacement with the same offset, which is counter-intuitive for implementation, especially for a followup patch to support adding insertions around replacements. With the current ordering, we only need to make `applyAllReplacements` iterate the replacements set reversely when applying them so that deletion/replacement is still applied before insertion with the same offset. Reviewers: klimek, djasper Subscribers: klimek, cfe-commits Differential Revision: https://reviews.llvm.org/D24663 Modified: cfe/trunk/include/clang/Tooling/Core/Replacement.h cfe/trunk/lib/Tooling/Core/Replacement.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=281819&r1=281818&r2=281819&view=diff ============================================================================== --- cfe/trunk/include/clang/Tooling/Core/Replacement.h (original) +++ cfe/trunk/include/clang/Tooling/Core/Replacement.h Sat Sep 17 07:26:42 2016 @@ -151,6 +151,7 @@ class Replacements { public: typedef ReplacementsImpl::const_iterator const_iterator; + typedef ReplacementsImpl::const_reverse_iterator const_reverse_iterator; Replacements() = default; @@ -193,6 +194,10 @@ class Replacements { const_iterator end() const { return Replaces.end(); } + const_reverse_iterator rbegin() const { return Replaces.rbegin(); } + + const_reverse_iterator rend() const { return Replaces.rend(); } + bool operator==(const Replacements &RHS) const { return Replaces == RHS.Replaces; } Modified: cfe/trunk/lib/Tooling/Core/Replacement.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Replacement.cpp?rev=281819&r1=281818&r2=281819&view=diff ============================================================================== --- cfe/trunk/lib/Tooling/Core/Replacement.cpp (original) +++ cfe/trunk/lib/Tooling/Core/Replacement.cpp Sat Sep 17 07:26:42 2016 @@ -83,11 +83,8 @@ bool operator<(const Replacement &LHS, c if (LHS.getOffset() != RHS.getOffset()) return LHS.getOffset() < RHS.getOffset(); - // Apply longer replacements first, specifically so that deletions are - // executed before insertions. It is (hopefully) never the intention to - // delete parts of newly inserted code. if (LHS.getLength() != RHS.getLength()) - return LHS.getLength() > RHS.getLength(); + return LHS.getLength() < RHS.getLength(); if (LHS.getFilePath() != RHS.getFilePath()) return LHS.getFilePath() < RHS.getFilePath(); @@ -407,9 +404,7 @@ unsigned Replacements::getShiftedCodePos bool applyAllReplacements(const Replacements &Replaces, Rewriter &Rewrite) { bool Result = true; - for (Replacements::const_iterator I = Replaces.begin(), - E = Replaces.end(); - I != E; ++I) { + for (auto I = Replaces.rbegin(), E = Replaces.rend(); I != E; ++I) { if (I->isApplicable()) { Result = I->apply(Rewrite) && Result; } else { @@ -436,8 +431,7 @@ llvm::Expected<std::string> applyAllRepl "<stdin>", 0, llvm::MemoryBuffer::getMemBuffer(Code, "<stdin>")); FileID ID = SourceMgr.createFileID(Files.getFile("<stdin>"), SourceLocation(), clang::SrcMgr::C_User); - for (Replacements::const_iterator I = Replaces.begin(), E = Replaces.end(); - I != E; ++I) { + for (auto I = Replaces.rbegin(), E = Replaces.rend(); I != E; ++I) { Replacement Replace("<stdin>", I->getOffset(), I->getLength(), I->getReplacementText()); if (!Replace.apply(Rewrite)) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
