Support fix-it hints that add new lines
authorDavid Malcolm <dmalcolm@redhat.com>
Tue, 2 May 2017 19:03:56 +0000 (19:03 +0000)
committerDavid Malcolm <dmalcolm@gcc.gnu.org>
Tue, 2 May 2017 19:03:56 +0000 (19:03 +0000)
Previously fix-it hints couldn't contain newlines.  This is
due to the need to print something user-readable for them
within diagnostic-show-locus, and for handling them within
edit-context for printing diffs and regenerating content.

This patch enables limited support for fix-it hints with newlines,
for suggesting adding new lines.
Such a fix-it hint must have exactly one newline character, at the
end of the content.  It must be an insertion at the beginning of
a line (so that e.g. fix-its that split a pre-existing line are
still rejected).

They are printed by diagnostic-show-locus with a '+' in the
left-hand margin, like this:

test.c:42:4: note: suggest adding 'break;' here
+      break;
     case 'b':
     ^~~~~~~~~

and the printer injects "spans" if the insertion location is not
near the primary range of the diagnostic e.g.:

test.c:4:2: note: unrecognized 'putchar'; suggest including '<stdio.h>'
test.c:1:1:
+#include <stdio.h>

test.c:4:2:
  putchar (ch);
  ^~~~~~~

gcc/ChangeLog:
* diagnostic-show-locus.c
(layout::should_print_annotation_line_p): Make private.
(layout::print_annotation_line): Make private.
(layout::annotation_line_showed_range_p): Make private.
(layout::show_ruler): Make private.
(layout::print_source_line): Make private.  Pass in line and
line_width, rather than calling location_get_source_line.  Drop
returned value.
(layout::print_leading_fixits): New method.
(layout::print_any_fixits): Rename to...
(layout::print_trailing_fixits): ...this, and make private.
Don't print newline fixits.
(diagnostic_show_locus): Move logic for printing one row into...
(layout::print_line): ...this new function.  Move the
location_get_source_line call and error-handling from
print_source_line to here.  Call print_leading_fixits, and rename
print_any_fixits to print_trailing_fixits.
(selftest::test_fixit_insert_containing_newline): Update now that
newlines are partially supported.
(selftest::test_fixit_insert_containing_newline_2): New test.
(selftest::test_fixit_replace_containing_newline): Update comments.
(selftest::diagnostic_show_locus_c_tests): Call the new test.
* edit-context.c (class added_line): New class.
(class edited_line): Describe newline handling in comment.
(edited_line::actually_edited_p): New method.
(edited_line::print_content): Delete redundant decl.
(edited_line::m_predecessors): New field.
(edited_file::print_content): Call edited_line::print_content.
(edited_file::print_diff): Update to support newlines.
(edited_file::print_diff_hunk): Likewise.
(edited_file::print_run_of_changed_lines): New function.
(edited_file::print_diff_line): Convert to...
(print_diff_line): ...this.
(edited_file::get_effective_line_count): New function.
(edited_line::edited_line): Initialize new field m_predecessors.
(edited_line::~edited_line): Clean up m_predecessors.
(edited_line::apply_fixit): Handle newlines.
(edited_line::get_effective_line_count): New function.
(edited_line::print_content): New function.
(edited_line::print_diff_lines): New function.
(selftest::test_applying_fixits_insert_containing_newline): New
test.
(selftest::test_applying_fixits_replace_containing_newline): New
test.
(selftest::insert_line): New function.
(selftest::test_applying_fixits_multiple_lines): Add example of
inserting a line.
(selftest::edit_context_c_tests): Call the new tests.

gcc/testsuite/ChangeLog:
* gcc.dg/plugin/diagnostic-test-show-locus-bw.c
(test_fixit_insert_newline): New function.
* gcc.dg/plugin/diagnostic-test-show-locus-color.c
(test_fixit_insert_newline): New function.
* gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c
(test_fixit_insert_newline): New function.
* gcc.dg/plugin/diagnostic-test-show-locus-parseable-fixits.c
(test_fixit_insert_newline): New function.
* gcc.dg/plugin/diagnostic_plugin_test_show_locus.c
(test_show_locus): Handle test_fixit_insert_newline.

libcpp/ChangeLog:
* include/line-map.h (class rich_location): Update description of
newline handling.
(class fixit_hint): Likewise.
(fixit_hint::ends_with_newline_p): New decl.
* line-map.c (rich_location::maybe_add_fixit): Support newlines
in fix-it hints that are insertions of single lines at the start
of a line.  Don't consolidate into such fix-it hints.
(fixit_hint::ends_with_newline_p): New method.

From-SVN: r247522

12 files changed:
gcc/ChangeLog
gcc/diagnostic-show-locus.c
gcc/edit-context.c
gcc/testsuite/ChangeLog
gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw.c
gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-color.c
gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c
gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-parseable-fixits.c
gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_show_locus.c
libcpp/ChangeLog
libcpp/include/line-map.h
libcpp/line-map.c

index f4a64804589e0e3e12d173849d1987cbbc4eaf19..2b6dfb3a45c2ff19946c9da24bfea0d7e3731324 100644 (file)
@@ -1,3 +1,54 @@
+2017-05-02  David Malcolm  <dmalcolm@redhat.com>
+
+       * diagnostic-show-locus.c
+       (layout::should_print_annotation_line_p): Make private.
+       (layout::print_annotation_line): Make private.
+       (layout::annotation_line_showed_range_p): Make private.
+       (layout::show_ruler): Make private.
+       (layout::print_source_line): Make private.  Pass in line and
+       line_width, rather than calling location_get_source_line.  Drop
+       returned value.
+       (layout::print_leading_fixits): New method.
+       (layout::print_any_fixits): Rename to...
+       (layout::print_trailing_fixits): ...this, and make private.
+       Don't print newline fixits.
+       (diagnostic_show_locus): Move logic for printing one row into...
+       (layout::print_line): ...this new function.  Move the
+       location_get_source_line call and error-handling from
+       print_source_line to here.  Call print_leading_fixits, and rename
+       print_any_fixits to print_trailing_fixits.
+       (selftest::test_fixit_insert_containing_newline): Update now that
+       newlines are partially supported.
+       (selftest::test_fixit_insert_containing_newline_2): New test.
+       (selftest::test_fixit_replace_containing_newline): Update comments.
+       (selftest::diagnostic_show_locus_c_tests): Call the new test.
+       * edit-context.c (class added_line): New class.
+       (class edited_line): Describe newline handling in comment.
+       (edited_line::actually_edited_p): New method.
+       (edited_line::print_content): Delete redundant decl.
+       (edited_line::m_predecessors): New field.
+       (edited_file::print_content): Call edited_line::print_content.
+       (edited_file::print_diff): Update to support newlines.
+       (edited_file::print_diff_hunk): Likewise.
+       (edited_file::print_run_of_changed_lines): New function.
+       (edited_file::print_diff_line): Convert to...
+       (print_diff_line): ...this.
+       (edited_file::get_effective_line_count): New function.
+       (edited_line::edited_line): Initialize new field m_predecessors.
+       (edited_line::~edited_line): Clean up m_predecessors.
+       (edited_line::apply_fixit): Handle newlines.
+       (edited_line::get_effective_line_count): New function.
+       (edited_line::print_content): New function.
+       (edited_line::print_diff_lines): New function.
+       (selftest::test_applying_fixits_insert_containing_newline): New
+       test.
+       (selftest::test_applying_fixits_replace_containing_newline): New
+       test.
+       (selftest::insert_line): New function.
+       (selftest::test_applying_fixits_multiple_lines): Add example of
+       inserting a line.
+       (selftest::edit_context_c_tests): Call the new tests.
+
 2017-05-02  Bin Cheng  <bin.cheng@arm.com>
 
        * tree-ssa-loop-ivopts.c (get_scaled_computation_cost_at): Delete
index 3c10b69c2326e51be207ce6940090e4ca1d92450..b91c9d55da305f27d54d1d49b7f2c0c4cc7f47e8 100644 (file)
@@ -203,16 +203,20 @@ class layout
 
   expanded_location get_expanded_location (const line_span *) const;
 
-  bool print_source_line (int row, line_bounds *lbounds_out);
+  void print_line (int row);
+
+ private:
+  void print_leading_fixits (int row);
+  void print_source_line (int row, const char *line, int line_width,
+                         line_bounds *lbounds_out);
   bool should_print_annotation_line_p (int row) const;
   void print_annotation_line (int row, const line_bounds lbounds);
+  void print_trailing_fixits (int row);
+
   bool annotation_line_showed_range_p (int line, int start_column,
                                       int finish_column) const;
-  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 ();
@@ -1055,21 +1059,15 @@ layout::calculate_line_spans ()
     }
 }
 
-/* Attempt to print line ROW of source code, potentially colorized at any
-   ranges.
-   Return true if the line was printed, populating *LBOUNDS_OUT.
-   Return false if the source line could not be read, leaving *LBOUNDS_OUT
-   untouched.  */
+/* Print line ROW of source code, potentially colorized at any ranges, and
+   populate *LBOUNDS_OUT.
+   LINE is the source line (not necessarily 0-terminated) and LINE_WIDTH
+   is its width.  */
 
-bool
-layout::print_source_line (int row, line_bounds *lbounds_out)
+void
+layout::print_source_line (int row, const char *line, int line_width,
+                          line_bounds *lbounds_out)
 {
-  int line_width;
-  const char *line = location_get_source_line (m_exploc.file, row,
-                                              &line_width);
-  if (!line)
-    return false;
-
   m_colorizer.set_normal_text ();
 
   /* We will stop printing the source line at any trailing
@@ -1124,7 +1122,6 @@ layout::print_source_line (int row, line_bounds *lbounds_out)
 
   lbounds_out->m_first_non_ws = first_non_ws;
   lbounds_out->m_last_non_ws = last_non_ws;
-  return true;
 }
 
 /* Determine if we should print an annotation line for ROW.
@@ -1186,7 +1183,46 @@ layout::print_annotation_line (int row, const line_bounds lbounds)
   print_newline ();
 }
 
-/* Subroutine of layout::print_any_fixits.
+/* If there are any fixit hints inserting new lines before source line ROW,
+   print them.
+
+   They are printed on lines of their own, before the source line
+   itself, with a leading '+'.  */
+
+void
+layout::print_leading_fixits (int row)
+{
+  for (unsigned int i = 0; i < m_fixit_hints.length (); i++)
+    {
+      const fixit_hint *hint = m_fixit_hints[i];
+
+      if (!hint->ends_with_newline_p ())
+       /* Not a newline fixit; print it in print_trailing_fixits.  */
+       continue;
+
+      gcc_assert (hint->insertion_p ());
+
+      if (hint->affects_line_p (m_exploc.file, row))
+       {
+         /* Printing the '+' with normal colorization
+            and the inserted line with "insert" colorization
+            helps them stand out from each other, and from
+            the surrounding text.  */
+         m_colorizer.set_normal_text ();
+         pp_character (m_pp, '+');
+         m_colorizer.set_fixit_insert ();
+         /* Print all but the trailing newline of the fix-it hint.
+            We have to print the newline separately to avoid
+            getting additional pp prefixes printed.  */
+         for (size_t i = 0; i < hint->get_length () - 1; i++)
+           pp_character (m_pp, hint->get_string ()[i]);
+         m_colorizer.set_normal_text ();
+         pp_newline (m_pp);
+       }
+    }
+}
+
+/* Subroutine of layout::print_trailing_fixits.
 
    Determine if the annotation line printed for LINE contained
    the exact range from START_COLUMN to FINISH_COLUMN.  */
@@ -1208,15 +1244,21 @@ layout::annotation_line_showed_range_p (int line, int start_column,
 
 /* 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.  */
+   starting new lines if necessary.
+   Fix-it hints that insert new lines are handled separately,
+   in layout::print_leading_fixits.  */
 
 void
-layout::print_any_fixits (int row)
+layout::print_trailing_fixits (int row)
 {
   int column = 0;
   for (unsigned int i = 0; i < m_fixit_hints.length (); i++)
     {
       const fixit_hint *hint = m_fixit_hints[i];
+
+      if (hint->ends_with_newline_p ())
+       continue;
+
       if (hint->affects_line_p (m_exploc.file, row))
        {
          /* For now we assume each fixit hint can only touch one line.  */
@@ -1416,6 +1458,27 @@ layout::show_ruler (int max_column) const
   pp_newline (m_pp);
 }
 
+/* Print leading fix-its (for new lines inserted before the source line)
+   then the source line, followed by an annotation line
+   consisting of any caret/underlines, then any fixits.
+   If the source line can't be read, print nothing.  */
+void
+layout::print_line (int row)
+{
+  int line_width;
+  const char *line = location_get_source_line (m_exploc.file, row,
+                                              &line_width);
+  if (!line)
+    return;
+
+  line_bounds lbounds;
+  print_leading_fixits (row);
+  print_source_line (row, line, line_width, &lbounds);
+  if (should_print_annotation_line_p (row))
+    print_annotation_line (row, lbounds);
+  print_trailing_fixits (row);
+}
+
 } /* End of anonymous namespace.  */
 
 /* Print the physical source code corresponding to the location of
@@ -1460,18 +1523,7 @@ diagnostic_show_locus (diagnostic_context * context,
        }
       int last_line = line_span->get_last_line ();
       for (int row = line_span->get_first_line (); row <= last_line; row++)
-       {
-         /* Print the source line, followed by an annotation line
-            consisting of any caret/underlines, then any fixits.
-            If the source line can't be read, print nothing.  */
-         line_bounds lbounds;
-         if (layout.print_source_line (row, &lbounds))
-           {
-             if (layout.should_print_annotation_line_p (row))
-               layout.print_annotation_line (row, lbounds);
-             layout.print_any_fixits (row);
-           }
-       }
+       layout.print_line (row);
     }
 
   pp_set_prefix (context->printer, saved_prefix);
@@ -2101,8 +2153,7 @@ test_fixit_consolidation (const line_table_case &case_)
   }
 }
 
-/* Insertion fix-it hint: adding a "break;" on a line by itself.
-   This will fail, as newlines aren't yet supported.  */
+/* Insertion fix-it hint: adding a "break;" on a line by itself.  */
 
 static void
 test_fixit_insert_containing_newline (const line_table_case &case_)
@@ -2119,32 +2170,97 @@ test_fixit_insert_containing_newline (const line_table_case &case_)
   line_table_test ltt (case_);
   linemap_add (line_table, LC_ENTER, false, tmp.get_filename (), 3);
 
-  /* Add a "break;" on a line by itself before line 3 i.e. before
-     column 1 of line 3. */
   location_t case_start = linemap_position_for_column (line_table, 5);
   location_t case_finish = linemap_position_for_column (line_table, 13);
   location_t case_loc = make_location (case_start, case_start, case_finish);
-  rich_location richloc (line_table, case_loc);
   location_t line_start = linemap_position_for_column (line_table, 1);
-  richloc.add_fixit_insert_before (line_start, "      break;\n");
-
-  /* Newlines are not yet supported within fix-it hints, so
-     the fix-it should not be displayed.  */
-  ASSERT_TRUE (richloc.seen_impossible_fixit_p ());
 
   if (case_finish > LINE_MAP_MAX_LOCATION_WITH_COLS)
     return;
 
+  /* Add a "break;" on a line by itself before line 3 i.e. before
+     column 1 of line 3. */
+  {
+    rich_location richloc (line_table, case_loc);
+    richloc.add_fixit_insert_before (line_start, "      break;\n");
+    test_diagnostic_context dc;
+    diagnostic_show_locus (&dc, &richloc, DK_ERROR);
+    ASSERT_STREQ ("\n"
+                 "+      break;\n"
+                 "     case 'b':\n"
+                 "     ^~~~~~~~~\n",
+                 pp_formatted_text (dc.printer));
+  }
+
+  /* Verify that attempts to add text with a newline fail when the
+     insertion point is *not* at the start of a line.  */
+  {
+    rich_location richloc (line_table, case_loc);
+    richloc.add_fixit_insert_before (case_start, "break;\n");
+    ASSERT_TRUE (richloc.seen_impossible_fixit_p ());
+    test_diagnostic_context dc;
+    diagnostic_show_locus (&dc, &richloc, DK_ERROR);
+    ASSERT_STREQ ("\n"
+                 "     case 'b':\n"
+                 "     ^~~~~~~~~\n",
+                 pp_formatted_text (dc.printer));
+  }
+}
+
+/* Insertion fix-it hint: adding a "#include <stdio.h>\n" to the top
+   of the file, where the fix-it is printed in a different line-span
+   to the primary range of the diagnostic.  */
+
+static void
+test_fixit_insert_containing_newline_2 (const line_table_case &case_)
+{
+  /* Create a tempfile and write some text to it.
+     .........................0000000001111111.
+     .........................1234567890123456.  */
+  const char *old_content = ("test (int ch)\n"  /* line 1. */
+                            "{\n"              /* line 2. */
+                            " putchar (ch);\n" /* line 3. */
+                            "}\n");            /* line 4. */
+
+  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);
+
+  /* The primary range is the "putchar" token.  */
+  location_t putchar_start
+    = linemap_position_for_line_and_column (line_table, ord_map, 3, 2);
+  location_t putchar_finish
+    = linemap_position_for_line_and_column (line_table, ord_map, 3, 8);
+  location_t putchar_loc
+    = make_location (putchar_start, putchar_start, putchar_finish);
+  rich_location richloc (line_table, putchar_loc);
+
+  /* Add a "#include <stdio.h>" on a line by itself at the top of the file.  */
+  location_t file_start
+     = linemap_position_for_line_and_column (line_table, ord_map,  1, 1);
+  richloc.add_fixit_insert_before (file_start, "#include <stdio.h>\n");
+
+  if (putchar_finish > LINE_MAP_MAX_LOCATION_WITH_COLS)
+    return;
+
   test_diagnostic_context dc;
   diagnostic_show_locus (&dc, &richloc, DK_ERROR);
   ASSERT_STREQ ("\n"
-               "     case 'b':\n"
-               "     ^~~~~~~~~\n",
+               "FILENAME:1:1:\n"
+               "+#include <stdio.h>\n"
+               " test (int ch)\n"
+               "FILENAME:3:2:\n"
+               "  putchar (ch);\n"
+               "  ^~~~~~~\n",
                pp_formatted_text (dc.printer));
 }
 
 /* Replacement fix-it hint containing a newline.
-   This will fail, as newlines aren't yet supported.  */
+   This will fail, as newlines are only supported when inserting at the
+   beginning of a line.  */
 
 static void
 test_fixit_replace_containing_newline (const line_table_case &case_)
@@ -2167,7 +2283,7 @@ test_fixit_replace_containing_newline (const line_table_case &case_)
   source_range range = source_range::from_locations (start, finish);
   richloc.add_fixit_replace (range, "\n =");
 
-  /* Newlines are not yet supported within fix-it hints, so
+  /* Arbitrary newlines are not yet supported within fix-it hints, so
      the fix-it should not be displayed.  */
   ASSERT_TRUE (richloc.seen_impossible_fixit_p ());
 
@@ -2199,6 +2315,7 @@ diagnostic_show_locus_c_tests ()
   for_each_line_table_case (test_diagnostic_show_locus_fixit_lines);
   for_each_line_table_case (test_fixit_consolidation);
   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);
 }
 
index bea8a8af688b6f1d9ef5c404ac9b8b49f7d23252..6f35ca291f1c2fe181dcec64ea172fc11c6c2d52 100644 (file)
@@ -86,23 +86,52 @@ class edited_file
  private:
   bool print_content (pretty_printer *pp);
   void print_diff (pretty_printer *pp, bool show_filenames);
-  void print_diff_hunk (pretty_printer *pp, int start_of_hunk,
-                       int end_of_hunk);
-  void print_diff_line (pretty_printer *pp, char prefix_char,
-                       const char *line, int line_size);
+  int print_diff_hunk (pretty_printer *pp, int old_start_of_hunk,
+                      int old_end_of_hunk, int new_start_of_hunk);
   edited_line *get_line (int line);
   edited_line *get_or_insert_line (int line);
   int get_num_lines (bool *missing_trailing_newline);
 
+  int get_effective_line_count (int old_start_of_hunk,
+                               int old_end_of_hunk);
+
+  void print_run_of_changed_lines (pretty_printer *pp,
+                                  int start_of_run,
+                                  int end_of_run);
+
   const char *m_filename;
   typed_splay_tree<int, edited_line *> m_edited_lines;
   int m_num_lines;
 };
 
+/* A line added before an edited_line.  */
+
+class added_line
+{
+ public:
+  added_line (const char *content, int len)
+  : m_content (xstrndup (content, len)), m_len (len) {}
+  ~added_line () { free (m_content); }
+
+  const char *get_content () const { return m_content; }
+  int get_len () const { return m_len; }
+
+ private:
+  char *m_content;
+  int m_len;
+};
+
 /* The state of one edited line within an edited_file.
    As well as the current content of the line, it contains a record of
    the changes, so that further changes can be applied in the correct
-   place.  */
+   place.
+
+   When handling fix-it hints containing newlines, new lines are added
+   as added_line predecessors to an edited_line.  Hence it's possible
+   for an "edited_line" to not actually have been changed, but to merely
+   be a placeholder for the lines added before it.  This can be tested
+   for with actuall_edited_p, and has a slight effect on how diff hunks
+   are generated.  */
 
 class edited_line
 {
@@ -121,16 +150,25 @@ class edited_line
                    const char *replacement_str,
                    int replacement_len);
 
+  int get_effective_line_count () const;
+
+  /* Has the content of this line actually changed, or are we merely
+     recording predecessor added_lines?  */
+  bool actually_edited_p () const { return m_line_events.length () > 0; }
+
+  void print_content (pretty_printer *pp) const;
+  void print_diff_lines (pretty_printer *pp) const;
+
  private:
   void ensure_capacity (int len);
   void ensure_terminated ();
-  void print_content (pretty_printer *pp) const;
 
   int m_line_num;
   char *m_content;
   int m_len;
   int m_alloc_sz;
   auto_vec <line_event> m_line_events;
+  auto_vec <added_line *> m_predecessors;
 };
 
 /* Class for representing edit events that have occurred on one line of
@@ -160,6 +198,12 @@ class line_event
   int m_delta;
 };
 
+/* Forward decls.  */
+
+static void
+print_diff_line (pretty_printer *pp, char prefix_char,
+                const char *line, int line_size);
+
 /* Implementation of class edit_context.  */
 
 /* edit_context's ctor.  */
@@ -375,7 +419,7 @@ edited_file::print_content (pretty_printer *pp)
     {
       edited_line *el = get_line (line_num);
       if (el)
-       pp_string (pp, el->get_content ());
+       el->print_content (pp);
       else
        {
          int len;
@@ -417,6 +461,10 @@ edited_file::print_diff (pretty_printer *pp, bool show_filenames)
 
   const int context_lines = 3;
 
+  /* Track new line numbers minus old line numbers.  */
+
+  int line_delta = 0;
+
   while (el)
     {
       int start_of_hunk = el->get_line_num ();
@@ -432,39 +480,53 @@ edited_file::print_diff (pretty_printer *pp, bool show_filenames)
            = m_edited_lines.successor (el->get_line_num ());
          if (!next_el)
            break;
-         if (el->get_line_num () + context_lines
+
+         int end_of_printed_hunk = el->get_line_num () + context_lines;
+         if (!el->actually_edited_p ())
+           end_of_printed_hunk--;
+
+         if (end_of_printed_hunk
              >= next_el->get_line_num () - context_lines)
            el = next_el;
          else
            break;
        }
+
       int end_of_hunk = el->get_line_num ();
       end_of_hunk += context_lines;
+      if (!el->actually_edited_p ())
+       end_of_hunk--;
       if (end_of_hunk > line_count)
        end_of_hunk = line_count;
 
-      print_diff_hunk (pp, start_of_hunk, end_of_hunk);
-
+      int new_start_of_hunk = start_of_hunk + line_delta;
+      line_delta += print_diff_hunk (pp, start_of_hunk, end_of_hunk,
+                                    new_start_of_hunk);
       el = m_edited_lines.successor (el->get_line_num ());
     }
 }
 
 /* Print one hunk within a unified diff to PP, covering the
-   given range of lines.  */
+   given range of lines.  OLD_START_OF_HUNK and OLD_END_OF_HUNK are
+   line numbers in the unedited version of the file.
+   NEW_START_OF_HUNK is a line number in the edited version of the file.
+   Return the change in the line count within the hunk.  */
 
-void
-edited_file::print_diff_hunk (pretty_printer *pp, int start_of_hunk,
-                             int end_of_hunk)
+int
+edited_file::print_diff_hunk (pretty_printer *pp, int old_start_of_hunk,
+                             int old_end_of_hunk, int new_start_of_hunk)
 {
-  int num_lines = end_of_hunk - start_of_hunk + 1;
+  int old_num_lines = old_end_of_hunk - old_start_of_hunk + 1;
+  int new_num_lines
+    = get_effective_line_count (old_start_of_hunk, old_end_of_hunk);
 
   pp_string (pp, colorize_start (pp_show_color (pp), "diff-hunk"));
-  pp_printf (pp, "@@ -%i,%i +%i,%i @@\n", start_of_hunk, num_lines,
-            start_of_hunk, num_lines);
+  pp_printf (pp, "@@ -%i,%i +%i,%i @@\n", old_start_of_hunk, old_num_lines,
+            new_start_of_hunk, new_num_lines);
   pp_string (pp, colorize_stop (pp_show_color (pp)));
 
-  int line_num = start_of_hunk;
-  while (line_num <= end_of_hunk)
+  int line_num = old_start_of_hunk;
+  while (line_num <= old_end_of_hunk)
     {
       edited_line *el = get_line (line_num);
       if (el)
@@ -475,34 +537,8 @@ edited_file::print_diff_hunk (pretty_printer *pp, int start_of_hunk,
          while (get_line (line_num))
            line_num++;
          const int last_changed_line_in_run = line_num - 1;
-
-         /* Show old version of lines.  */
-         pp_string (pp, colorize_start (pp_show_color (pp),
-                                        "diff-delete"));
-         for (line_num = first_changed_line_in_run;
-              line_num <= last_changed_line_in_run;
-              line_num++)
-           {
-             int line_len;
-             const char *old_line
-               = location_get_source_line (m_filename, line_num, &line_len);
-             print_diff_line (pp, '-', old_line, line_len);
-           }
-         pp_string (pp, colorize_stop (pp_show_color (pp)));
-
-         /* Show new version of lines.  */
-         pp_string (pp, colorize_start (pp_show_color (pp),
-                                        "diff-insert"));
-         for (line_num = first_changed_line_in_run;
-              line_num <= last_changed_line_in_run;
-              line_num++)
-           {
-             edited_line *el_in_run = get_line (line_num);
-             gcc_assert (el_in_run);
-             print_diff_line (pp, '+', el_in_run->get_content (),
-                              el_in_run->get_len ());
-           }
-         pp_string (pp, colorize_stop (pp_show_color (pp)));
+         print_run_of_changed_lines (pp, first_changed_line_in_run,
+                                     last_changed_line_in_run);
        }
       else
        {
@@ -514,15 +550,59 @@ edited_file::print_diff_hunk (pretty_printer *pp, int start_of_hunk,
          line_num++;
        }
     }
+
+  return new_num_lines - old_num_lines;
+}
+
+/* Subroutine of edited_file::print_diff_hunk: given a run of lines
+   from START_OF_RUN to END_OF_RUN that all have edited_line instances,
+   print the diff to PP.  */
+
+void
+edited_file::print_run_of_changed_lines (pretty_printer *pp,
+                                        int start_of_run,
+                                        int end_of_run)
+{
+  /* Show old version of lines.  */
+  pp_string (pp, colorize_start (pp_show_color (pp),
+                                "diff-delete"));
+  for (int line_num = start_of_run;
+       line_num <= end_of_run;
+       line_num++)
+    {
+      edited_line *el_in_run = get_line (line_num);
+      gcc_assert (el_in_run);
+      if (el_in_run->actually_edited_p ())
+       {
+         int line_len;
+         const char *old_line
+           = location_get_source_line (m_filename, line_num, &line_len);
+         print_diff_line (pp, '-', old_line, line_len);
+       }
+    }
+  pp_string (pp, colorize_stop (pp_show_color (pp)));
+
+  /* Show new version of lines.  */
+  pp_string (pp, colorize_start (pp_show_color (pp),
+                                "diff-insert"));
+  for (int line_num = start_of_run;
+       line_num <= end_of_run;
+       line_num++)
+    {
+      edited_line *el_in_run = get_line (line_num);
+      gcc_assert (el_in_run);
+      el_in_run->print_diff_lines (pp);
+    }
+  pp_string (pp, colorize_stop (pp_show_color (pp)));
 }
 
 /* Print one line within a diff, starting with PREFIX_CHAR,
    followed by the LINE of content, of length LEN.  LINE is
    not necessarily 0-terminated.  Print a trailing newline.  */
 
-void
-edited_file::print_diff_line (pretty_printer *pp, char prefix_char,
-                             const char *line, int len)
+static void
+print_diff_line (pretty_printer *pp, char prefix_char,
+                const char *line, int len)
 {
   pp_character (pp, prefix_char);
   for (int i = 0; i < len; i++)
@@ -530,6 +610,27 @@ edited_file::print_diff_line (pretty_printer *pp, char prefix_char,
   pp_character (pp, '\n');
 }
 
+/* Determine the number of lines that will be present after
+   editing for the range of lines from OLD_START_OF_HUNK to
+   OLD_END_OF_HUNK inclusive.  */
+
+int
+edited_file::get_effective_line_count (int old_start_of_hunk,
+                                      int old_end_of_hunk)
+{
+  int line_count = 0;
+  for (int old_line_num = old_start_of_hunk; old_line_num <= old_end_of_hunk;
+       old_line_num++)
+    {
+      edited_line *el = get_line (old_line_num);
+      if (el)
+       line_count += el->get_effective_line_count ();
+      else
+       line_count++;
+    }
+  return line_count;
+}
+
 /* Get the state of LINE within the file, or NULL if it is untouched.  */
 
 edited_line *
@@ -591,7 +692,8 @@ edited_file::get_num_lines (bool *missing_trailing_newline)
 edited_line::edited_line (const char *filename, int line_num)
 : m_line_num (line_num),
   m_content (NULL), m_len (0), m_alloc_sz (0),
-  m_line_events ()
+  m_line_events (),
+  m_predecessors ()
 {
   const char *line = location_get_source_line (filename, line_num,
                                               &m_len);
@@ -606,7 +708,12 @@ edited_line::edited_line (const char *filename, int line_num)
 
 edited_line::~edited_line ()
 {
+  unsigned i;
+  added_line *pred;
+
   free (m_content);
+  FOR_EACH_VEC_ELT (m_predecessors, i, pred)
+    delete pred;
 }
 
 /* A callback for deleting edited_line *, for use as a
@@ -643,6 +750,17 @@ edited_line::apply_fixit (int start_column,
                          const char *replacement_str,
                          int replacement_len)
 {
+  /* Handle newlines.  They will only ever be at the end of the
+     replacement text, thanks to the filtering in rich_location.  */
+  if (replacement_len > 1)
+    if (replacement_str[replacement_len - 1] == '\n')
+      {
+       /* Stash in m_predecessors, stripping off newline.  */
+       m_predecessors.safe_push (new added_line (replacement_str,
+                                                 replacement_len - 1));
+       return true;
+      }
+
   start_column = get_effective_column (start_column);
   next_column = get_effective_column (next_column);
 
@@ -689,6 +807,57 @@ edited_line::apply_fixit (int start_column,
   return true;
 }
 
+/* Determine the number of lines that will be present after
+   editing for this line.  Typically this is just 1, but
+   if newlines have been added before this line, they will
+   also be counted.  */
+
+int
+edited_line::get_effective_line_count () const
+{
+  return m_predecessors.length () + 1;
+}
+
+/* Subroutine of edited_file::print_content.
+   Print this line and any new lines added before it, to PP.  */
+
+void
+edited_line::print_content (pretty_printer *pp) const
+{
+  unsigned i;
+  added_line *pred;
+  FOR_EACH_VEC_ELT (m_predecessors, i, pred)
+    {
+      pp_string (pp, pred->get_content ());
+      pp_newline (pp);
+    }
+  pp_string (pp, m_content);
+}
+
+/* Subroutine of edited_file::print_run_of_changed_lines for
+   printing diff hunks to PP.
+   Print the '+' line for this line, and any newlines added
+   before it.
+   Note that if this edited_line was actually edited, the '-'
+   line has already been printed.  If it wasn't, then we merely
+   have a placeholder edited_line for adding newlines to, and
+   we need to print a ' ' line for the edited_line as we haven't
+   printed it yet.  */
+
+void
+edited_line::print_diff_lines (pretty_printer *pp) const
+{
+  unsigned i;
+  added_line *pred;
+  FOR_EACH_VEC_ELT (m_predecessors, i, pred)
+    print_diff_line (pp, '+', pred->get_content (),
+                    pred->get_len ());
+  if (actually_edited_p ())
+    print_diff_line (pp, '+', m_content, m_len);
+  else
+    print_diff_line (pp, ' ', m_content, m_len);
+}
+
 /* Ensure that the buffer for m_content is at least large enough to hold
    a string of length LEN and its 0-terminator, doubling on repeated
    allocations.  */
@@ -967,6 +1136,57 @@ test_applying_fixits_insert_after_failure (const line_table_case &case_)
   ASSERT_EQ (NULL, edit.generate_diff (false));
 }
 
+/* Test applying an "insert" fixit that adds a newline.  */
+
+static void
+test_applying_fixits_insert_containing_newline (const line_table_case &case_)
+{
+  /* Create a tempfile and write some text to it.
+     .........................0000000001111111.
+     .........................1234567890123456.  */
+  const char *old_content = ("    case 'a':\n" /* line 1. */
+                            "      x = a;\n"  /* line 2. */
+                            "    case 'b':\n" /* line 3. */
+                            "      x = b;\n");/* line 4. */
+
+  temp_source_file tmp (SELFTEST_LOCATION, ".c", old_content);
+  const char *filename = tmp.get_filename ();
+  line_table_test ltt (case_);
+  linemap_add (line_table, LC_ENTER, false, tmp.get_filename (), 3);
+
+  /* Add a "break;" on a line by itself before line 3 i.e. before
+     column 1 of line 3. */
+  location_t case_start = linemap_position_for_column (line_table, 5);
+  location_t case_finish = linemap_position_for_column (line_table, 13);
+  location_t case_loc = make_location (case_start, case_start, case_finish);
+  rich_location richloc (line_table, case_loc);
+  location_t line_start = linemap_position_for_column (line_table, 1);
+  richloc.add_fixit_insert_before (line_start, "      break;\n");
+
+  if (case_finish > LINE_MAP_MAX_LOCATION_WITH_COLS)
+    return;
+
+  edit_context edit;
+  edit.add_fixits (&richloc);
+  auto_free <char *> new_content = edit.get_content (filename);
+  ASSERT_STREQ (("    case 'a':\n"
+                "      x = a;\n"
+                "      break;\n"
+                "    case 'b':\n"
+                "      x = b;\n"),
+               new_content);
+
+  /* Verify diff.  */
+  auto_free <char *> diff = edit.generate_diff (false);
+  ASSERT_STREQ (("@@ -1,4 +1,5 @@\n"
+                "     case 'a':\n"
+                "       x = a;\n"
+                "+      break;\n"
+                "     case 'b':\n"
+                "       x = b;\n"),
+               diff);
+}
+
 /* Test applying a "replace" fixit that grows the affected line.  */
 
 static void
@@ -1057,6 +1277,44 @@ test_applying_fixits_shrinking_replace (const line_table_case &case_)
     }
 }
 
+/* Replacement fix-it hint containing a newline.  */
+
+static void
+test_applying_fixits_replace_containing_newline (const line_table_case &case_)
+{
+  /* Create a tempfile and write some text to it.
+    .........................0000000001111.
+    .........................1234567890123.  */
+  const char *old_content = "foo = bar ();\n";
+
+  temp_source_file tmp (SELFTEST_LOCATION, ".c", old_content);
+  const char *filename = tmp.get_filename ();
+  line_table_test ltt (case_);
+  linemap_add (line_table, LC_ENTER, false, filename, 1);
+
+  /* Replace the " = " with "\n  = ", as if we were reformatting an
+     overly long line.  */
+  location_t start = linemap_position_for_column (line_table, 4);
+  location_t finish = linemap_position_for_column (line_table, 6);
+  location_t loc = linemap_position_for_column (line_table, 13);
+  rich_location richloc (line_table, loc);
+  source_range range = source_range::from_locations (start, finish);
+  richloc.add_fixit_replace (range, "\n  = ");
+
+  /* Newlines are only supported within fix-it hints that
+     are at the start of lines (for entirely new lines), hence
+     this fix-it should not be displayed.  */
+  ASSERT_TRUE (richloc.seen_impossible_fixit_p ());
+
+  if (finish > LINE_MAP_MAX_LOCATION_WITH_COLS)
+    return;
+
+  edit_context edit;
+  edit.add_fixits (&richloc);
+  auto_free <char *> new_content = edit.get_content (filename);
+  //ASSERT_STREQ ("foo\n  = bar ();\n", new_content);
+}
+
 /* Test applying a "remove" fixit.  */
 
 static void
@@ -1204,6 +1462,32 @@ change_line (edit_context &edit, int line_num)
   return loc;
 }
 
+/* Subroutine of test_applying_fixits_multiple_lines.
+   Add the text "INSERTED\n" in front of the given line.  */
+
+static location_t
+insert_line (edit_context &edit, int line_num)
+{
+  const line_map_ordinary *ord_map
+    = LINEMAPS_LAST_ORDINARY_MAP (line_table);
+  const int column = 1;
+  location_t loc =
+    linemap_position_for_line_and_column (line_table, ord_map,
+                                         line_num, column);
+
+  expanded_location exploc = expand_location (loc);
+  if (loc <= LINE_MAP_MAX_LOCATION_WITH_COLS)
+    {
+      ASSERT_EQ (line_num, exploc.line);
+      ASSERT_EQ (column, exploc.column);
+    }
+
+  rich_location insert (line_table, loc);
+  insert.add_fixit_insert_before ("INSERTED\n");
+  edit.add_fixits (&insert);
+  return loc;
+}
+
 /* Test of editing multiple lines within a long file,
    to ensure that diffs are generated as expected.  */
 
@@ -1229,6 +1513,7 @@ test_applying_fixits_multiple_lines (const line_table_case &case_)
   change_line (edit, 2);
   change_line (edit, 3);
   change_line (edit, 4);
+  insert_line (edit, 5);
 
   /* A run of nearby lines, within the contextual limit.  */
   change_line (edit, 150);
@@ -1240,7 +1525,7 @@ test_applying_fixits_multiple_lines (const line_table_case &case_)
 
   /* Verify diff.  */
   auto_free <char *> diff = edit.generate_diff (false);
-  ASSERT_STREQ ("@@ -1,7 +1,7 @@\n"
+  ASSERT_STREQ ("@@ -1,7 +1,8 @@\n"
                " line 1\n"
                "-line 2\n"
                "-line 3\n"
@@ -1248,10 +1533,11 @@ test_applying_fixits_multiple_lines (const line_table_case &case_)
                "+CHANGED: line 2\n"
                "+CHANGED: line 3\n"
                "+CHANGED: line 4\n"
+               "+INSERTED\n"
                " line 5\n"
                " line 6\n"
                " line 7\n"
-               "@@ -147,10 +147,10 @@\n"
+               "@@ -147,10 +148,10 @@\n"
                " line 147\n"
                " line 148\n"
                " line 149\n"
@@ -1510,8 +1796,10 @@ edit_context_c_tests ()
   for_each_line_table_case (test_applying_fixits_insert_after);
   for_each_line_table_case (test_applying_fixits_insert_after_at_line_end);
   for_each_line_table_case (test_applying_fixits_insert_after_failure);
+  for_each_line_table_case (test_applying_fixits_insert_containing_newline);
   for_each_line_table_case (test_applying_fixits_growing_replace);
   for_each_line_table_case (test_applying_fixits_shrinking_replace);
+  for_each_line_table_case (test_applying_fixits_replace_containing_newline);
   for_each_line_table_case (test_applying_fixits_remove);
   for_each_line_table_case (test_applying_fixits_multiple);
   for_each_line_table_case (test_applying_fixits_multiple_lines);
index 4a7c4e44a6b5f7e3644bc98fdc09937872129e4a..33a5cde0ab1f24a5624cd74007f3edc99c0668fe 100644 (file)
@@ -1,3 +1,16 @@
+2017-05-02  David Malcolm  <dmalcolm@redhat.com>
+
+       * gcc.dg/plugin/diagnostic-test-show-locus-bw.c
+       (test_fixit_insert_newline): New function.
+       * gcc.dg/plugin/diagnostic-test-show-locus-color.c
+       (test_fixit_insert_newline): New function.
+       * gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c
+       (test_fixit_insert_newline): New function.
+       * gcc.dg/plugin/diagnostic-test-show-locus-parseable-fixits.c
+       (test_fixit_insert_newline): New function.
+       * gcc.dg/plugin/diagnostic_plugin_test_show_locus.c
+       (test_show_locus): Handle test_fixit_insert_newline.
+
 2017-05-02  Bin Cheng  <bin.cheng@arm.com>
 
        * g++.dg/tree-ssa/ivopts-3.C: Adjust test string.
index e8112bfa3a5522610ce5707849acd6e3b6009b98..25b9d6cb3826d47fd139fee80e06b8a9f6e50ab2 100644 (file)
@@ -249,3 +249,23 @@ void test_many_nested_locations (void)
      MOLLIT ANIM ID EST LABORUM
    { dg-end-multiline-output "" } */
 }
+
+/* Unit test for rendering of fix-it hints that add new lines.  */
+
+void test_fixit_insert_newline (void)
+{
+#if 0
+  switch (op)
+    {
+    case 'a':
+      x = a;
+    case 'b':  /* { dg-warning "newline insertion" } */
+      x = b;
+    }
+/* { dg-begin-multiline-output "" }
++      break;
+     case 'b':
+     ^~~~~~~~
+   { dg-end-multiline-output "" } */
+#endif
+}
index 712375e0b192792dd8c5e49eddc715e074aa4dd2..3fb1c240238e50f29fd3a4a4a0dce023fba2c392 100644 (file)
@@ -193,3 +193,23 @@ void test_fixit_replace (void)
    { dg-end-multiline-output "" } */
 #endif
 }
+
+/* Unit test for rendering of fix-it hints that add new lines.  */
+
+void test_fixit_insert_newline (void)
+{
+#if 0
+  switch (op)
+    {
+    case 'a':
+      x = a;
+    case 'b':  /* { dg-warning "newline insertion" } */
+      x = b;
+    }
+/* { dg-begin-multiline-output "" }
++\e[32m\e[K      break;\e[m\e[K
+     \e[01;35m\e[Kcase 'b'\e[m\e[K:
+     \e[01;35m\e[K^~~~~~~~\e[m\e[K
+   { dg-end-multiline-output "" } */
+#endif
+}
index afbaf6354012f2979671454a0c29fab7c876b3ba..4522dcd350e7189aac42b98817021253d0718100 100644 (file)
@@ -36,7 +36,20 @@ void test_fixit_replace (void)
 #endif
 }
 
+/* Unit test for rendering of fix-it hints that add new lines.  */
 
+void test_fixit_insert_newline (void)
+{
+#if 0
+  switch (op)
+    {
+    case 'a':
+      x = a;
+    case 'b': /* { dg-warning "newline insertion" } */
+      x = b;
+    }
+#endif
+}
 
 /* Verify the output from -fdiagnostics-generate-patch.
    We expect a header, containing the filename.  This is the absolute path,
@@ -74,4 +87,12 @@ void test_fixit_replace (void)
  #endif
  }
  
+@@ -45,6 +45,7 @@
+     {
+     case 'a':
+       x = a;
++      break;
+     case 'b':
+       x = b;
+     }
    { dg-end-multiline-output "" } */
index 1490c981e4533b457cbcb318e9bf43136d7f911a..25a7c3c507216221738b98df5e22c789077e1ae8 100644 (file)
@@ -39,3 +39,19 @@ void test_fixit_replace (void)
 /* { dg-regexp "fix-it:.*\\{38:3-38:21\\}:.*" } */
 #endif
 }
+
+/* Unit test for rendering of fix-it hints that add new lines.  */
+
+void test_fixit_insert_newline (void)
+{
+#if 0
+  switch (op)
+    {
+    case 'a':
+      x = a;
+    case 'b': /* { dg-warning "newline insertion" } */
+      x = b;
+    }
+/* { dg-regexp "fix-it:.*\\{52:1-52:1\\}:.*\\n" } */
+#endif
+}
index 3efc7dfa0b4c40ed7b9cad8e75da79011cfdce68..6afb58414f0b6e80398a47c428165c9ac0d45417 100644 (file)
@@ -268,6 +268,18 @@ test_show_locus (function *fun)
       warning_at_rich_loc (&richloc, 0, "example of insertion hints");
     }
 
+  if (0 == strcmp (fnname, "test_fixit_insert_newline"))
+    {
+      const int line = fnstart_line + 6;
+      location_t line_start = get_loc (line, 0);
+      location_t case_start = get_loc (line, 4);
+      location_t case_finish = get_loc (line, 11);
+      location_t case_loc = make_location (case_start, case_start, case_finish);
+      rich_location richloc (line_table, case_loc);
+      richloc.add_fixit_insert_before (line_start, "      break;\n");
+      warning_at_rich_loc (&richloc, 0, "example of newline insertion hint");
+    }
+
   if (0 == strcmp (fnname, "test_fixit_remove"))
     {
       const int line = fnstart_line + 2;
index ab225d535191787451a9673733ab0882ece8e95c..c01ab44d3f7cc11edc5caedaa51d31096e204753 100644 (file)
@@ -1,3 +1,14 @@
+2017-05-02  David Malcolm  <dmalcolm@redhat.com>
+
+       * include/line-map.h (class rich_location): Update description of
+       newline handling.
+       (class fixit_hint): Likewise.
+       (fixit_hint::ends_with_newline_p): New decl.
+       * line-map.c (rich_location::maybe_add_fixit): Support newlines
+       in fix-it hints that are insertions of single lines at the start
+       of a line.  Don't consolidate into such fix-it hints.
+       (fixit_hint::ends_with_newline_p): New method.
+
 2017-05-01  David Malcolm  <dmalcolm@redhat.com>
 
        * include/line-map.h (source_range::intersects_line_p): Delete.
index 0c44f01b3bd544491cec2508335f0b6ebb365d6e..90d65d746ab43df5badd06223c0c7be307ada0ee 100644 (file)
@@ -1551,7 +1551,11 @@ class fixit_hint;
 
    Attempts to add a fix-it hint within a macro expansion will fail.
 
-   We do not yet support newlines in fix-it text; attempts to do so will fail.
+   There is only limited support for newline characters in fix-it hints:
+   only hints with newlines which insert an entire new line are permitted,
+   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.
 
    The rich_location API handles these failures gracefully, so that
    diagnostics can attempt to add fix-it hints without each needing
@@ -1690,7 +1694,13 @@ protected:
        [start, next_loc)
    Insertions have start == next_loc: "replace" the empty string at the
    start location with the new string.
-   Deletions are replacement with the empty string.  */
+   Deletions are replacement with the empty string.
+
+   There is only limited support for newline characters in fix-it hints
+   as noted above in the comment for class rich_location.
+   A fixit_hint instance can have at most one newline character; if
+   present, the newline character must be the final character of
+   the content (preventing e.g. fix-its that split a pre-existing line).  */
 
 class fixit_hint
 {
@@ -1712,6 +1722,8 @@ class fixit_hint
 
   bool insertion_p () const { return m_start == m_next_loc; }
 
+  bool ends_with_newline_p () const;
+
  private:
   /* We don't use source_range here since, unlike most places,
      this is a half-open/half-closed range:
index 176e58d25451750c9c2b157ffc1eeac3452a66c4..c4b7cb2365126d51bc5ad861eba41c6ee807555a 100644 (file)
@@ -2325,16 +2325,44 @@ rich_location::maybe_add_fixit (source_location start,
   if (reject_impossible_fixit (next_loc))
     return;
 
-  /* We do not yet support newlines within fix-it hints.  */
-  if (strchr (new_content, '\n'))
+  const char *newline = strchr (new_content, '\n');
+  if (newline)
     {
-      stop_supporting_fixits ();
-      return;
+      /* For now, we can only support insertion of whole lines
+        i.e. starts at start of line, and the newline is at the end of
+        the insertion point.  */
+
+      /* It must be an insertion, not a replacement/deletion.  */
+      if (start != next_loc)
+       {
+         stop_supporting_fixits ();
+         return;
+       }
+
+      /* 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 ();
+         return;
+       }
+
+      /* The newline must be at end of NEW_CONTENT.
+        We could eventually split up fix-its at newlines if we wanted
+        to allow more generality (e.g. to allow adding multiple lines
+        with one add_fixit call.  */
+      if (newline[1] != '\0')
+       {
+         stop_supporting_fixits ();
+         return;
+       }
     }
 
-  /* Consolidate neighboring fixits.  */
+  /* Consolidate neighboring fixits.
+     Don't consolidate into newline-insertion fixits.  */
   fixit_hint *prev = get_last_fixit_hint ();
-  if (prev)
+  if (prev && !prev->ends_with_newline_p ())
     if (prev->maybe_append (start, next_loc, new_content))
       return;
 
@@ -2398,3 +2426,13 @@ fixit_hint::maybe_append (source_location start,
   m_bytes[m_len] = '\0';
   return true;
 }
+
+/* Return true iff this hint's content ends with a newline.  */
+
+bool
+fixit_hint::ends_with_newline_p () const
+{
+  if (m_len == 0)
+    return false;
+  return m_bytes[m_len - 1] == '\n';
+}