diff options
author | David Malcolm <dmalcolm@redhat.com> | 2016-08-19 21:18:05 +0000 |
---|---|---|
committer | David Malcolm <dmalcolm@gcc.gnu.org> | 2016-08-19 21:18:05 +0000 |
commit | 2ffe0809cb3e9a49387a4657dea65a287b377617 (patch) | |
tree | 783b8fccab5b0d0ff9313af8bdd05fec2a5ccdc7 /gcc | |
parent | d9056349fce2a4b8cd80da5a0657b0e899a5a989 (diff) | |
download | gcc-2ffe0809cb3e9a49387a4657dea65a287b377617.zip gcc-2ffe0809cb3e9a49387a4657dea65a287b377617.tar.gz gcc-2ffe0809cb3e9a49387a4657dea65a287b377617.tar.bz2 |
Reimplement removal fix-it hints in terms of replace
This patch eliminates class fixit_remove, reimplementing
rich_location::add_fixit_remove in terms of replacement with the
empty string. Deleting the removal subclass simplifies
fixit-handling code, as we only have two concrete fixit_hint
subclasses to deal with, rather than three.
The patch also fixes some problems in diagnostic-show-locus.c for
situations where a replacement fix-it has a different range to the
range of the diagnostic, by unifying the drawing of the two kinds of
fixits. For example, this:
foo = bar.field;
^
m_field
becomes:
foo = bar.field;
^
-----
m_field
showing the range to be replaced.
gcc/ChangeLog:
* diagnostic-show-locus.c
(layout::annotation_line_showed_range_p): New method.
(layout::print_any_fixits): Remove case fixit_hint::REMOVE.
Reimplement case fixit_hint::REPLACE to cover removals, and
replacements where the range of the replacement isn't one
of the ranges in the rich_location.
(test_one_liner_fixit_replace): Likewise.
(selftest::test_one_liner_fixit_replace_non_equal_range): New
function.
(selftest::test_one_liner_fixit_replace_equal_secondary_range):
New function.
(selftest::test_diagnostic_show_locus_one_liner): Call the new
functions.
* diagnostic.c (print_parseable_fixits): Remove case
fixit_hint::REMOVE.
libcpp/ChangeLog:
* include/line-map.h (fixit_hint::kind): Delete REPLACE.
(class fixit_remove): Delete.
* line-map.c (rich_location::add_fixit_remove): Reimplement
by calling add_fixit_replace with an empty string.
(fixit_remove::fixit_remove): Delete.
(fixit_remove::affects_line_p): Delete.
From-SVN: r239632
Diffstat (limited to 'gcc')
-rw-r--r-- | gcc/ChangeLog | 18 | ||||
-rw-r--r-- | gcc/diagnostic-show-locus.c | 124 | ||||
-rw-r--r-- | gcc/diagnostic.c | 4 |
3 files changed, 122 insertions, 24 deletions
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 6593300..02c85f8 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,21 @@ +2016-08-19 David Malcolm <dmalcolm@redhat.com> + + * diagnostic-show-locus.c + (layout::annotation_line_showed_range_p): New method. + (layout::print_any_fixits): Remove case fixit_hint::REMOVE. + Reimplement case fixit_hint::REPLACE to cover removals, and + replacements where the range of the replacement isn't one + of the ranges in the rich_location. + (test_one_liner_fixit_replace): Likewise. + (selftest::test_one_liner_fixit_replace_non_equal_range): New + function. + (selftest::test_one_liner_fixit_replace_equal_secondary_range): + New function. + (selftest::test_diagnostic_show_locus_one_liner): Call the new + functions. + * diagnostic.c (print_parseable_fixits): Remove case + fixit_hint::REMOVE. + 2016-08-19 Uros Bizjak <ubizjak@gmail.com> PR target/77270 diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c index 4498f7c..32b1078 100644 --- a/gcc/diagnostic-show-locus.c +++ b/gcc/diagnostic-show-locus.c @@ -199,6 +199,8 @@ class layout bool print_source_line (int row, line_bounds *lbounds_out); void print_annotation_line (int row, const line_bounds lbounds); + bool annotation_line_showed_range_p (int line, int start_column, + int finish_column) const; void print_any_fixits (int row, const rich_location *richloc); void show_ruler (int max_column) const; @@ -1053,6 +1055,26 @@ layout::print_annotation_line (int row, const line_bounds lbounds) print_newline (); } +/* Subroutine of layout::print_any_fixits. + + Determine if the annotation line printed for LINE contained + the exact range from START_COLUMN to FINISH_COLUMN. */ + +bool +layout::annotation_line_showed_range_p (int line, int start_column, + int finish_column) const +{ + layout_range *range; + int i; + FOR_EACH_VEC_ELT (m_layout_ranges, i, range) + if (range->m_start.m_line == line + && range->m_start.m_column == start_column + && range->m_finish.m_line == line + && range->m_finish.m_column == finish_column) + return true; + return false; +} + /* If there are any fixit hints on source line ROW within RICHLOC, print them. They are printed in order, attempting to combine them onto lines, but starting new lines if necessary. */ @@ -1083,33 +1105,39 @@ layout::print_any_fixits (int row, const rich_location *richloc) } break; - case fixit_hint::REMOVE: + case fixit_hint::REPLACE: { - fixit_remove *remove = static_cast <fixit_remove *> (hint); - /* This assumes the removal just affects one line. */ - source_range src_range = remove->get_range (); + fixit_replace *replace = static_cast <fixit_replace *> (hint); + source_range src_range = replace->get_range (); + int line = LOCATION_LINE (src_range.m_start); int start_column = LOCATION_COLUMN (src_range.m_start); int finish_column = LOCATION_COLUMN (src_range.m_finish); - move_to_column (&column, start_column); - for (int column = start_column; column <= finish_column; column++) + + /* If the range of the replacement wasn't printed in the + annotation line, then print an extra underline to + indicate exactly what is being replaced. + Always show it for removals. */ + if (!annotation_line_showed_range_p (line, start_column, + finish_column) + || replace->get_length () == 0) { + move_to_column (&column, start_column); m_colorizer.set_fixit_hint (); - pp_character (m_pp, '-'); + for (; column <= finish_column; column++) + pp_character (m_pp, '-'); m_colorizer.set_normal_text (); } - } - break; - - case fixit_hint::REPLACE: - { - fixit_replace *replace = static_cast <fixit_replace *> (hint); - int start_column - = LOCATION_COLUMN (replace->get_range ().m_start); - move_to_column (&column, start_column); - m_colorizer.set_fixit_hint (); - pp_string (m_pp, replace->get_string ()); - m_colorizer.set_normal_text (); - column += replace->get_length (); + /* Print the replacement text. REPLACE also covers + removals, so only do this extra work (potentially starting + a new line) if we have actual replacement text. */ + if (replace->get_length () > 0) + { + move_to_column (&column, start_column); + m_colorizer.set_fixit_hint (); + pp_string (m_pp, replace->get_string ()); + m_colorizer.set_normal_text (); + column += replace->get_length (); + } } break; @@ -1502,6 +1530,60 @@ test_one_liner_fixit_replace () pp_formatted_text (dc.printer)); } +/* Replace fix-it hint: replacing "field" with "m_field", + but where the caret was elsewhere. */ + +static void +test_one_liner_fixit_replace_non_equal_range () +{ + test_diagnostic_context dc; + location_t equals = linemap_position_for_column (line_table, 5); + location_t start = linemap_position_for_column (line_table, 11); + location_t finish = linemap_position_for_column (line_table, 15); + rich_location richloc (line_table, equals); + source_range range; + range.m_start = start; + range.m_finish = finish; + richloc.add_fixit_replace (range, "m_field"); + diagnostic_show_locus (&dc, &richloc, DK_ERROR); + /* The replacement range is not indicated in the annotation line, so + it should be indicated via an additional underline. */ + ASSERT_STREQ ("\n" + " foo = bar.field;\n" + " ^\n" + " -----\n" + " m_field\n", + pp_formatted_text (dc.printer)); +} + +/* Replace fix-it hint: replacing "field" with "m_field", + where the caret was elsewhere, but where a secondary range + exactly covers "field". */ + +static void +test_one_liner_fixit_replace_equal_secondary_range () +{ + test_diagnostic_context dc; + location_t equals = linemap_position_for_column (line_table, 5); + location_t start = linemap_position_for_column (line_table, 11); + location_t finish = linemap_position_for_column (line_table, 15); + rich_location richloc (line_table, equals); + location_t field = make_location (start, start, finish); + richloc.add_range (field, false); + source_range range; + range.m_start = start; + range.m_finish = finish; + richloc.add_fixit_replace (range, "m_field"); + diagnostic_show_locus (&dc, &richloc, DK_ERROR); + /* The replacement range is indicated in the annotation line, + so it shouldn't be indicated via an additional underline. */ + ASSERT_STREQ ("\n" + " foo = bar.field;\n" + " ^ ~~~~~\n" + " m_field\n", + pp_formatted_text (dc.printer)); +} + /* Run the various one-liner tests. */ static void @@ -1532,6 +1614,8 @@ test_diagnostic_show_locus_one_liner (const line_table_case &case_) test_one_liner_fixit_insert (); test_one_liner_fixit_remove (); test_one_liner_fixit_replace (); + test_one_liner_fixit_replace_non_equal_range (); + test_one_liner_fixit_replace_equal_secondary_range (); } /* Run all of the selftests within this file. */ diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c index fec48c4..b47da38 100644 --- a/gcc/diagnostic.c +++ b/gcc/diagnostic.c @@ -758,10 +758,6 @@ print_parseable_fixits (pretty_printer *pp, rich_location *richloc) } break; - case fixit_hint::REMOVE: - print_escaped_string (pp, ""); - break; - case fixit_hint::REPLACE: { const fixit_replace *replace |