Skip to content

Commit

Permalink
Prevent fix-it hints from affecting more than one line
Browse files Browse the repository at this point in the history
Attempts to apply a removal or replacement fix-it hint to a source
range that covers multiple lines currently lead to nonsensical
results from the printing code in diagnostic-show-locus.c.

We were already filtering them out in edit-context.c (leading
to -fdiagnostics-generate-patch not generating any output for
the whole TU).

Reject attempts to add such fix-it hints within rich_location,
fixing the diagnostic-show-locus.c issue.

gcc/ChangeLog:
	* diagnostic-show-locus.c
	(selftest::test_fixit_deletion_affecting_newline): New function.
	(selftest::diagnostic_show_locus_c_tests): Call it.

libcpp/ChangeLog:
	* include/line-map.h (class rich_location): Document that attempts
	to delete or replace a range *affecting* multiple lines will fail.
	* line-map.c (rich_location::maybe_add_fixit): Implement this
	restriction.



git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@249403 138bc75d-0d04-0410-961f-82ee72b054a4
  • Loading branch information
dmalcolm committed Jun 20, 2017
1 parent 199666f commit c2403f3
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 2 deletions.
6 changes: 6 additions & 0 deletions gcc/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
2017-06-20 David Malcolm <[email protected]>

* diagnostic-show-locus.c
(selftest::test_fixit_deletion_affecting_newline): New function.
(selftest::diagnostic_show_locus_c_tests): Call it.

2017-06-20 Andreas Schwab <[email protected]>

PR target/80970
Expand Down
48 changes: 48 additions & 0 deletions gcc/diagnostic-show-locus.c
Original file line number Diff line number Diff line change
Expand Up @@ -2793,6 +2793,53 @@ test_fixit_replace_containing_newline (const line_table_case &case_)
pp_formatted_text (dc.printer));
}

/* Fix-it hint, attempting to delete a newline.
This will fail, as we currently only support fix-it hints that
affect one line at a time. */

static void
test_fixit_deletion_affecting_newline (const line_table_case &case_)
{
/* Create a tempfile and write some text to it.
..........................0000000001111.
..........................1234567890123. */
const char *old_content = ("foo = bar (\n"
" );\n");

temp_source_file tmp (SELFTEST_LOCATION, ".c", old_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);

/* Attempt to delete the " (\n...)". */
location_t start
= linemap_position_for_line_and_column (line_table, ord_map, 1, 10);
location_t caret
= linemap_position_for_line_and_column (line_table, ord_map, 1, 11);
location_t finish
= linemap_position_for_line_and_column (line_table, ord_map, 2, 7);
location_t loc = make_location (caret, start, finish);
rich_location richloc (line_table, loc);
richloc. add_fixit_remove ();

/* Fix-it hints that affect more than one line are not yet supported, so
the fix-it should not be displayed. */
ASSERT_TRUE (richloc.seen_impossible_fixit_p ());

if (finish > LINE_MAP_MAX_LOCATION_WITH_COLS)
return;

test_diagnostic_context dc;
diagnostic_show_locus (&dc, &richloc, DK_ERROR);
ASSERT_STREQ ("\n"
" foo = bar (\n"
" ~^\n"
" );\n"
" ~ \n",
pp_formatted_text (dc.printer));
}

/* Run all of the selftests within this file. */

void
Expand All @@ -2813,6 +2860,7 @@ diagnostic_show_locus_c_tests ()
for_each_line_table_case (test_fixit_insert_containing_newline);
for_each_line_table_case (test_fixit_insert_containing_newline_2);
for_each_line_table_case (test_fixit_replace_containing_newline);
for_each_line_table_case (test_fixit_deletion_affecting_newline);
}

} // namespace selftest
Expand Down
7 changes: 7 additions & 0 deletions libcpp/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
2017-06-20 David Malcolm <[email protected]>

* include/line-map.h (class rich_location): Document that attempts
to delete or replace a range *affecting* multiple lines will fail.
* line-map.c (rich_location::maybe_add_fixit): Implement this
restriction.

2017-06-09 David Malcolm <[email protected]>

* include/line-map.h
Expand Down
2 changes: 2 additions & 0 deletions libcpp/include/line-map.h
Original file line number Diff line number Diff line change
Expand Up @@ -1556,6 +1556,8 @@ class fixit_hint;
inserting at the start of a line, and finishing with a newline
(with no interior newline characters). Other attempts to add
fix-it hints containing newline characters will fail.
Similarly, attempts to delete or replace a range *affecting* multiple
lines will fail.
The rich_location API handles these failures gracefully, so that
diagnostics can attempt to add fix-it hints without each needing
Expand Down
21 changes: 19 additions & 2 deletions libcpp/line-map.c
Original file line number Diff line number Diff line change
Expand Up @@ -2327,6 +2327,25 @@ rich_location::maybe_add_fixit (source_location start,
if (reject_impossible_fixit (next_loc))
return;

/* Only allow fix-it hints that affect a single line in one file.
Compare the end-points. */
expanded_location exploc_start
= linemap_client_expand_location_to_spelling_point (start);
expanded_location exploc_next_loc
= linemap_client_expand_location_to_spelling_point (next_loc);
/* They must be within the same file... */
if (exploc_start.file != exploc_next_loc.file)
{
stop_supporting_fixits ();
return;
}
/* ...and on the same line. */
if (exploc_start.line != exploc_next_loc.line)
{
stop_supporting_fixits ();
return;
}

const char *newline = strchr (new_content, '\n');
if (newline)
{
Expand All @@ -2342,8 +2361,6 @@ rich_location::maybe_add_fixit (source_location start,
}

/* The insertion must be at the start of a line. */
expanded_location exploc_start
= linemap_client_expand_location_to_spelling_point (start);
if (exploc_start.column != 1)
{
stop_supporting_fixits ();
Expand Down

0 comments on commit c2403f3

Please sign in to comment.