diff options
author | Joel E. Denny <jdenny.ornl@gmail.com> | 2019-08-15 21:17:48 +0000 |
---|---|---|
committer | Joel E. Denny <jdenny.ornl@gmail.com> | 2019-08-15 21:17:48 +0000 |
commit | 9be6d7edb20b87dfa35d21e981591a9a81959344 (patch) | |
tree | f204ab31e62a6e7a66e8f2493d1c8b7c3c2bf313 /clang/lib/Rewrite/Rewriter.cpp | |
parent | be8a2f75657b27ea113517c3279f0489a7f4018c (diff) | |
download | llvm-9be6d7edb20b87dfa35d21e981591a9a81959344.zip llvm-9be6d7edb20b87dfa35d21e981591a9a81959344.tar.gz llvm-9be6d7edb20b87dfa35d21e981591a9a81959344.tar.bz2 |
[Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug
I'd like to add these comments to warn others of problems I
encountered when trying to use `RemoveLineIfEmpty`. I originally
tried to fix the problem, but I realized I could implement the
functionality more easily and efficiently in my calling code where I
can make the simplifying assumption that there are no prior edits to
the line from which text is being removed. While I've lost the
motivation to write a fix, which doesn't look easy, I figure a warning
to others is better than silence.
I've added a unit test to demonstrate the problem. I don't know how
to mark it as an expected failure, so I just marked it disabled.
Reviewed By: jkorous
Differential Revision: https://reviews.llvm.org/D61466
llvm-svn: 369049
Diffstat (limited to 'clang/lib/Rewrite/Rewriter.cpp')
-rw-r--r-- | clang/lib/Rewrite/Rewriter.cpp | 11 |
1 files changed, 11 insertions, 0 deletions
diff --git a/clang/lib/Rewrite/Rewriter.cpp b/clang/lib/Rewrite/Rewriter.cpp index 881399e..33718b7 100644 --- a/clang/lib/Rewrite/Rewriter.cpp +++ b/clang/lib/Rewrite/Rewriter.cpp @@ -96,6 +96,17 @@ void RewriteBuffer::RemoveText(unsigned OrigOffset, unsigned Size, } if (posI != end() && *posI == '\n') { Buffer.erase(curLineStartOffs, lineSize + 1/* + '\n'*/); + // FIXME: Here, the offset of the start of the line is supposed to be + // expressed in terms of the original input not the "real" rewrite + // buffer. How do we compute that reliably? It might be tempting to use + // curLineStartOffs + OrigOffset - RealOffset, but that assumes the + // difference between the original and real offset is the same at the + // removed text and at the start of the line, but that's not true if + // edits were previously made earlier on the line. This bug is also + // documented by a FIXME on the definition of + // clang::Rewriter::RewriteOptions::RemoveLineIfEmpty. A reproducer for + // the implementation below is the test RemoveLineIfEmpty in + // clang/unittests/Rewrite/RewriteBufferTest.cpp. AddReplaceDelta(curLineStartOffs, -(lineSize + 1/* + '\n'*/)); } } |