[PATCH] D24717: Merge deletions that are contained in a larger deletion.

Daniel Jasper via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 19 03:10:58 PDT 2016


djasper added a comment.

Thinking about this some more, starting to merge deletions now, but only some of them is a bit suspect. I think we either want to allow even more or continue to be restrictive for now.

I think fundamentally, there are two questions that we need to answer:

1. Is this something that the user/tool author would likely want to do?
2. Is add the replacement order-dependent in any way?

I have no clue about #1, I'd have to see use cases. E.g. what use case are you trying to solve here?

But lets look at #2: I think I have come up with an easy definition of what makes something order-dependent. Lets assume we have two replacements A and B that both refer to the same original code (I am using A and B as single replacements or as sets of a single replacement for simplicity). The question is whether A.add(B) is order-dependent. I think we should define this as (assuming we have a function that shifts a replacement by another replacement like your getReplacementInChangedCode from https://reviews.llvm.org/D24383):

A.add(B) is order-dependent (and thus should conflict, if A.merge(getReplacementInChangedCode(B)) != B.merge(getReplacementInChangedCode(A)).

I think, this enables exactly the kinds of additions that we have so far enabled, which seems good. It also enables overlapping deletions, e.g. deleting range [0-2] and [1-3] will result in deleting [0-3], not matter in which order.


https://reviews.llvm.org/D24717





More information about the cfe-commits mailing list