Prevent fix-it hints from affecting more than one line

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.

From-SVN: r249403

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 7cb99a6..4293a05 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2017-06-20  David Malcolm  <dmalcolm@redhat.com>
+
+       * 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  <schwab@suse.de>
 
        PR target/80970

diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index f410a32..8bf4d9e 100644 (file)
--- 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/ChangeLog b/libcpp/ChangeLog
index 13a33c3..9665f6e 100644 (file)
--- a/libcpp/ChangeLog
+++ b/libcpp/ChangeLog
@@ -1,3 +1,10 @@
+2017-06-20  David Malcolm  <dmalcolm@redhat.com>
+
+       * 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  <dmalcolm@redhat.com>
 
        * include/line-map.h

diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index be3041d..f5c19e3 100644 (file)
--- 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 694137a..7ba003a 100644 (file)
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -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)
     {
@@ -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 ();