This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[committed] Prevent fix-it hints from affecting more than one line
- From: David Malcolm <dmalcolm at redhat dot com>
- To: gcc-patches at gcc dot gnu dot org
- Cc: David Malcolm <dmalcolm at redhat dot com>
- Date: Tue, 20 Jun 2017 07:17:51 -0400
- Subject: [committed] Prevent fix-it hints from affecting more than one line
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=dmalcolm at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 6A37EC049E16
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 6A37EC049E16
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.
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
Committed to trunk as r249403.
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.
---
gcc/diagnostic-show-locus.c | 48 +++++++++++++++++++++++++++++++++++++++++++++
libcpp/include/line-map.h | 2 ++
libcpp/line-map.c | 21 ++++++++++++++++++--
3 files changed, 69 insertions(+), 2 deletions(-)
diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index f410a32..8bf4d9e 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -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
@@ -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
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index be3041d..f5c19e3 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -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
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index 5caaf6b..76f7e7c 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -2326,6 +2326,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)
{
@@ -2341,8 +2360,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 ();
--
1.8.5.3