On Wed, Jan 18, 2012 at 12:01 PM, Richard Trieu <[email protected]> wrote:
> On Wed, Jan 11, 2012 at 6:22 PM, Richard Trieu <[email protected]> wrote: > >> On Tue, Dec 13, 2011 at 2:32 PM, Chandler Carruth >> <[email protected]>wrote: >> >>> I've left detailed comments on the codereview. Some of them might be >>> addressed by some of my high-level comments: >>> >>> 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. >>> >>> 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. >> >>> >>> 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. >>> >>> 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. >> >> >>> 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. >>> >>> Moved from strings to a steam-based system. >> >>> >>> 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. >>> >> >> 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. >> > > Ping. > Ping.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
