maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

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

llvm-svn: 281819
This commit is contained in:
Eric Liu 2016-09-17 12:26:42 +00:00
parent dfbbbc86a1
commit e990652542
2 changed files with 8 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
clang/include/clang/Tooling/Core/Replacement.h
View File

@ -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;
}

12
clang/lib/Tooling/Core/Replacement.cpp
View File

@ -83,11 +83,8 @@ bool operator<(const Replacement &LHS, const Replacement &RHS) {
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::getShiftedCodePosition(unsigned Position) const {
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> applyAllReplacements(StringRef Code,
"<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))