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
This commit is contained in:
David Malcolm
David Malcolm
parent
d9056349fc
commit
2ffe0809cb
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
{
|
||||
fixit_remove *remove = static_cast <fixit_remove *> (hint);
|
||||
/* This assumes the removal just affects one line. */
|
||||
source_range src_range = remove->get_range ();
|
||||
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++)
|
||||
{
|
||||
m_colorizer.set_fixit_hint ();
|
||||
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 ();
|
||||
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);
|
||||
|
||||
/* 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 ();
|
||||
for (; column <= finish_column; column++)
|
||||
pp_character (m_pp, '-');
|
||||
m_colorizer.set_normal_text ();
|
||||
}
|
||||
/* 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. */
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -1,3 +1,12 @@
|
|||
2016-08-19 David Malcolm <dmalcolm@redhat.com>
|
||||
|
||||
* 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.
|
||||
|
||||
2016-08-19 Joseph Myers <joseph@codesourcery.com>
|
||||
|
||||
PR c/32187
|
||||
|
|
|
|||
|
|
@ -1422,7 +1422,7 @@ protected:
|
|||
class fixit_hint
|
||||
{
|
||||
public:
|
||||
enum kind {INSERT, REMOVE, REPLACE};
|
||||
enum kind {INSERT, REPLACE};
|
||||
|
||||
virtual ~fixit_hint () {}
|
||||
|
||||
|
|
@ -1453,27 +1453,6 @@ class fixit_insert : public fixit_hint
|
|||
size_t m_len;
|
||||
};
|
||||
|
||||
class fixit_remove : public fixit_hint
|
||||
{
|
||||
public:
|
||||
fixit_remove (source_range src_range);
|
||||
~fixit_remove () {}
|
||||
|
||||
enum kind get_kind () const { return REMOVE; }
|
||||
bool affects_line_p (const char *file, int line);
|
||||
source_location get_start_loc () const { return m_src_range.m_start; }
|
||||
bool maybe_get_end_loc (source_location *out) const
|
||||
{
|
||||
*out = m_src_range.m_finish;
|
||||
return true;
|
||||
}
|
||||
|
||||
source_range get_range () const { return m_src_range; }
|
||||
|
||||
private:
|
||||
source_range m_src_range;
|
||||
};
|
||||
|
||||
class fixit_replace : public fixit_hint
|
||||
{
|
||||
public:
|
||||
|
|
|
|||
|
|
@ -2086,8 +2086,7 @@ rich_location::add_fixit_insert (source_location where,
|
|||
void
|
||||
rich_location::add_fixit_remove (source_range src_range)
|
||||
{
|
||||
linemap_assert (m_num_fixit_hints < MAX_FIXIT_HINTS);
|
||||
m_fixit_hints[m_num_fixit_hints++] = new fixit_remove (src_range);
|
||||
add_fixit_replace (src_range, "");
|
||||
}
|
||||
|
||||
/* Add a fixit-hint, suggesting replacement of the content at
|
||||
|
|
@ -2130,21 +2129,6 @@ fixit_insert::affects_line_p (const char *file, int line)
|
|||
return false;
|
||||
}
|
||||
|
||||
/* class fixit_remove. */
|
||||
|
||||
fixit_remove::fixit_remove (source_range src_range)
|
||||
: m_src_range (src_range)
|
||||
{
|
||||
}
|
||||
|
||||
/* Implementation of fixit_hint::affects_line_p for fixit_remove. */
|
||||
|
||||
bool
|
||||
fixit_remove::affects_line_p (const char *file, int line)
|
||||
{
|
||||
return m_src_range.intersects_line_p (file, line);
|
||||
}
|
||||
|
||||
/* class fixit_replace. */
|
||||
|
||||
fixit_replace::fixit_replace (source_range src_range,
|
||||
|
|
|
|||
Loading…
Reference in New Issue