[clang] [Rewrite] Fix offset computation in RemoveText (PR #73827)
Sebastian Poeplau via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 18 08:06:57 PDT 2024
https://github.com/sebastianpoeplau updated https://github.com/llvm/llvm-project/pull/73827
>From f31443bb4de3cec4e7d98ac3c147f09a7ca297fe Mon Sep 17 00:00:00 2001
From: Matthieu Eyraud <eyraud at adacore.com>
Date: Wed, 16 Aug 2023 13:34:10 +0000
Subject: [PATCH] [Rewrite] Fix offset computation in RemoveText
When removing a source range from the RewriterBuffer, Clang needs to
compute the size of the actual range in the RewriterBuffer (as new
elements may have been inserted at any position of the original range).
Once this is done, it then maps the original source location offset to
real offsets into the RewriteBuffer, supposedly accounting for
modifications of the RewriteBuffer (insertion/deletion of text).
However, it actually does not account for modifications at the beginning
of the range, which is inconsistent behavior as they are taken into
account when computing the size of the range. This commit fixes this
slightly-off behavior.
---
.../clang/Rewrite/Core/RewriteBuffer.h | 2 +-
clang/include/clang/Rewrite/Core/Rewriter.h | 9 ++++--
clang/lib/Rewrite/Rewriter.cpp | 28 +++++++++++++++----
clang/unittests/Rewrite/RewriterTest.cpp | 25 +++++++++++++++++
4 files changed, 54 insertions(+), 10 deletions(-)
diff --git a/clang/include/clang/Rewrite/Core/RewriteBuffer.h b/clang/include/clang/Rewrite/Core/RewriteBuffer.h
index b8f34174b71566..295910aa715fc5 100644
--- a/clang/include/clang/Rewrite/Core/RewriteBuffer.h
+++ b/clang/include/clang/Rewrite/Core/RewriteBuffer.h
@@ -58,7 +58,7 @@ class RewriteBuffer {
/// RemoveText - Remove the specified text.
void RemoveText(unsigned OrigOffset, unsigned Size,
- bool removeLineIfEmpty = false);
+ bool removeLineIfEmpty = false, bool AfterInserts = true);
/// InsertText - Insert some text at the specified point, where the offset in
/// the buffer is specified relative to the original SourceBuffer. The
diff --git a/clang/include/clang/Rewrite/Core/Rewriter.h b/clang/include/clang/Rewrite/Core/Rewriter.h
index c89015e4055820..4cf846d777bd4d 100644
--- a/clang/include/clang/Rewrite/Core/Rewriter.h
+++ b/clang/include/clang/Rewrite/Core/Rewriter.h
@@ -139,17 +139,20 @@ class Rewriter {
/// RemoveText - Remove the specified text region.
bool RemoveText(SourceLocation Start, unsigned Length,
- RewriteOptions opts = RewriteOptions());
+ RewriteOptions opts = RewriteOptions(),
+ bool AfterInserts = true);
/// Remove the specified text region.
bool RemoveText(CharSourceRange range,
RewriteOptions opts = RewriteOptions()) {
- return RemoveText(range.getBegin(), getRangeSize(range, opts), opts);
+ return RemoveText(range.getBegin(), getRangeSize(range, opts), opts,
+ !opts.IncludeInsertsAtBeginOfRange);
}
/// Remove the specified text region.
bool RemoveText(SourceRange range, RewriteOptions opts = RewriteOptions()) {
- return RemoveText(range.getBegin(), getRangeSize(range, opts), opts);
+ return RemoveText(range.getBegin(), getRangeSize(range, opts), opts,
+ !opts.IncludeInsertsAtBeginOfRange);
}
/// ReplaceText - This method replaces a range of characters in the input
diff --git a/clang/lib/Rewrite/Rewriter.cpp b/clang/lib/Rewrite/Rewriter.cpp
index 0e6ae365064463..9f68cf20f49107 100644
--- a/clang/lib/Rewrite/Rewriter.cpp
+++ b/clang/lib/Rewrite/Rewriter.cpp
@@ -55,19 +55,34 @@ static inline bool isWhitespaceExceptNL(unsigned char c) {
}
void RewriteBuffer::RemoveText(unsigned OrigOffset, unsigned Size,
- bool removeLineIfEmpty) {
+ bool removeLineIfEmpty, bool AfterInserts) {
// Nothing to remove, exit early.
if (Size == 0) return;
- unsigned RealOffset = getMappedOffset(OrigOffset, true);
+ unsigned RealOffset = getMappedOffset(OrigOffset, AfterInserts);
assert(RealOffset+Size <= Buffer.size() && "Invalid location");
// Remove the dead characters.
Buffer.erase(RealOffset, Size);
// Add a delta so that future changes are offset correctly.
- AddReplaceDelta(OrigOffset, -Size);
-
+ if (!AfterInserts) {
+ // In this case, we must adjust two deltas:
+ // * The one that corresponds to text inserted at the RealOffset
+ // location. We must thus compute the size of the text inserted there.
+ //
+ // * The one that corresponds to the rest of the range, excluding the
+ // text inserted at the RealOffset location.
+ unsigned OffsetBeforeInserts = RealOffset;
+ unsigned OffsetAfterInserts = getMappedOffset(OrigOffset, true);
+ unsigned InsertedTextSize = OffsetAfterInserts - OffsetBeforeInserts;
+ if (InsertedTextSize != 0)
+ AddInsertDelta(OrigOffset, -InsertedTextSize);
+ if (InsertedTextSize != Size)
+ AddReplaceDelta(OrigOffset, -(Size - InsertedTextSize));
+ } else {
+ AddReplaceDelta(OrigOffset, -Size);
+ }
if (removeLineIfEmpty) {
// Find the line that the remove occurred and if it is completely empty
// remove the line as well.
@@ -303,11 +318,12 @@ bool Rewriter::InsertTextAfterToken(SourceLocation Loc, StringRef Str) {
/// RemoveText - Remove the specified text region.
bool Rewriter::RemoveText(SourceLocation Start, unsigned Length,
- RewriteOptions opts) {
+ RewriteOptions opts, bool AfterInserts) {
if (!isRewritable(Start)) return true;
FileID FID;
unsigned StartOffs = getLocationOffsetAndFileID(Start, FID);
- getEditBuffer(FID).RemoveText(StartOffs, Length, opts.RemoveLineIfEmpty);
+ getEditBuffer(FID).RemoveText(StartOffs, Length, opts.RemoveLineIfEmpty,
+ AfterInserts);
return false;
}
diff --git a/clang/unittests/Rewrite/RewriterTest.cpp b/clang/unittests/Rewrite/RewriterTest.cpp
index ca72dde46fda79..46050eac7ed650 100644
--- a/clang/unittests/Rewrite/RewriterTest.cpp
+++ b/clang/unittests/Rewrite/RewriterTest.cpp
@@ -77,4 +77,29 @@ TEST(Rewriter, ReplaceTextRangeTypes) {
EXPECT_EQ(T.Rewrite.getRewrittenText(T.makeCharRange(42, 47)), "0;");
}
+TEST(Rewriter, RemoveTextRangeTypes) {
+ // Check that the correct text is removed for CharSourceRange and SourceRange.
+ // Ranges remain in terms of the original text, but include any inserted text
+ // within the range.
+ StringRef Code = "int main(int argc, char *argv[]) { return argc; }";
+ // insert "foo" ^
+ // get ^~~~~ = "fooargc;"
+ // remove char range ^~
+ // get ^~~~~ = "gc;"
+ // insert "bar " ^
+ // get ^~~~~ = "bargc;"
+ // remove source range ^~
+ // get ^~~~~ = ";"
+ RangeTypeTest T(Code, 42, 44);
+ SourceLocation InsertSloc = T.makeLoc(42);
+ T.Rewrite.InsertText(InsertSloc, "foo");
+ T.Rewrite.RemoveText(T.CRange);
+ EXPECT_EQ(T.Rewrite.getRewrittenText(T.makeCharRange(42, 47)), "gc;");
+
+ T.Rewrite.InsertText(InsertSloc, "bar ");
+ EXPECT_EQ(T.Rewrite.getRewrittenText(T.makeCharRange(42, 47)), "bar gc;");
+ T.Rewrite.RemoveText(T.SRange);
+ EXPECT_EQ(T.Rewrite.getRewrittenText(T.makeCharRange(42, 47)), ";");
+}
+
} // anonymous namespace
More information about the cfe-commits
mailing list