This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[committed] Support fix-it hints that add new lines
- 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, 2 May 2017 15:37:49 -0400
- Subject: [committed] Support fix-it hints that add new lines
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx03.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 44BEE126712
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 44BEE126712
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);
^~~~~~~
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
Committed to trunk as r247522.
I'm working on implementing both of the above examples.
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.
---
gcc/diagnostic-show-locus.c | 211 ++++++++---
gcc/edit-context.c | 394 ++++++++++++++++++---
.../gcc.dg/plugin/diagnostic-test-show-locus-bw.c | 20 ++
.../plugin/diagnostic-test-show-locus-color.c | 20 ++
.../diagnostic-test-show-locus-generate-patch.c | 21 ++
.../diagnostic-test-show-locus-parseable-fixits.c | 16 +
.../plugin/diagnostic_plugin_test_show_locus.c | 12 +
libcpp/include/line-map.h | 16 +-
libcpp/line-map.c | 50 ++-
9 files changed, 652 insertions(+), 108 deletions(-)
diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index 3c10b69..b91c9d5 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -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);
}
diff --git a/gcc/edit-context.c b/gcc/edit-context.c
index bea8a8a..6f35ca2 100644
--- a/gcc/edit-context.c
+++ b/gcc/edit-context.c
@@ -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);
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw.c
index e8112bf..25b9d6c 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw.c
@@ -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
+}
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-color.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-color.c
index 712375e..3fb1c24 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-color.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-color.c
@@ -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 "" }
++[32m[K break;[m[K
+ [01;35m[Kcase 'b'[m[K:
+ [01;35m[K^~~~~~~~[m[K
+ { dg-end-multiline-output "" } */
+#endif
+}
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c
index afbaf63..4522dcd 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c
@@ -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 "" } */
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-parseable-fixits.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-parseable-fixits.c
index 1490c98..25a7c3c 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-parseable-fixits.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-parseable-fixits.c
@@ -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
+}
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_show_locus.c b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_show_locus.c
index 3efc7df..6afb584 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_show_locus.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_show_locus.c
@@ -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;
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 0c44f01..90d65d7 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -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:
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index 176e58d..c4b7cb2 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -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';
+}
--
1.8.5.3