<div dir="ltr">On Wed, Jul 31, 2013 at 11:03 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank" class="cremed">klimek@google.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div class="h5">On Wed, Jul 31, 2013 at 7:59 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank" class="cremed">chandlerc@google.com</a>></span> wrote:<br>
</div></div><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div><br><div class="gmail_quote">On Wed, Jul 31, 2013 at 8:49 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank" class="cremed">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>As noted, I think the design generally makes sense - my main concern is that the tool that applies the replacements should not be cpp11-migrator-specific. This would be useful for basically *all* refactorings.</div>



<div><br></div><div>I think in the end we'll want 2 things:</div><div>- some functions in lib/Tooling/ that allow outputting those replacement collections from clang-tools</div><div>- a tool to aggregate, deduplicate and apply all the changes</div>


</blockquote></div><br></div>Just to clarify an important design constraint from my perspective:</div><div class="gmail_extra"><br></div><div class="gmail_extra">It shouldn't be just any tool to aggregate, deduplicate, and apply all the changes. It should specifically be the *same* code path that a tool uses when it runs over all TUs in-process. This is to me *really* important to ensure we get consistent behavior across these two possible workflows:</div>


<div class="gmail_extra"><br></div><div class="gmail_extra">1) Run tool X over code in a.cc, b.cc, and c.cc in a single invocation. Internally hands rewrites to a library in Clang that de-dups and applies the edits.</div>


<div class="gmail_extra"><br></div><div class="gmail_extra">2) Run tool X over code in a.cc, b.cc, and c.cc; one invocation pre file. Each run writes out its edits in some form. Run tool Y which reads these edits and hands them to a library in Clang that de-dups and applies the edits.</div>


<div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra">So, I essentially think that it *can't* be a separate system from Clang itself, it is intrinsically tied or it will yield different behavior (and endless confusion).</div>

</div></blockquote><div><br></div></div></div><div>I agree in principle, but I think the common core should live in lib/Tooling (and already does), and it's really really small (as it simply deduplicates the edits, perhaps reports conflicts, and then just uses the Rewriter to apply the changes - those are around 10 LOC).</div>

<div><br></div><div>Everything else in the extra tool is about reading the changes from disk,</div></div></div></div></blockquote><div><br></div><div>This makes sense to be in the helper tool...</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> and using multi-threading to apply them in a scalable way.</div></div></div></div></blockquote><div><br></div><div>I think this should be in the core code. If you have 10k files that you want to edit and you can analyze them in-process fast enough (may become realistic w/ modules), we should also apply the edits in a scalable way.</div>
<div><br></div><div>Clearly, this can always be an incremental thing, I'm just trying to clarify the important constraint for me on the end state.</div></div></div></div>