| Message ID | 1454626307-45040-1-git-send-email-dmalcolm@redhat.com |
|---|---|
| State | New |
| Headers | show |
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®rtested 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
diff --git a/gcc/testsuite/gcc.dg/cpp/trad/comment-2.c b/gcc/testsuite/gcc.dg/cpp/trad/comment-2.c index 8d54e3a..310f569 100644 --- a/gcc/testsuite/gcc.dg/cpp/trad/comment-2.c +++ b/gcc/testsuite/gcc.dg/cpp/trad/comment-2.c @@ -8,4 +8,4 @@ /* - /* { dg-warning "within comment" } */ + /* { dg-warning "2: within comment" } */ diff --git a/gcc/testsuite/gcc.dg/cpp/warn-comments.c b/gcc/testsuite/gcc.dg/cpp/warn-comments.c index 1cdf75c..bbe2821 100644 --- a/gcc/testsuite/gcc.dg/cpp/warn-comments.c +++ b/gcc/testsuite/gcc.dg/cpp/warn-comments.c @@ -1,7 +1,7 @@ // { dg-do preprocess } // { dg-options "-std=gnu99 -fdiagnostics-show-option -Wcomments" } -/* /* */ // { dg-warning "\"\.\*\" within comment .-Wcomment." } +/* /* */ // { dg-warning "4: \"\.\*\" within comment .-Wcomment." } // \ - // { dg-warning "multi-line comment .-Wcomment." "multi-line" { target *-*-* } 6 } + // { dg-warning "1: multi-line comment .-Wcomment." "multi-line" { target *-*-* } 6 } diff --git a/libcpp/errors.c b/libcpp/errors.c index d92b386..9847378 100644 --- a/libcpp/errors.c +++ b/libcpp/errors.c @@ -141,7 +141,8 @@ cpp_diagnostic_with_line (cpp_reader * pfile, int level, int reason, if (!pfile->cb.error) abort (); rich_location richloc (pfile->line_table, src_loc); - richloc.override_column (column); + if (column) + richloc.override_column (column); ret = pfile->cb.error (pfile, level, reason, &richloc, _(msgid), ap); return ret; diff --git a/libcpp/line-map.c b/libcpp/line-map.c index fcf0259..e9175df 100644 --- a/libcpp/line-map.c +++ b/libcpp/line-map.c @@ -2036,13 +2036,22 @@ rich_location::lazily_expand_location () return m_expanded_location; } -/* Set the column of the primary location. */ +/* Set the column of the primary location. This can only be called for + rich_location instances for which the primary location has + caret==start==finish. */ void rich_location::override_column (int column) { lazily_expand_location (); + gcc_assert (m_ranges[0].m_show_caret_p); + gcc_assert (m_ranges[0].m_caret.column == m_expanded_location.column); + gcc_assert (m_ranges[0].m_start.column == m_expanded_location.column); + gcc_assert (m_ranges[0].m_finish.column == m_expanded_location.column); m_expanded_location.column = column; + m_ranges[0].m_caret.column = column; + m_ranges[0].m_start.column = column; + m_ranges[0].m_finish.column = column; } /* Add the given range. */
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®rtested 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. --- gcc/testsuite/gcc.dg/cpp/trad/comment-2.c | 2 +- gcc/testsuite/gcc.dg/cpp/warn-comments.c | 4 ++-- libcpp/errors.c | 3 ++- libcpp/line-map.c | 11 ++++++++++- 4 files changed, 15 insertions(+), 5 deletions(-)