Possible bug in clang::Rewriter, clang::RewriteBuffer, and/or clang::RewriteRope

(also posted in the LLVM Discord)

Hey all, so I think I’ve found a bug in clang::Rewriter and/or clang::RewriteBuffer. If you try to remove then insert (or replace) text on a line that’s already had text inserted, the buffer overwrites the text that came after the removed text. For example, if I have this code

if (N < 20) {
    printf("small n\n");
    return 0;
  }
  else if (N < 3) {
    printf("oh hi\n");
  }

and then I insert printf("before\n"); before the return and replace return 0; with return 42; , I get this:

if (N < 20) {
    printf("small n\n");
    printf("before\n");return 42;< 3) {
    printf("oh hi\n");
  }

where the } else if (N has been overwritten. I’m assuming this is not intended behavior.

I’ve poked a bit in the Rewriter and RewriteBuffer implementations and I’m guessing it’s a bug in how RewriteRope is implemented where characters are erased but not properly marked as erased. Anyone have any insight on fixing this or how exactly all the Rewriter stuff is implemented so I don’t have to figure it out by staring at the code?

Update: The bug was that defaulting to IncludeInsertsAtBeginOfRange = true for replacing text leads to Rewriter::getRangeSize() getting the wrong size for the range – if there are any inserts before the replace point, the size of those will be included in the size of text to be removed, which is what was causing the “overwritten” text.

I’m fixing the Rewriter::ReplaceText() functions to also take a RewriteOptions argument (similar to RemoveText) so the user can decide if they want that behavior or not. However, it might be more in line with the semantics of ReplaceText() (and the interests of not bamboozling the user with unexpected default behavior) to force IncludeInsertsAtBeginOfRange to false.

When I submit this for review, that’ll be one of the things to discuss.

Further update: this bug only happens when the insert location and the start of the replace range are equal.