<br><br><div class="gmail_quote">On Mon, Mar 26, 2012 at 6:07 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I like the look of this with the cyan highlighting. A few comments:<br>
<br>
* I was able to ilicit a crash from this by replacing one of the<br>
non-type parameters on line 41 with a character literal (eg:<br>
s/2/'x'/). I haven't reduced this down to a simple test case yet - but<br>
I'd like to see a simple repro along with the fix if/when you make it<br>
(so it's clear what the problem is/isn't)<br></blockquote><div><br></div><div>I found the source of this problem.  Ints are width 32 and chars are width 8.  And APSInt can only be compared if their widths are the same.  Some bit extending will fix this. </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
* I do wonder whether we could do aligned flat printing out of the<br>
types one on top of the other with the necessary whitespace for<br>
alignment. I don't think this is necessary for checkin & probably<br>
going to be easier to iterate on based on user feedback & examples of<br>
particularly heinous template type mismatches in the wild (rather than<br>
bikeshedding this whole thing).<br>
<br>
* It'd be handy to get some feedback from other Clang engineers about<br>
the mechanism you've used to plumb highlighting information down<br>
through the diagnostics engine. I'm not sure there's anything much<br>
better - but I'd at least like it to be clear what we're doing to<br>
power this feature<br>
<br>
* Do we need both formats? (normal and "show-template-tree") - are<br>
there particular scenarios where you find one works really well over<br>
the other in both directions?<br>
<br>
* Testing output coloring seems valuable, given the regression you saw<br>
in your last iteration of the patch. It's a pity we don't have any<br>
other tests for our console coloring - if people feel this is By<br>
Design & not worth validating, I'll defer to their opinion.<br>
<br>
In any case it looks like this will really add a lot of value to Clang users,<br>
<br>
- David<br>
<div><div class="h5"><br>
On Mon, Mar 26, 2012 at 2:08 PM, Richard Trieu <<a href="mailto:rtrieu@google.com">rtrieu@google.com</a>> wrote:<br>
> Updated patch.  Thanks to David Blaikie who rebased the patch on a more<br>
> recent revision.  Should be easier to work with now.<br>
><br>
> Other small differences.  The special ToggleBold value has been changed from<br>
> an enum value to a const char.  The output stream was converted the enum to<br>
> an int instead of a char.  Also changed the highlighting to have color.<br>
>  Template differences now stand out more.<br>
><br>
</div></div>> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
><br>
</blockquote></div><br>