[PATCH] D24383: Add addOrMerge interface to tooling::Replacements.

[Daniel Jasper via cfe-commits]

djasper added a comment.

Ok.. Thought some more and discussed with Manuel.

I think we should do a partial solution for now. I still think addOrMerge is harmful and it is always never the right thing to use. The codepaths that currently implement some version of it, should likely reconsider.

What I think we should implement is that some combinations of inserts and replacements are accepted by "add". Specifically, imagine you have an original text "aa" and a replacement that replaces it to "bb". Now, imagine you have an additional insert of "c" at offsets 0, 1 or 2:

- Offset 0: Not ambiguous, result should be "cbb".
- Offset 1: Conflict, report error.
- Offset 2: Not ambiguous, result should be "bbc"

So basically, inserts adjacent to replacements (including deletions) have a well defined order and should be supported.

Two replacements still need a defined order and we should probably error for now. I have some ideas on how to solve this. The most intuitive one might be to have a pair functions "StringRef getReplacement(unsigned offset)"/"void setReplacement(unsigned offset, StringRef text)", that control the replacement at a given offset. A replacement object shouldn't have more than one insert at any given offset. But again, I think that's a problem we can get to later. I think most tools that currently would insert stuff at the same location are actually wrong and we should continue to error.

Similarly, we never want to merge overlapping replacements, but we should ensure that directly adjacent ones work, e.g. "aa" can be convert into "bc" with two replacements (one for the first character, one for the second) that are combined with add().

What do you think?


https://reviews.llvm.org/D24383