[PATCH] [Rewriter] Fix RemoveText when IncludeInsertsAtBeginOfRange=true

[Justin Bogner via cfe-commits]

Alex Corrado <alexander.corrado at gmail.com> writes:
> Hello,
>
> I’m new to Clang, and I’d like to start off by saying that it's been a
> real treat to work with so far.
>
> I’ve been putting the Rewriter through its paces and found a bug:
> RemoveText does not use the correct starting offset when
> IncludeInsertsAtBeginOfRange=true in the RewriteOptions and some text
> has been inserted at the beginning of the range.
>
> I’ve attached a patch. Apologies if it isn’t in the correct format, as
> this is my first one.

This looks pretty reasonable, thanks for working on it! We like to
accompany changes with tests, so could you please write a test for this?

I'm guessing the test belongs in test/Rewriter. There are examples of
how our tests work there that should get you on the right track, but
feel free to ask questions if you have any trouble.

> Thanks again for all your great work!
>
> -Alex
>
> From 1006ff28881f3df293e1f1b587fda686b624910a Mon Sep 17 00:00:00 2001
> From: Alex Corrado <alexander.corrado at gmail.com>
> Date: Sun, 9 Aug 2015 15:09:15 -0400
> Subject: [PATCH] [Rewriter] Fix RemoveText when
>  IncludeInsertsAtBeginOfRange=true
>
> ---
>  include/clang/Rewrite/Core/RewriteBuffer.h | 3 ++-
>  lib/Rewrite/Rewriter.cpp                   | 7 ++++---
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/include/clang/Rewrite/Core/RewriteBuffer.h b/include/clang/Rewrite/Core/RewriteBuffer.h
> index d69c69b..6259918 100644
> --- a/include/clang/Rewrite/Core/RewriteBuffer.h
> +++ b/include/clang/Rewrite/Core/RewriteBuffer.h
> @@ -56,7 +56,8 @@ public:
>  
>    /// 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/lib/Rewrite/Rewriter.cpp b/lib/Rewrite/Rewriter.cpp
> index be09a36..cc7b24c 100644
> --- a/lib/Rewrite/Rewriter.cpp
> +++ b/lib/Rewrite/Rewriter.cpp
> @@ -49,11 +49,11 @@ static inline bool isWhitespace(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.
> @@ -295,7 +295,8 @@ bool Rewriter::RemoveText(SourceLocation Start, unsigned Length,
>    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,
> +                                !opts.IncludeInsertsAtBeginOfRange);
>    return false;
>  }