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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to