This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] PR preprocessor/69664: fix rich_location::override_column
- From: David Malcolm <dmalcolm at redhat dot com>
- To: gcc-patches at gcc dot gnu dot org
- Cc: David Malcolm <dmalcolm at redhat dot com>
- Date: Thu, 4 Feb 2016 17:51:47 -0500
- Subject: [PATCH] PR preprocessor/69664: fix rich_location::override_column
- Authentication-results: sourceware.org; auth=none
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(-)
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. */
--
1.8.5.3