procyberian/gcc
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

diagnostic-show-locus.c: handle fixits on lines outside the regular ranges

Browse source 
The diagnostic_show_locus implementation determines the set
of line spans that need printing based on the ranges within the
rich_location (in layout::calculate_line_spans).

Currently this doesn't take into account fix-it hints, and hence
we fail to print fix-it hints that are on lines outside of
those ranges.

This patch updates the implementation to take fix-it hints into
account when calculating the pertinent line spans, so that such fix-it
hints do get printed.  It also adds some validation, to ensure that
we don't attempt to print fix-its hints affecting a different source
file.

gcc/ChangeLog:
	* diagnostic-show-locus.c (class layout): Add field m_fixit_hints.
	(layout_range::intersects_line_p): New method.
	(test_range_contains_point_for_single_point): Rename to...
	(test_layout_range_for_single_point): ...this, and add testing
	for layout_range::intersects_line_p.
	(test_range_contains_point_for_single_line): Rename to...
	(test_layout_range_for_single_line): ...this,  and add testing
	for layout_range::intersects_line_p.
	(test_range_contains_point_for_multiple_lines): Rename to...
	(test_layout_range_for_multiple_lines): ...this,  and add testing
	for layout_range::intersects_line_p.
	(layout::layout): Populate m_fixit_hints.
	(layout::get_expanded_location): Handle the case of a line-span
	for a fix-it hint.
	(layout::validate_fixit_hint_p): New method.
	(get_line_span_for_fixit_hint): New function.
	(layout::calculate_line_spans): Add spans for fixit-hints.
	(layout::should_print_annotation_line_p): New method.
	(layout::print_any_fixits): Drop param "richloc", instead using
	validated fixits in m_fixit_hints.  Add "const" to hint pointers.
	(diagnostic_show_locus): Avoid printing blank annotation lines.
	(selftest::test_diagnostic_context::test_diagnostic_context):
	Initialize show_column and start_span.
	(selftest::test_diagnostic_context::start_span_cb): New static
	function.
	(selftest::test_diagnostic_show_locus_fixit_lines): New function.
	(selftest::diagnostic_show_locus_c_tests): Update for function
	renamings.  Call test_diagnostic_show_locus_fixit_lines.

libcpp/ChangeLog:
	* include/line-map.h (class fixit_remove): Remove stray decl.
	(fixit_hint::affects_line_p): Make const.
	(fixit_insert::affects_line_p): Likewise.
	(fixit_replace::affects_line_p): Likewise.
	* line-map.c (fixit_insert::affects_line_p): Likewise.
	(fixit_replace::affects_line_p): Likewise.

From-SVN: r239906
This commit is contained in:
David Malcolm 2016-08-31 18:54:55 +00:00 • committed by David Malcolm
parent d12a0b5aab
commit 3d4f9f878d
5 changed files with 315 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
gcc/ChangeLog
View file

@ -1,3 +1,34 @@
2016-08-31 David Malcolm <dmalcolm@redhat.com>
* diagnostic-show-locus.c (class layout): Add field m_fixit_hints.
(layout_range::intersects_line_p): New method.
(test_range_contains_point_for_single_point): Rename to...
(test_layout_range_for_single_point): ...this, and add testing
for layout_range::intersects_line_p.
(test_range_contains_point_for_single_line): Rename to...
(test_layout_range_for_single_line): ...this, and add testing
for layout_range::intersects_line_p.
(test_range_contains_point_for_multiple_lines): Rename to...
(test_layout_range_for_multiple_lines): ...this, and add testing
for layout_range::intersects_line_p.
(layout::layout): Populate m_fixit_hints.
(layout::get_expanded_location): Handle the case of a line-span
for a fix-it hint.
(layout::validate_fixit_hint_p): New method.
(get_line_span_for_fixit_hint): New function.
(layout::calculate_line_spans): Add spans for fixit-hints.
(layout::should_print_annotation_line_p): New method.
(layout::print_any_fixits): Drop param "richloc", instead using
validated fixits in m_fixit_hints. Add "const" to hint pointers.
(diagnostic_show_locus): Avoid printing blank annotation lines.
(selftest::test_diagnostic_context::test_diagnostic_context):
Initialize show_column and start_span.
(selftest::test_diagnostic_context::start_span_cb): New static
function.
(selftest::test_diagnostic_show_locus_fixit_lines): New function.
(selftest::diagnostic_show_locus_c_tests): Update for function
renamings. Call test_diagnostic_show_locus_fixit_lines.
2016-08-31 Marc Glisse <marc.glisse@inria.fr>
PR tree-optimization/73714

292
gcc/diagnostic-show-locus.c
View file

@ -129,6 +129,7 @@ class layout_range
const expanded_location *caret_exploc);
bool contains_point (int row, int column) const;
bool intersects_line_p (int row) const;
layout_point m_start;
layout_point m_finish;
@ -203,14 +204,17 @@ class layout
expanded_location get_expanded_location (const line_span *) const;
bool print_source_line (int row, line_bounds *lbounds_out);
bool should_print_annotation_line_p (int row) const;
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 print_any_fixits (int row);
void show_ruler (int max_column) const;
private:
bool validate_fixit_hint_p (const fixit_hint *hint);
void calculate_line_spans ();
void print_newline ();
@ -237,6 +241,7 @@ class layout
colorizer m_colorizer;
bool m_colorize_source_p;
auto_vec <layout_range> m_layout_ranges;
auto_vec <const fixit_hint *> m_fixit_hints;
auto_vec <line_span> m_line_spans;
int m_x_offset;
};
@ -460,9 +465,22 @@ layout_range::contains_point (int row, int column) const
return column <= m_finish.m_column;
}
/* Does this layout_range contain any part of line ROW? */
bool
layout_range::intersects_line_p (int row) const
{
gcc_assert (m_start.m_line <= m_finish.m_line);
if (row < m_start.m_line)
return false;
if (row > m_finish.m_line)
return false;
return true;
}
#if CHECKING_P
/* A helper function for testing layout_range::contains_point. */
/* A helper function for testing layout_range. */
static layout_range
make_range (int start_line, int start_col, int end_line, int end_col)
@ -475,16 +493,19 @@ make_range (int start_line, int start_col, int end_line, int end_col)
&start_exploc);
}
/* Selftests for layout_range::contains_point. */
/* Selftests for layout_range::contains_point and
layout_range::intersects_line_p. */
/* Selftest for layout_range::contains_point where the layout_range
/* Selftest for layout_range, where the layout_range
is a range with start==end i.e. a single point. */
static void
test_range_contains_point_for_single_point ()
test_layout_range_for_single_point ()
{
layout_range point = make_range (7, 10, 7, 10);
/* Tests for layout_range::contains_point. */
/* Before the line. */
ASSERT_FALSE (point.contains_point (6, 1));
@ -499,16 +520,23 @@ test_range_contains_point_for_single_point ()
/* After the line. */
ASSERT_FALSE (point.contains_point (8, 1));
/* Tests for layout_range::intersects_line_p. */
ASSERT_FALSE (point.intersects_line_p (6));
ASSERT_TRUE (point.intersects_line_p (7));
ASSERT_FALSE (point.intersects_line_p (8));
}
/* Selftest for layout_range::contains_point where the layout_range
/* Selftest for layout_range, where the layout_range
is the single-line range shown as "Example A" above. */
static void
test_range_contains_point_for_single_line ()
test_layout_range_for_single_line ()
{
layout_range example_a = make_range (2, 22, 2, 38);
/* Tests for layout_range::contains_point. */
/* Before the line. */
ASSERT_FALSE (example_a.contains_point (1, 1));
@ -529,16 +557,23 @@ test_range_contains_point_for_single_line ()
/* After the line. */
ASSERT_FALSE (example_a.contains_point (2, 39));
/* Tests for layout_range::intersects_line_p. */
ASSERT_FALSE (example_a.intersects_line_p (1));
ASSERT_TRUE (example_a.intersects_line_p (2));
ASSERT_FALSE (example_a.intersects_line_p (3));
}
/* Selftest for layout_range::contains_point where the layout_range
/* Selftest for layout_range, where the layout_range
is the multi-line range shown as "Example B" above. */
static void
test_range_contains_point_for_multiple_lines ()
test_layout_range_for_multiple_lines ()
{
layout_range example_b = make_range (3, 14, 5, 8);
/* Tests for layout_range::contains_point. */
/* Before first line. */
ASSERT_FALSE (example_b.contains_point (1, 1));
@ -573,6 +608,13 @@ test_range_contains_point_for_multiple_lines ()
/* After the line. */
ASSERT_FALSE (example_b.contains_point (6, 1));
/* Tests for layout_range::intersects_line_p. */
ASSERT_FALSE (example_b.intersects_line_p (2));
ASSERT_TRUE (example_b.intersects_line_p (3));
ASSERT_TRUE (example_b.intersects_line_p (4));
ASSERT_TRUE (example_b.intersects_line_p (5));
ASSERT_FALSE (example_b.intersects_line_p (6));
}
#endif /* #if CHECKING_P */
@ -709,7 +751,7 @@ compatible_locations_p (location_t loc_a, location_t loc_b)
/* Constructor for class layout.
Filter the ranges from the rich_location to those that we can
sanely print, populating m_layout_ranges.
sanely print, populating m_layout_ranges and m_fixit_hints.
Determine the range of lines that we will print, splitting them
up into an ordered list of disjoint spans of contiguous line numbers.
Determine m_x_offset, to ensure that the primary caret
@ -725,6 +767,7 @@ layout::layout (diagnostic_context * context,
m_colorizer (context, diagnostic_kind),
m_colorize_source_p (context->colorize_source_p),
m_layout_ranges (richloc->get_num_locations ()),
m_fixit_hints (richloc->get_num_fixit_hints ()),
m_line_spans (1 + richloc->get_num_locations ()),
m_x_offset (0)
{
@ -798,6 +841,15 @@ layout::layout (diagnostic_context * context,
m_layout_ranges.safe_push (ri);
}
/* Populate m_fixit_hints, filtering to only those that are in the
same file. */
for (unsigned int i = 0; i < richloc->get_num_fixit_hints (); i++)
{
const fixit_hint *hint = richloc->get_fixit_hint (i);
if (validate_fixit_hint_p (hint))
m_fixit_hints.safe_push (hint);
}
/* Populate m_line_spans. */
calculate_line_spans ();
@ -867,12 +919,92 @@ layout::get_expanded_location (const line_span *line_span) const
}
}
/* Otherwise, use the location of the first fixit-hint present within
the line_span. */
for (unsigned int i = 0; i < m_fixit_hints.length (); i++)
{
const fixit_hint *hint = m_fixit_hints[i];
location_t loc = hint->get_start_loc ();
expanded_location exploc = expand_location (loc);
if (line_span->contains_line_p (exploc.line))
return exploc;
}
/* It should not be possible to have a line span that didn't
contain any of the layout_range instances. */
contain any of the layout_range or fixit_hint instances. */
gcc_unreachable ();
return m_exploc;
}
/* Determine if HINT is meaningful to print within this layout. */
bool
layout::validate_fixit_hint_p (const fixit_hint *hint)
{
switch (hint->get_kind ())
{
case fixit_hint::INSERT:
{
const fixit_insert *insert = static_cast <const fixit_insert *> (hint);
location_t loc = insert->get_location ();
if (LOCATION_FILE (loc) != m_exploc.file)
return false;
}
break;
case fixit_hint::REPLACE:
{
const fixit_replace *replace
= static_cast <const fixit_replace *> (hint);
source_range src_range = replace->get_range ();
if (LOCATION_FILE (src_range.m_start) != m_exploc.file)
return false;
if (LOCATION_FILE (src_range.m_finish) != m_exploc.file)
return false;
}
break;
default:
gcc_unreachable ();
}
return true;
}
/* Determine the range of lines affected by HINT.
This assumes that HINT has already been filtered by
validate_fixit_hint_p, and so affects the correct source file. */
static line_span
get_line_span_for_fixit_hint (const fixit_hint *hint)
{
gcc_assert (hint);
switch (hint->get_kind ())
{
case fixit_hint::INSERT:
{
const fixit_insert *insert = static_cast <const fixit_insert *> (hint);
location_t loc = insert->get_location ();
int line = LOCATION_LINE (loc);
return line_span (line, line);
}
break;
case fixit_hint::REPLACE:
{
const fixit_replace *replace
= static_cast <const fixit_replace *> (hint);
source_range src_range = replace->get_range ();
return line_span (LOCATION_LINE (src_range.m_start),
LOCATION_LINE (src_range.m_finish));
}
break;
default:
gcc_unreachable ();
}
}
/* We want to print the pertinent source code at a diagnostic. The
rich_location can contain multiple locations. This will have been
filtered into m_exploc (the caret for the primary location) and
@ -918,6 +1050,14 @@ layout::calculate_line_spans ()
lr->m_finish.m_line));
}
/* Also add spans for any fix-it hints, in case they cover other lines. */
for (unsigned int i = 0; i < m_fixit_hints.length (); i++)
{
const fixit_hint *hint = m_fixit_hints[i];
gcc_assert (hint);
tmp_spans.safe_push (get_line_span_for_fixit_hint (hint));
}
/* Sort them. */
tmp_spans.qsort(line_span::comparator);
@ -1031,6 +1171,20 @@ layout::print_source_line (int row, line_bounds *lbounds_out)
return true;
}
/* Determine if we should print an annotation line for ROW.
i.e. if any of m_layout_ranges contains ROW. */
bool
layout::should_print_annotation_line_p (int row) const
{
layout_range *range;
int i;
FOR_EACH_VEC_ELT (m_layout_ranges, i, range)
if (range->intersects_line_p (row))
return true;
return false;
}
/* Print a line consisting of the caret/underlines for the given
source line. */
@ -1096,17 +1250,17 @@ layout::annotation_line_showed_range_p (int line, int start_column,
return false;
}
/* If there are any fixit hints on source line ROW within RICHLOC, print them.
/* If there are any fixit hints on source line ROW, print them.
They are printed in order, attempting to combine them onto lines, but
starting new lines if necessary. */
void
layout::print_any_fixits (int row, const rich_location *richloc)
layout::print_any_fixits (int row)
{
int column = 0;
for (unsigned int i = 0; i < richloc->get_num_fixit_hints (); i++)
for (unsigned int i = 0; i < m_fixit_hints.length (); i++)
{
fixit_hint *hint = richloc->get_fixit_hint (i);
const fixit_hint *hint = m_fixit_hints[i];
if (hint->affects_line_p (m_exploc.file, row))
{
/* For now we assume each fixit hint can only touch one line. */
@ -1114,7 +1268,8 @@ layout::print_any_fixits (int row, const rich_location *richloc)
{
case fixit_hint::INSERT:
{
fixit_insert *insert = static_cast <fixit_insert *> (hint);
const fixit_insert *insert
= static_cast <const fixit_insert *> (hint);
/* This assumes the insertion just affects one line. */
int start_column
= LOCATION_COLUMN (insert->get_location ());
@ -1128,7 +1283,8 @@ layout::print_any_fixits (int row, const rich_location *richloc)
case fixit_hint::REPLACE:
{
fixit_replace *replace = static_cast <fixit_replace *> (hint);
const fixit_replace *replace
= static_cast <const 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);
@ -1370,8 +1526,9 @@ diagnostic_show_locus (diagnostic_context * context,
line_bounds lbounds;
if (layout.print_source_line (row, &lbounds))
{
layout.print_annotation_line (row, lbounds);
layout.print_any_fixits (row, richloc);
if (layout.should_print_annotation_line_p (row))
layout.print_annotation_line (row, lbounds);
layout.print_any_fixits (row);
}
}
}
@ -1395,11 +1552,22 @@ class test_diagnostic_context : public diagnostic_context
{
diagnostic_initialize (this, 0);
show_caret = true;
show_column = true;
start_span = start_span_cb;
}
~test_diagnostic_context ()
{
diagnostic_finish (this);
}
/* Implementation of diagnostic_start_span_fn, hiding the
real filename (to avoid printing the names of tempfiles). */
static void
start_span_cb (diagnostic_context *context, expanded_location exploc)
{
exploc.file = "FILENAME";
default_diagnostic_start_span_fn (context, exploc);
}
};
/* Verify that diagnostic_show_locus works sanely on UNKNOWN_LOCATION. */
@ -1739,6 +1907,85 @@ test_diagnostic_show_locus_one_liner (const line_table_case &case_)
test_one_liner_many_fixits ();
}
/* Verify that we print fixits even if they only affect lines
outside those covered by the ranges in the rich_location. */
static void
test_diagnostic_show_locus_fixit_lines (const line_table_case &case_)
{
/* Create a tempfile and write some text to it.
...000000000111111111122222222223333333333.
...123456789012345678901234567890123456789. */
const char *content
= ("struct point { double x; double y; };\n" /* line 1. */
"struct point origin = {x: 0.0,\n" /* line 2. */
" y\n" /* line 3. */
"\n" /* line 4. */
"\n" /* line 5. */
" : 0.0};\n"); /* line 6. */
temp_source_file tmp (SELFTEST_LOCATION, ".c", content);
line_table_test ltt (case_);
const line_map_ordinary *ord_map
= linemap_check_ordinary (linemap_add (line_table, LC_ENTER, false,
tmp.get_filename (), 0));
linemap_line_start (line_table, 1, 100);
const location_t final_line_end
= linemap_position_for_line_and_column (line_table, ord_map, 6, 36);
/* Don't attempt to run the tests if column data might be unavailable. */
if (final_line_end > LINE_MAP_MAX_LOCATION_WITH_COLS)
return;
/* A pair of tests for modernizing the initializers to C99-style. */
/* The one-liner case (line 2). */
{
test_diagnostic_context dc;
const location_t x
= linemap_position_for_line_and_column (line_table, ord_map, 2, 24);
const location_t colon
= linemap_position_for_line_and_column (line_table, ord_map, 2, 25);
rich_location richloc (line_table, colon);
richloc.add_fixit_insert (x, ".");
richloc.add_fixit_replace (colon, "=");
diagnostic_show_locus (&dc, &richloc, DK_ERROR);
ASSERT_STREQ ("\n"
" struct point origin = {x: 0.0,\n"
" ^\n"
" .=\n",
pp_formatted_text (dc.printer));
}
/* The multiline case. The caret for the rich_location is on line 6;
verify that insertion fixit on line 3 is still printed (and that
span starts are printed due to the gap between the span at line 3
and that at line 6). */
{
test_diagnostic_context dc;
const location_t y
= linemap_position_for_line_and_column (line_table, ord_map, 3, 24);
const location_t colon
= linemap_position_for_line_and_column (line_table, ord_map, 6, 25);
rich_location richloc (line_table, colon);
richloc.add_fixit_insert (y, ".");
richloc.add_fixit_replace (colon, "=");
diagnostic_show_locus (&dc, &richloc, DK_ERROR);
ASSERT_STREQ ("\n"
"FILENAME:3:24:\n"
" y\n"
" .\n"
"FILENAME:6:25:\n"
" : 0.0};\n"
" ^\n"
" =\n",
pp_formatted_text (dc.printer));
}
}
/* Verify that fix-it hints are appropriately consolidated.
If any fix-it hints in a rich_location involve locations beyond
@ -1898,15 +2145,16 @@ test_fixit_consolidation (const line_table_case &case_)
void
diagnostic_show_locus_c_tests ()
{
test_range_contains_point_for_single_point ();
test_range_contains_point_for_single_line ();
test_range_contains_point_for_multiple_lines ();
test_layout_range_for_single_point ();
test_layout_range_for_single_line ();
test_layout_range_for_multiple_lines ();
test_get_line_width_without_trailing_whitespace ();
test_diagnostic_show_locus_unknown_location ();
for_each_line_table_case (test_diagnostic_show_locus_one_liner);
for_each_line_table_case (test_diagnostic_show_locus_fixit_lines);
for_each_line_table_case (test_fixit_consolidation);
}

9
libcpp/ChangeLog
View file

@ -1,3 +1,12 @@
2016-08-31 David Malcolm <dmalcolm@redhat.com>
* include/line-map.h (class fixit_remove): Remove stray decl.
(fixit_hint::affects_line_p): Make const.
(fixit_insert::affects_line_p): Likewise.
(fixit_replace::affects_line_p): Likewise.
* line-map.c (fixit_insert::affects_line_p): Likewise.
(fixit_replace::affects_line_p): Likewise.
2016-08-30 David Malcolm <dmalcolm@redhat.com>
* include/line-map.h (class semi_embedded_vec): New class.

7
libcpp/include/line-map.h
View file

@ -1412,7 +1412,6 @@ semi_embedded_vec<T, NUM_EMBEDDED>::truncate (int len)
class fixit_hint;
class fixit_insert;
class fixit_remove;
class fixit_replace;
/* A "rich" source code location, for use when printing diagnostics.
@ -1599,7 +1598,7 @@ public:
virtual ~fixit_hint () {}
virtual enum kind get_kind () const = 0;
virtual bool affects_line_p (const char *file, int line) = 0;
virtual bool affects_line_p (const char *file, int line) const = 0;
virtual source_location get_start_loc () const = 0;
virtual bool maybe_get_end_loc (source_location *out) const = 0;
/* Vfunc for consolidating successor fixits. */
@ -1615,7 +1614,7 @@ class fixit_insert : public fixit_hint
const char *new_content);
~fixit_insert ();
enum kind get_kind () const { return INSERT; }
bool affects_line_p (const char *file, int line);
bool affects_line_p (const char *file, int line) const;
source_location get_start_loc () const { return m_where; }
bool maybe_get_end_loc (source_location *) const { return false; }
bool maybe_append_replace (line_maps *set,
@ -1640,7 +1639,7 @@ class fixit_replace : public fixit_hint
~fixit_replace ();
enum kind get_kind () const { return REPLACE; }
bool affects_line_p (const char *file, int line);
bool affects_line_p (const char *file, int line) const;
source_location get_start_loc () const { return m_src_range.m_start; }
bool maybe_get_end_loc (source_location *out) const
{

4
libcpp/line-map.c
View file

@ -2314,7 +2314,7 @@ fixit_insert::~fixit_insert ()
/* Implementation of fixit_hint::affects_line_p for fixit_insert. */
bool
fixit_insert::affects_line_p (const char *file, int line)
fixit_insert::affects_line_p (const char *file, int line) const
{
expanded_location exploc
= linemap_client_expand_location_to_spelling_point (m_where);
@ -2351,7 +2351,7 @@ fixit_replace::~fixit_replace ()
/* Implementation of fixit_hint::affects_line_p for fixit_replace. */
bool
fixit_replace::affects_line_p (const char *file, int line)
fixit_replace::affects_line_p (const char *file, int line) const
{
return m_src_range.intersects_line_p (file, line);
}