Re: [diagnostics-branch] use precise locations in C front-end

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Aldy" == Aldy Hernandez <aldyh@redhat.com> writes:

Aldy> The following patch is a top down approach for adding more precise
Aldy> location information to the C front-end.  We start with the parser, and
Aldy> push locations down as we build expressions and declarations.

I gave this a try over the weekend on a project I'm working on.

First, you may want to flip the default to -fshow-column on the
branch.  I think that will show the errors more nicely.

Second, -fshow-column as-is does not interact nicely with Emacs.
According to the GNU standards, GNU programs should count columns
starting at 1 and then:

    Calculate column numbers assuming that space and all ASCII
    printing characters have equal width, and assuming tab stops every
    8 columns.

We currently print logical column numbers, instead.  While I think
this would be a better design, presumably other GNU programs already
follow the above.

Emacs, despite the default setting of compilation-first-column, seems
to actually be using zero-based columns.  If you change cpp_buf_column
to be 1-based, you get the wrong results in Emacs :(

So, I came up with the appended.

I'm not checking this in yet.  Perhaps we should try to change the GNU
standards first, or at least fix the Emacs bug.  Also, we probably
should consider caching the most recent column in the buffer.

FWIW, I am not concerned about this changing gcc's behavior.
-fshow-column is not the default, and while it is the default for
libcpp, libcpp's column numbers are wrong enough, often enough, that
it is hard to imagine anybody relying on them.

I ran into a couple other location oddities using the branch.  I will
try to distill them down a bit before reporting them.  One of them I
suspect is a libcpp problem.

Tom

Index: libcpp/internal.h
===================================================================
--- libcpp/internal.h	(revision 142716)
+++ libcpp/internal.h	(working copy)
@@ -60,9 +60,12 @@
     || (((prevc) == 'p' || (prevc) == 'P') \
         && CPP_OPTION (pfile, extended_numbers))))
 
+extern int cpp_buf_column (const unsigned char *line_base,
+			   const unsigned char *cur);
+
 #define CPP_OPTION(PFILE, OPTION) ((PFILE)->opts.OPTION)
 #define CPP_BUFFER(PFILE) ((PFILE)->buffer)
-#define CPP_BUF_COLUMN(BUF, CUR) ((CUR) - (BUF)->line_base)
+#define CPP_BUF_COLUMN(BUF, CUR) cpp_buf_column ((BUF)->line_base, (CUR))
 #define CPP_BUF_COL(BUF) CPP_BUF_COLUMN(BUF, (BUF)->cur)
 
 #define CPP_INCREMENT_LINE(PFILE, COLS_HINT) do { \
Index: libcpp/lex.c
===================================================================
--- libcpp/lex.c	(revision 142716)
+++ libcpp/lex.c	(working copy)
@@ -1886,3 +1886,18 @@
       return CPP_TOKEN_FLD_NONE;
     }
 }
+
+/* Compute the column number according to GNU rules.  */
+int
+cpp_buf_column (const unsigned char *base, const unsigned char *cur)
+{
+  int col = 0;
+  for (; base < cur; ++base)
+    {
+      if (*base == '\t')
+	col = 8 * (col / 8 + 1);
+      else
+	++col;
+    }
+  return col;
+}