Re: [PATCH] PR preprocessor/69664: fix rich_location::override_column

[Jeff Law]

On 02/04/2016 03:51 PM, David Malcolm wrote:

In gcc 5 and earlier, struct diagnostic_info had a field:
   unsigned int override_column;
which could be set by the macro:
   diagnostic_override_column

This was only used by the frontends' callbacks for handling errors
from libcpp: c_cpp_error for the c-family, and cb_cpp_error for Fortran:
   if (column_override)
     diagnostic_override_column (&diagnostic, column_override);
based on a column_override value passed by libcpp to the callback.

I removed the field when introducing rich_location (in r229884),
instead adding a rich_location::override_column method, called from
libcpp immediately before calling the frontend's error-handling
callback (the callback takes a rich_location *).

I got the implementation of rich_location::override_column wrong
(sorry), and PR preprocessor/69664 is a symptom of this.

Specifically, I was only overriding the column within
m_expanded_location, which affects some parts of
diagnostic-show-locus.c, but I was not overriding the column within
the various expanded_location instances within the rich_location's
m_ranges[0].

Hence the wrong column information is printed for diagnostics
emitted by libcpp where the column is overridden.
This happens for any of the "_with_line" diagnostics calls in
libcpp that are passed a non-zero column override.
I believe these are all confined to libcpp/lex.c.

The attached patch fixes this, by ensuring that all relevant
columns are updated by rich_location::override_column, and
also adding the missing conditional to only call it for non-zero
column_override values (this time in libcpp before calling the
error-handling callback, rather than from within the error-handling
callbacks).

It also adds expected column numbers to some pre-existing tests,
giving us test coverage for this.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu;
OK for trunk?

gcc/testsuite/ChangeLog:
        PR preprocessor/69664
        * gcc.dg/cpp/trad/comment-2.c: Add expected column number.
        * gcc.dg/cpp/warn-comments.c: Likewise.

libcpp/ChangeLog:
        PR preprocessor/69664
        * errors.c (cpp_diagnostic_with_line): Only call
        rich_location::override_column if the column is non-zero.
        * line-map.c (rich_location::override_column): Update columns
        within m_ranges[0].  Add assertions to verify that doing so is
        sane.

OK

Thanks,
Jeff