<div class="gmail_quote">On Mon, Jan 30, 2012 at 10:41 AM, Richard Trieu <span dir="ltr"><<a href="mailto:rtrieu@google.com">rtrieu@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="HOEnZb"><div class="h5"><div class="gmail_quote">On Wed, Jan 18, 2012 at 12:01 PM, Richard Trieu <span dir="ltr"><<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@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><div><div class="gmail_quote">On Wed, Jan 11, 2012 at 6:22 PM, Richard Trieu <span dir="ltr"><<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@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 Tue, Dec 13, 2011 at 2:32 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I've left detailed comments on the codereview. Some of them might be addressed by some of my high-level comments:<div><br></div><div>How the various pieces of this interact are not clear from an initial reading. I think we need lots of comments to clearly document how the QualType's move into the ASTDiagnostics layer, and what the expect result is when we are rendering strings in that layer, and how those results are re-composed into the final diagnostic. Without that context at each layer, it's hard to understand how this works.</div>
<div><br></div></blockquote></div><div>I lot of that has been cleaned up. Comments have been added to document how the types are loaded. Magic values have been moved to shared headers. </div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div></div><div><br></div><div>The implementation of the new text is also quite hard to understand for me. I have a theory as to why: There are really two orthogonal things going on: one is computing the different (pruned) tree to print, and the second is rendering the various parts of that tree to text. I think it would help te separate these two as much as possible. What I'm envisioning is first building an object which represents (in some tree-like data-structure with a reasonable set of APIs) the "interesting" pieces of the type(s). Then, methods on the object which (recursively) build a textual representation out of it. Among other readability advantages this would especially help by potentially factoring the two different styles of formatting (tree vs. elision) more firmly -- they could be a largely distinct collection of methods.</div>
<div><br></div></blockquote></div><div>The diffing has been separated into two phases. The first phase walks the type and builds up a diff tree. The second phase takes this diff tree and converts it into the output string.</div>
<div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div></div><div>As part of this (and as I mention in my detailed comments) it would be good to move to a stream-based rendering system to make the composition of the text easier to read as well as much more efficient.</div>
<div><br></div></blockquote></div><div>Moved from strings to a steam-based system.</div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div></div><div>
<br></div><div>Finally, I think you'll need Doug to look at the actual logic of computing the "interesting" set of nodes. I think that's going to be one of the more tricky parts of this to get right.</div>
</blockquote></div></div><br><div>Major changes from the last patch is the separation of the building of the diff information tree and the eventual construction of the output string. Elision is handled a bit more consistently. Also fixed a place where the two types get switched. </div>
</blockquote></div><br></div></div><div>Ping.</div>
</blockquote></div></div></div>Ping.
</blockquote></div>Ping.