ajohnson-uoregon updated this revision to Diff 364282.
ajohnson-uoregon retitled this revision from "[clang][Rewriter] patch to fix
bug with ReplaceText erasing too many characters when text has been inserted
before the replace location. " to "[clang][Rewriter] patch to fix bug with
ReplaceText erasing too many characters when text has been inserted before the
replace location.".
ajohnson-uoregon added a comment.
Making clang-format happy
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107503/new/
https://reviews.llvm.org/D107503
Files:
clang/include/clang/Rewrite/Core/Rewriter.h
clang/lib/Rewrite/Rewriter.cpp
clang/unittests/Rewrite/RewriterTest.cpp
Index: clang/unittests/Rewrite/RewriterTest.cpp
===================================================================
--- clang/unittests/Rewrite/RewriterTest.cpp
+++ clang/unittests/Rewrite/RewriterTest.cpp
@@ -78,34 +78,54 @@
EXPECT_EQ(T.Rewrite.getRewrittenText(T.makeCharRange(42, 47)), "0;");
}
-TEST(Rewriter, ReplaceAfterInsert) {
+TEST(Rewriter, ReplaceAfterInsertNoOpts) {
// Check that remove/insert after insert behaves correctly based on the
- // RewriteOptions passed.
+ // RewriteOptions passed. This should erase extra text.
- StringRef code = "int main(int argc, char *argv[]) { return argc; } double f(int text_to_erase) {}";
- // ^ insert "int answer = 42;"
- // ^ replace with answer
- // ^ replace with 42
- // extra text to erase ^
+ StringRef code = "int main(int argc, char *argv[]) { return argc; } double "
+ "f(int text_to_erase) {}";
+ // insert "int answer = 42;" ^
+ // replace with 42 ^
+ // extra text to erase ^
std::unique_ptr<ASTUnit> AST = tooling::buildASTFromCode(code);
ASTContext &C = AST->getASTContext();
- Rewriter Rewrite = Rewriter(C.getSourceManager(), C.getLangOpts());;
+ Rewriter Rewrite = Rewriter(C.getSourceManager(), C.getLangOpts());
SourceLocation FileStart = AST->getStartOfMainFileID();
SourceLocation insert_point = FileStart.getLocWithOffset(34);
Rewrite.InsertText(insert_point, "int answer = 42;");
- SourceRange replace_range = SourceRange(FileStart.getLocWithOffset(42),
+ SourceRange replace_range = SourceRange(FileStart.getLocWithOffset(34),
FileStart.getLocWithOffset(45));
+ Rewrite.ReplaceText(replace_range, "42;");
+ EXPECT_EQ(
+ Rewrite.getRewrittenText(SourceRange(FileStart.getLocWithOffset(33),
+ FileStart.getLocWithOffset(65))),
+ "{int answer = 42;42; text_to_erase");
+}
+
+TEST(Rewriter, ReplaceAfterInsertOpts) {
+ // Check that remove/insert after insert behaves correctly based on the
+ // RewriteOptions passed. This should NOT erase extra text.
+
+ StringRef code = "int main(int argc, char *argv[]) { return argc; } double "
+ "f(int text_to_erase) {}";
+ // insert "int answer = 42;" ^
+ // replace with 42 ^
+ // extra text to erase ^
+ std::unique_ptr<ASTUnit> AST = tooling::buildASTFromCode(code);
+ ASTContext &C = AST->getASTContext();
+ Rewriter Rewrite = Rewriter(C.getSourceManager(), C.getLangOpts());
+ SourceLocation FileStart = AST->getStartOfMainFileID();
+ SourceLocation insert_point = FileStart.getLocWithOffset(34);
+ Rewrite.InsertText(insert_point, "int answer = 42;");
+ SourceRange replace_range = SourceRange(FileStart.getLocWithOffset(34),
+ FileStart.getLocWithOffset(46));
Rewriter::RewriteOptions opts;
opts.IncludeInsertsAtBeginOfRange = false;
- Rewrite.ReplaceText(replace_range, "answer", opts);
- EXPECT_EQ(Rewrite.getRewrittenText(
- SourceRange(FileStart.getLocWithOffset(33), FileStart.getLocWithOffset(48))),
- "{int answer = 42; return answer; }");
- Rewrite.ReplaceText(replace_range, "42", Rewriter::RewriteOptions());
- EXPECT_EQ(Rewrite.getRewrittenText(
- SourceRange(FileStart.getLocWithOffset(33), FileStart.getLocWithOffset(55))),
- "{int answer = 42; return 42; } double");
+ Rewrite.ReplaceText(replace_range, "42;", opts);
+ EXPECT_EQ(
+ Rewrite.getRewrittenText(SourceRange(FileStart.getLocWithOffset(33),
+ FileStart.getLocWithOffset(58))),
+ "{int answer = 42;42; } double f(");
}
-
} // anonymous namespace
Index: clang/lib/Rewrite/Rewriter.cpp
===================================================================
--- clang/lib/Rewrite/Rewriter.cpp
+++ clang/lib/Rewrite/Rewriter.cpp
@@ -327,7 +327,8 @@
return false;
}
-bool Rewriter::ReplaceText(SourceRange range, SourceRange replacementRange, RewriteOptions opts) {
+bool Rewriter::ReplaceText(SourceRange range, SourceRange replacementRange,
+ RewriteOptions opts) {
if (!isRewritable(range.getBegin())) return true;
if (!isRewritable(range.getEnd())) return true;
if (replacementRange.isInvalid()) return true;
Index: clang/include/clang/Rewrite/Core/Rewriter.h
===================================================================
--- clang/include/clang/Rewrite/Core/Rewriter.h
+++ clang/include/clang/Rewrite/Core/Rewriter.h
@@ -164,7 +164,7 @@
/// replace, pass RewriteOptions with IncludeInsertsAtBeginOfRange = false
/// (unless different behavior is desired).
bool ReplaceText(CharSourceRange range, StringRef NewStr,
- RewriteOptions opts = RewriteOptions()) {
+ RewriteOptions opts = RewriteOptions()) {
return ReplaceText(range.getBegin(), getRangeSize(range, opts), NewStr);
}
@@ -174,7 +174,7 @@
/// replace, pass RewriteOptions with IncludeInsertsAtBeginOfRange = false
/// (unless different behavior is desired).
bool ReplaceText(SourceRange range, StringRef NewStr,
- RewriteOptions opts = RewriteOptions()) {
+ RewriteOptions opts = RewriteOptions()) {
return ReplaceText(range.getBegin(), getRangeSize(range, opts), NewStr);
}
@@ -184,7 +184,7 @@
/// pass RewriteOptions with IncludeInsertsAtBeginOfRange = false (unless
/// different behavior is desired).
bool ReplaceText(SourceRange range, SourceRange replacementRange,
- RewriteOptions opts = RewriteOptions());
+ RewriteOptions opts = RewriteOptions());
/// Increase indentation for the lines between the given source range.
/// To determine what the indentation should be, 'parentIndent' is used
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
