[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 11 18:22:42 PST 2012

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120111/d2c196e5/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: template-diff5.patch
Type: text/x-patch
Size: 67659 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120111/d2c196e5/attachment.bin>

More information about the cfe-commits mailing list