Re: [committed] diagnostics: add line numbers to source (PR other/84889)

[David Malcolm <dmalcolm at redhat dot com>]

On Thu, 2018-08-09 at 11:45 -0400, David Malcolm wrote:
> On Thu, 2018-08-09 at 15:40 +0000, Joseph Myers wrote:
> > On Thu, 9 Aug 2018, David Malcolm wrote:
> > 
> > > This patch adds a left margin to the lines of source (and
> > > annotations)
> > > printed by diagnostic_show_locus, so that e.g. rather than:
> > 
> > To confirm: if the lines contain tabs anywhere, will the code
> > replace
> > them 
> > by spaces when a margin is added to ensure that all lines really do
> > get 
> > consistently indented by the same amount as part of adding the
> > left 
> > margin?
> 
> I hadn't thought of that; bother.  (that's why we make these kinds of
> changes in stage 1).
> 
> I'm not sure that we already handled that case properly; the previous
> behavior was to print a leading space before the source line.
> 
> I'll investigate.

It turns out that we convert tab characters to *single* space
characters when printing source code.

This behavior has been present since Manu first implemented
-fdiagnostics-show-caret in r186305 (aka
5a9830842f69ebb059061e26f8b0699cbd85121e, PR 24985), where it was this
logic (there in diagnostic.c's diagnostic_show_locus):
      char c = *line == '\t' ? ' ' : *line;
      pp_character (context->printer, c);

(that logic is now in diagnostic-show-locus.c in
layout::print_source_line)

Arguably this is a bug, but it's intimately linked to the way in which
we track "column numbers".  Our "column numbers" are currently simply a
1-based byte-count, I believe, so a tab character is treated by us as
simply an increment of 1 right now.  There are similar issues with
multibyte characters, which are being tracked in PR 49973.

Adding the left margin with line numbers doesn't change this bug, but
makes fixing it slightly more complicated.

I've opened PR other/86904 ("Column numbers ignore tab characters") to
track it.

Dave