<div dir="ltr">On Wed, Jul 31, 2013 at 12:26 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@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 class="gmail_extra"><div class="im"><br><div class="gmail_quote">On Tue, Jul 30, 2013 at 1:48 PM, Vane, Edwin <span dir="ltr"><<a href="mailto:edwin.vane@intel.com" target="_blank">edwin.vane@intel.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>> From: Alex Rosenberg [mailto:<a href="mailto:alexr@leftfield.org" target="_blank">alexr@leftfield.org</a>]<br>


> Sent: <span><span>Tuesday, July 30, 2013 4:31 PM</span></span><br>
> To: Vane, Edwin<br>
> Cc: Tareq A. Siraj; <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
> Subject: Re: [clang-tools-extra] r187428 - cpp11-migrate: Write <span>header</span><br>
> <span>replacements</span> to disk<br>
><br>
</div><div>> YAML was included as a serialization format to be used only for LLVM-internal<br>
> testing tools, not as a supported output format. In fact, we discussed expanding<br>
> the role of YAMLTraits in order to support some of our actually used output<br>
> formats like plists to support generic logic in serialization.<br>
<br>
</div>I'm not sure how the original intention for YAML is relevant to the situation. We're making using of existing functionality in LLVM to simplify a problem.<br>
<div><br>
> I'm pretty strongly against this direction.<br>
<br>
</div>Why exactly?<br>
<div><br>
> I know of exactly zero tools that use YAML as a format for diff/patch/rewriting.<br>
<br>
</div>We don't want to target existing tools. One new tool exists so far: <a href="https://github.com/revane/migmerge_git" target="_blank">https://github.com/revane/migmerge_git</a> which is a prototype for a more powerful tool we'll soon be building. These output files are not meant for consumption by external tools (unless of course somebody wants to build such a tool). They serve only as an intermediate format for data flowing from >>1 C++11 Migrator processes to 1 Migration Post-Processor process via the filesystem. Inter-process communication would be nice but it's not easily portable.<br>


<br>
I've CC'd Manuel who, I think, is familiar with how Google solves this problem internally with protocol buffers. I believe he suggested YAML but we didn't exactly discuss the issue in detail. He might have something to add from his experience.</blockquote>

</div><br></div>So, you didn't CC Manuel, and Manuel wasn't involved in the original discussion.</div></div></blockquote><div><br></div><div>There was some pre-discussion on IRC, and I am actually following those patches on a high level.</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">Most of the problem here is that the original discussion was buried in a patch review thread that didn't really give any indication that there was a significant design decision being made here which would require review from a wider audience. I'm not sure the best way to achieve that, but I have some suggestions:</div>

<div class="gmail_extra"><br></div><div class="gmail_extra">1) Don't start with a patch, start with just a sketch of a design. Doesn't have to be formal, and should be very brief.</div><div class="gmail_extra">2) Send the email to cfe-dev@ and put 'RFC' or something in the title.</div>

<div class="gmail_extra">3) Directly CC people you want design feedback from. I think Manuel is appropriate here.</div><div class="gmail_extra"><br></div><div class="gmail_extra">Anyways, Alex seems to raise an interesting design question. I don't actually have an opinion on it (haven't thought about it much), but I wanted to try to improve the process here as I suspect that lots of others will have thoughts on this subject within the community. A reply to a commit mail is the wrong place to have a detailed discussion about the design. cfe-dev@ is better.</div>
</div></blockquote><div><br></div><div>I think the main reasons why using a structured format instead of patches here are:</div><div>The upside of "patch" formats doesn't apply here: patches are great if you want them to apply on potentially unknown slightly different revisions. Otherwise I don't see big pros for patches.</div>
<div>On the other hand, patches have a major downside here:</div><div>a) they're too coarse - they're usually line based; refactoring tools often need to handle replacements that fix stuff (non-overlapping) on the same line</div>
<div>b) they're hard to "work with" other than applying them; patch libraries usually just give you that, so if you want to extract the exact ranges on what is changing (for example to do deduplication, or to do other location based calculations) you're re-writing your own patch-parsing code</div>
<div>c) they're much harder to generate from a clang-tool; offset+length+replacement is something you just "have", no need to construct the actual end-result and run algorithms over the strings in the tool</div>
<div><br></div><div>To me patches are basically an optional 3rd level interface here, but we need something more fine grained in a structured format. Using YAML (or JSON) seems an obvious choice for that in the llvm context.</div>
<div><br></div><div>Cheers,</div><div>/Manuel</div></div></div></div>