<div class="gmail_quote">On Sun, Jun 10, 2012 at 5:47 PM, Thomas Minor <span dir="ltr"><<a href="mailto:sxeraverx@gmail.com" target="_blank">sxeraverx@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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.<br></blockquote>
<div><br></div><div>My problem with this is that it gives you unwanted coupling. Once users of the library rely on the order it's something that needs to be supported.</div><div><br></div><div>My main concern is still that I don't see how what you propose this should work with multiple TUs, where for example a header file is parsed by different TUs.</div>
<div><br></div><div>Cheers,</div><div>/Manuel</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>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.<br>

<br>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.<br>

<br>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.<br>

<br><br>For non-overlapping edits, reverse source order isn't even necessary--as it is, the Rewriter takes care of updating source locations.<br><br>Cheers,<br>-Thomas<div class="HOEnZb"><div class="h5"><br><br><div class="gmail_quote">
On Sun, Jun 10, 2012 at 3:08 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><div>On Sun, Jun 10, 2012 at 12:52 AM, Thomas Minor <span dir="ltr"><<a href="mailto:sxeraverx@gmail.com" target="_blank">sxeraverx@gmail.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hello,<br><br>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.<br>


</blockquote><div><br></div></div><div>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.</div>


<div><br></div><div>For non-overlapping edits, ordering is easy to achieve after-the-fact by applying in reverse source order.</div><div>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.</div>


<div><br></div><div>Thoughts?</div><span><font color="#888888"><div>/Manuel</div><div><br></div></font></span></div>
</blockquote></div><br>
</div></div></blockquote></div><br>