[cfe-commits] [Patch] Add highlighting and type eliding when two templates are used in a diagnostics. Also, template tree printing.
Chandler Carruth
chandlerc at google.com
Tue Dec 13 14:32:27 PST 2011
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.
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.
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.
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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111213/7dc4bfa0/attachment.html>
More information about the cfe-commits
mailing list