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

David Blaikie dblaikie at gmail.com
Mon Mar 26 18:07:25 PDT 2012


I like the look of this with the cyan highlighting. A few comments:

* I was able to ilicit a crash from this by replacing one of the
non-type parameters on line 41 with a character literal (eg:
s/2/'x'/). I haven't reduced this down to a simple test case yet - but
I'd like to see a simple repro along with the fix if/when you make it
(so it's clear what the problem is/isn't)

* I do wonder whether we could do aligned flat printing out of the
types one on top of the other with the necessary whitespace for
alignment. I don't think this is necessary for checkin & probably
going to be easier to iterate on based on user feedback & examples of
particularly heinous template type mismatches in the wild (rather than
bikeshedding this whole thing).

* It'd be handy to get some feedback from other Clang engineers about
the mechanism you've used to plumb highlighting information down
through the diagnostics engine. I'm not sure there's anything much
better - but I'd at least like it to be clear what we're doing to
power this feature

* Do we need both formats? (normal and "show-template-tree") - are
there particular scenarios where you find one works really well over
the other in both directions?

* Testing output coloring seems valuable, given the regression you saw
in your last iteration of the patch. It's a pity we don't have any
other tests for our console coloring - if people feel this is By
Design & not worth validating, I'll defer to their opinion.

In any case it looks like this will really add a lot of value to Clang users,

- David

On Mon, Mar 26, 2012 at 2:08 PM, Richard Trieu <rtrieu at google.com> wrote:
> Updated patch.  Thanks to David Blaikie who rebased the patch on a more
> recent revision.  Should be easier to work with now.
>
> Other small differences.  The special ToggleBold value has been changed from
> an enum value to a const char.  The output stream was converted the enum to
> an int instead of a char.  Also changed the highlighting to have color.
>  Template differences now stand out more.
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>




More information about the cfe-commits mailing list