There are about 220 or so diagnostics in trunk that use "%q+D" in their format string, which, as well as printing a quoted decl, overwrites any location_t supplied to the diagnostic, instead using the location of the associated decl.
During development of the location range patch kit I adjusted things to use both location&range of the decl for this case, but it looks I broke it at some point; in the version in trunk the code is currently discarding range information, so that just the caret is printed. For example: diagnostic-ranges-1.c:6:7: warning: unused variable 'redundant' [-Wunused-variable] int redundant; ^ The attached patch updates the handling of %q+D, simplifying the implementation, and ensuring that it retains the range information of the decl, giving: diagnostic-ranges-1.c:6:7: warning: unused variable ‘redundant’ [-Wunused-variable] int redundant; ^~~~~~~~~ As well as the above fix, the patch adds test coverage, both - for the specific case above, and - as a unit test for %q+D via one of the existing test plugins Successfully bootstrapped®rtested on x86_64-pc-linux-gnu; adds 5 PASS results to gcc.sum. OK for trunk? gcc/c-family/ChangeLog: * c-common.c (c_cpp_error): Update for change to rich_location::set_range. gcc/fortran/ChangeLog: * error.c (gfc_format_decoder): Update for change of text_info::set_range to text_info::set_location. gcc/ChangeLog: * pretty-print.c (text_info::set_range): Rename to... (text_info::set_location): ...this, converting 2nd param from source_range to a location_t. * pretty-print.h (text_info::set_location): Convert from inline function to external definition. (text_info::set_range): Delete. gcc/testsuite/ChangeLog: * gcc.dg/diagnostic-ranges-1.c: New test file. * gcc.dg/plugin/diagnostic-test-show-locus-bw.c (test_percent_q_plus_d): New test function. * gcc.dg/plugin/diagnostic_plugin_test_show_locus.c (test_show_locus): Rewrite test code using rich_location::set_range. Add code to unit-test the "%q+D" format code. libcpp/ChangeLog: * include/line-map.h (rich_location::set_range): Add line_maps * param; convert param from source_range to source_location. Drop "overwrite_loc_p" param. * line-map.c (rich_location::set_range): Likewise, acting as if "overwrite_loc_p" were true, and getting range from the location. --- gcc/c-family/c-common.c | 4 +--- gcc/fortran/error.c | 11 ++++----- gcc/pretty-print.c | 6 ++--- gcc/pretty-print.h | 9 +------- gcc/testsuite/gcc.dg/diagnostic-ranges-1.c | 11 +++++++++ .../gcc.dg/plugin/diagnostic-test-show-locus-bw.c | 12 ++++++++++ .../plugin/diagnostic_plugin_test_show_locus.c | 27 +++++++++++++++++----- libcpp/include/line-map.h | 4 ++-- libcpp/line-map.c | 14 ++++++----- 9 files changed, 64 insertions(+), 34 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/diagnostic-ranges-1.c diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index a8122b3..59cfc19 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -10129,9 +10129,7 @@ c_cpp_error (cpp_reader *pfile ATTRIBUTE_UNUSED, int level, int reason, gcc_unreachable (); } if (done_lexing) - richloc->set_range (0, - source_range::from_location (input_location), - true, true); + richloc->set_range (line_table, 0, input_location, true); diagnostic_set_info_translated (&diagnostic, msg, ap, richloc, dlevel); diagnostic_override_option_index (&diagnostic, diff --git a/gcc/fortran/error.c b/gcc/fortran/error.c index b4f7020..8f57aff 100644 --- a/gcc/fortran/error.c +++ b/gcc/fortran/error.c @@ -939,12 +939,11 @@ gfc_format_decoder (pretty_printer *pp, /* If location[0] != UNKNOWN_LOCATION means that we already processed one of %C/%L. */ int loc_num = text->get_location (0) == UNKNOWN_LOCATION ? 0 : 1; - source_range range - = source_range::from_location ( - linemap_position_for_loc_and_offset (line_table, - loc->lb->location, - offset)); - text->set_range (loc_num, range, true); + location_t src_loc + = linemap_position_for_loc_and_offset (line_table, + loc->lb->location, + offset); + text->set_location (loc_num, src_loc, true); pp_string (pp, result[loc_num]); return true; } diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c index 4a28d3c..3365074 100644 --- a/gcc/pretty-print.c +++ b/gcc/pretty-print.c @@ -31,14 +31,14 @@ along with GCC; see the file COPYING3. If not see #include <iconv.h> #endif -/* Overwrite the range within this text_info's rich_location. +/* Overwrite the given location/range within this text_info's rich_location. For use e.g. when implementing "+" in client format decoders. */ void -text_info::set_range (unsigned int idx, source_range range, bool caret_p) +text_info::set_location (unsigned int idx, location_t loc, bool show_caret_p) { gcc_checking_assert (m_richloc); - m_richloc->set_range (idx, range, caret_p, true); + m_richloc->set_range (line_table, idx, loc, show_caret_p); } location_t diff --git a/gcc/pretty-print.h b/gcc/pretty-print.h index cdee253..3dc1f6b 100644 --- a/gcc/pretty-print.h +++ b/gcc/pretty-print.h @@ -37,14 +37,7 @@ struct text_info void **x_data; rich_location *m_richloc; - inline void set_location (unsigned int idx, location_t loc, bool caret_p) - { - source_range src_range; - src_range.m_start = loc; - src_range.m_finish = loc; - set_range (idx, src_range, caret_p); - } - void set_range (unsigned int idx, source_range range, bool caret_p); + void set_location (unsigned int idx, location_t loc, bool caret_p); location_t get_location (unsigned int index_of_location) const; }; diff --git a/gcc/testsuite/gcc.dg/diagnostic-ranges-1.c b/gcc/testsuite/gcc.dg/diagnostic-ranges-1.c new file mode 100644 index 0000000..369c0b3 --- /dev/null +++ b/gcc/testsuite/gcc.dg/diagnostic-ranges-1.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-fdiagnostics-show-caret -Wall" } */ + +void test_range_of_unused_variable (void) +{ + int redundant; /* { dg-warning "unused variable" } */ +/* { dg-begin-multiline-output "" } + int redundant; + ^~~~~~~~~ + { dg-end-multiline-output "" } */ +} diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw.c index 44b47e0..8d44078 100644 --- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw.c +++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw.c @@ -190,3 +190,15 @@ void test_fixit_replace (void) { dg-end-multiline-output "" } */ #endif } + +/* Test of "%q+D" format code. */ + +int test_percent_q_plus_d (void) +{ + int local = 0; /* { dg-warning "example of plus in format code" } */ +/* { dg-begin-multiline-output "" } + int local = 0; + ^~~~~ + { dg-end-multiline-output "" } */ + return local; +} diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_show_locus.c b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_show_locus.c index 7ff2cff..02a2aef 100644 --- a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_show_locus.c +++ b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_show_locus.c @@ -224,9 +224,11 @@ test_show_locus (function *fun) source_range src_range; src_range.m_start = get_loc (line, 12); src_range.m_finish = get_loc (line, 20); - rich_location richloc (line_table, caret); - richloc.set_range (0, src_range, true, false); - warning_at_rich_loc (&richloc, 0, "test"); + location_t combined_loc = COMBINE_LOCATION_DATA (line_table, + caret, + src_range, + NULL); + warning_at (combined_loc, 0, "test"); } /* Example of a very wide line, where the information of interest @@ -238,9 +240,11 @@ test_show_locus (function *fun) source_range src_range; src_range.m_start = get_loc (line, 90); src_range.m_finish = get_loc (line, 98); - rich_location richloc (line_table, caret); - richloc.set_range (0, src_range, true, false); - warning_at_rich_loc (&richloc, 0, "test"); + location_t combined_loc = COMBINE_LOCATION_DATA (line_table, + caret, + src_range, + NULL); + warning_at (combined_loc, 0, "test"); } /* Example of multiple carets. */ @@ -313,6 +317,17 @@ test_show_locus (function *fun) global_dc->caret_chars[0] = '^'; global_dc->caret_chars[1] = '^'; } + + /* Example of using the "%q+D" format code, which as well as printing + a quoted decl, overrides the given location to use the location of + the decl. */ + if (0 == strcmp (fnname, "test_percent_q_plus_d")) + { + const int line = fnstart_line + 3; + tree local = (*fun->local_decls)[0]; + warning_at (input_location, 0, + "example of plus in format code for %q+D", local); + } } unsigned int diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h index 4f440fa..73c583e 100644 --- a/libcpp/include/line-map.h +++ b/libcpp/include/line-map.h @@ -1376,8 +1376,8 @@ class rich_location add_range (location_range *src_range); void - set_range (unsigned int idx, source_range src_range, - bool show_caret_p, bool overwrite_loc_p); + set_range (line_maps *set, unsigned int idx, source_location loc, + bool show_caret_p); unsigned int get_num_locations () const { return m_num_ranges; } diff --git a/libcpp/line-map.c b/libcpp/line-map.c index 4284303..333e835 100644 --- a/libcpp/line-map.c +++ b/libcpp/line-map.c @@ -2079,8 +2079,8 @@ rich_location::add_range (location_range *src_range) "%C" and "%L" format codes. */ void -rich_location::set_range (unsigned int idx, source_range src_range, - bool show_caret_p, bool overwrite_loc_p) +rich_location::set_range (line_maps *set, unsigned int idx, + source_location loc, bool show_caret_p) { linemap_assert (idx < MAX_RANGES); @@ -2088,6 +2088,8 @@ rich_location::set_range (unsigned int idx, source_range src_range, on the end of the array. */ linemap_assert (idx <= m_num_ranges); + source_range src_range = get_range_from_loc (set, loc); + location_range *locrange = &m_ranges[idx]; locrange->m_start = linemap_client_expand_location_to_spelling_point (src_range.m_start); @@ -2095,16 +2097,16 @@ rich_location::set_range (unsigned int idx, source_range src_range, = linemap_client_expand_location_to_spelling_point (src_range.m_finish); locrange->m_show_caret_p = show_caret_p; - if (overwrite_loc_p) - locrange->m_caret = locrange->m_start; + locrange->m_caret + = linemap_client_expand_location_to_spelling_point (loc); /* Are we adding a range onto the end? */ if (idx == m_num_ranges) m_num_ranges = idx + 1; - if (idx == 0 && overwrite_loc_p) + if (idx == 0) { - m_loc = src_range.m_start; + m_loc = loc; /* Mark any cached value here as dirty. */ m_have_expanded_location = false; } -- 1.8.5.3