[cfe-commits] Refactoring Tool patch submission

[Thomas Minor]

This certainly isn't the final solution. However, I think it's better than
applying them in alphabetical order. In this case, restricting them to
emission order is something that gives you more control.

The main problem seems to be with the concept of a SourceLocation, when
interacting with the rewriter--on a deletion, a whole range of
SourceLocations get mapped to a physical location, and on insertion, a
single SourceLocation gets mapped to a whole range of physical locations.

They should be invalidated whenever any source that included them or was
adjacent to them was rewritten. If the rewriter returned new virtual
locations corresponding to its insertions (invalidate 1, generate 2),
replacements (invalidate many, generate 2), and deletions (invalidate many,
generate 1), it would be much easier to specify the source ranges of
interest, and would get rid of the need to sequence edits at all, as well
as the whole InsertText/InsertTextAfter/InsertTextBefore ordeal.

A drawback of this method is that a tool that uses the Rewriter would have
to go and patch the AST itself, to update source locations of
replaced/inserted nodes and their neighbors. There doesn't seem to be an
automated way of doing this.


For non-overlapping edits, reverse source order isn't even necessary--as it
is, the Rewriter takes care of updating source locations.

Cheers,
-Thomas

On Sun, Jun 10, 2012 at 3:08 AM, Manuel Klimek <klimek at google.com> wrote:

> On Sun, Jun 10, 2012 at 12:52 AM, Thomas Minor <sxeraverx at gmail.com>wrote:
>
>> Hello,
>>
>> I would like to submit a patch to the Refactoring Tool. Currently, the
>> tool relies on std::set for uniqueing edits. However, this poses problems
>> for insertions (in the form of replacements of a zero-length range): they
>> can be reordered arbitrarily. This patch causes them to be applied in the
>> order submitted to the tool.
>>
>
> I'm not sure this is the right solution - I don't want to rely on the tool
> emitting edits in the right order, because I don't see a way this can be
> guaranteed when running over multiple translation units.
>
> For non-overlapping edits, ordering is easy to achieve after-the-fact by
> applying in reverse source order.
> For overlapping edits, the edits would need to contain information on how
> they are sequenced in a TU independent way; I don't think the sequence in
> which they are emitted is a good sequence, as it restricts the tools too
> much.
>
> Thoughts?
> /Manuel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120610/f6164649/attachment.html>