Commit ad53f123 by David Malcolm Committed by David Malcolm

Support fix-it hints that add new lines

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
parent 19576ba7
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> 2017-05-02 Bin Cheng <bin.cheng@arm.com>
* tree-ssa-loop-ivopts.c (get_scaled_computation_cost_at): Delete * tree-ssa-loop-ivopts.c (get_scaled_computation_cost_at): Delete
......
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> 2017-05-02 Bin Cheng <bin.cheng@arm.com>
* g++.dg/tree-ssa/ivopts-3.C: Adjust test string. * g++.dg/tree-ssa/ivopts-3.C: Adjust test string.
......
...@@ -249,3 +249,23 @@ void test_many_nested_locations (void) ...@@ -249,3 +249,23 @@ void test_many_nested_locations (void)
MOLLIT ANIM ID EST LABORUM MOLLIT ANIM ID EST LABORUM
{ dg-end-multiline-output "" } */ { 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
}
...@@ -193,3 +193,23 @@ void test_fixit_replace (void) ...@@ -193,3 +193,23 @@ void test_fixit_replace (void)
{ dg-end-multiline-output "" } */ { dg-end-multiline-output "" } */
#endif #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 "" }
+ break;
case 'b':
^~~~~~~~
{ dg-end-multiline-output "" } */
#endif
}
...@@ -36,7 +36,20 @@ void test_fixit_replace (void) ...@@ -36,7 +36,20 @@ void test_fixit_replace (void)
#endif #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. /* Verify the output from -fdiagnostics-generate-patch.
We expect a header, containing the filename. This is the absolute path, We expect a header, containing the filename. This is the absolute path,
...@@ -74,4 +87,12 @@ void test_fixit_replace (void) ...@@ -74,4 +87,12 @@ void test_fixit_replace (void)
#endif #endif
} }
@@ -45,6 +45,7 @@
{
case 'a':
x = a;
+ break;
case 'b':
x = b;
}
{ dg-end-multiline-output "" } */ { dg-end-multiline-output "" } */
...@@ -39,3 +39,19 @@ void test_fixit_replace (void) ...@@ -39,3 +39,19 @@ void test_fixit_replace (void)
/* { dg-regexp "fix-it:.*\\{38:3-38:21\\}:.*" } */ /* { dg-regexp "fix-it:.*\\{38:3-38:21\\}:.*" } */
#endif #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
}
...@@ -268,6 +268,18 @@ test_show_locus (function *fun) ...@@ -268,6 +268,18 @@ test_show_locus (function *fun)
warning_at_rich_loc (&richloc, 0, "example of insertion hints"); 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")) if (0 == strcmp (fnname, "test_fixit_remove"))
{ {
const int line = fnstart_line + 2; const int line = fnstart_line + 2;
......
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> 2017-05-01 David Malcolm <dmalcolm@redhat.com>
* include/line-map.h (source_range::intersects_line_p): Delete. * include/line-map.h (source_range::intersects_line_p): Delete.
......
...@@ -1551,7 +1551,11 @@ class fixit_hint; ...@@ -1551,7 +1551,11 @@ class fixit_hint;
Attempts to add a fix-it hint within a macro expansion will fail. 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 The rich_location API handles these failures gracefully, so that
diagnostics can attempt to add fix-it hints without each needing diagnostics can attempt to add fix-it hints without each needing
...@@ -1690,7 +1694,13 @@ protected: ...@@ -1690,7 +1694,13 @@ protected:
[start, next_loc) [start, next_loc)
Insertions have start == next_loc: "replace" the empty string at the Insertions have start == next_loc: "replace" the empty string at the
start location with the new string. 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 class fixit_hint
{ {
...@@ -1712,6 +1722,8 @@ class fixit_hint ...@@ -1712,6 +1722,8 @@ class fixit_hint
bool insertion_p () const { return m_start == m_next_loc; } bool insertion_p () const { return m_start == m_next_loc; }
bool ends_with_newline_p () const;
private: private:
/* We don't use source_range here since, unlike most places, /* We don't use source_range here since, unlike most places,
this is a half-open/half-closed range: this is a half-open/half-closed range:
......
...@@ -2325,16 +2325,44 @@ rich_location::maybe_add_fixit (source_location start, ...@@ -2325,16 +2325,44 @@ rich_location::maybe_add_fixit (source_location start,
if (reject_impossible_fixit (next_loc)) if (reject_impossible_fixit (next_loc))
return; return;
/* We do not yet support newlines within fix-it hints. */ const char *newline = strchr (new_content, '\n');
if (strchr (new_content, '\n')) if (newline)
{ {
stop_supporting_fixits (); /* For now, we can only support insertion of whole lines
return; 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 (); 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)) if (prev->maybe_append (start, next_loc, new_content))
return; return;
...@@ -2398,3 +2426,13 @@ fixit_hint::maybe_append (source_location start, ...@@ -2398,3 +2426,13 @@ fixit_hint::maybe_append (source_location start,
m_bytes[m_len] = '\0'; m_bytes[m_len] = '\0';
return true; 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';
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment