[cfe-commits] [Patch] Add highlighting and type eliding when two templates are used in a diagnostics. Also, template tree printing.

Richard Trieu rtrieu at google.com
Wed Jan 18 12:01:10 PST 2012


On Wed, Jan 11, 2012 at 6:22 PM, Richard Trieu <rtrieu at google.com> wrote:

> On Tue, Dec 13, 2011 at 2:32 PM, Chandler Carruth <chandlerc at google.com>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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120118/42156bbe/attachment.html>


More information about the cfe-commits mailing list