[cfe-commits] Request feedback for new diagnostic feature - template type diffing

[Richard Smith]

On Tue, August 30, 2011 05:05, Chandler Carruth wrote:
> On Mon, Aug 29, 2011 at 8:56 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> For the current issue at hand - I'm with Doug on this. I think visual
>> highlighting could add a lot of value over any solution that tries to be
>> terminal coloring agnostic.
>
> Again, I'm not suggesting that color doesn't have value. I'm suggesting that
> we shouldn't encode *information* in color that isn't otherwise present in the
> text. That's very different from the color making the information more
> valuable (often through improved clarity).

The pedant in me wants to point out that we wouldn't be encoding information
in formatting that isn't otherwise present -- the difference between the
template-ids would be reconstructible from the types as written in the
diagnostic (we'd need to unwrap enough of the type in the 'aka' to show this
anyway, in order for the highlighting to be useful).

>> That's not to say that we can't provide some degraded experience to
>> non-colored users, but I think it will be inherently worse than highlighting.
>
> The problem is *how* you provide the degraded experience. Sure, in some
> cases, the clang driver detects whether or not we have a terminal available
> that can render colors. However, there are many cases where *after* that
> detection occurs, the colors are then stripped out by various pieces of
> software. This can happen with screen, ssh, log processing, some defective
> terminals, etc. That means that we don't have the opportunity to generate more
> text at the moment the color gets stripped. The worst case scenario for me are
> problems of colorblindness. ;]

We could use ^/~~~ markers on the line after the diagnostic for indicating
which parts of the type differed (instead of / in addition to the formatting):

type-diff.cc:26:3: error: no matching function for call to 'foo'
  foo(A<B<C<D<int> > >, B<C<const double> > >());;
  ^~~
type-diff.cc:23:6: note: candidate function not viable: no known conversion from
      'A<B<C<D<int> > >, B<C<const double> > >' to 'A<B<C<D<const int> > >,
               ^             ~~~~~~~~~~~~                   ~~~~~
      B<C<D<const double> > > >' for 1st argument;
          ~~~~~~~~~~~~~~~
void foo(A<B<C<D<const int> > >, B<C<D<const double> > > > F) {}
     ^

Whatever we fall back on, I don't think it should be a separate note: this
particular diagnostic is already too noisy, and as can be seen in the diff for
test/Misc/diag-aka-types.cpp, the extra diagnostic will often be essentially
redundant.

As I've mentioned separately to Richard T, the patch will need to be amended
to correctly handle alias templates (where I don't think we can do much better
than to punt and not show the differences).

Richard