diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 0157559daef..6441ac1e688 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,42 @@ +2017-05-01 David Malcolm + + * diagnostic-show-locus.c (layout::get_expanded_location): Rewrite + to use new fixit_hint representation, using the "replace" logic. + (get_line_span_for_fixit_hint): Likewise. + (layout::print_any_fixits): Likewise. + (selftest::test_one_liner_many_fixits): Rename to... + (selftest::test_one_liner_many_fixits_1): ...this, and update + comment and expected output to reflect that the multiple fix-it + hints are now consolidated into one insertion. + (selftest::test_one_liner_many_fixits_2): New test. + (selftest::test_diagnostic_show_locus_one_liner): Update for + above. + (selftest::test_fixit_consolidation): Update for fix-it API + change. + * diagnostic.c (print_parseable_fixits): Likewise. + * edit-context.c (edited_line::m_line_events): Convert from + auto_vec to auto_vec . + (class line_event): Convert from abstract base class to a concrete + class, taking over the role of replace_event. + (class insert_event): Delete. + (class replace_event): Rename to class line_event. Convert to + half-open range. + (edit_context::add_fixits): Reimplement. + (edit_context::apply_insert): Delete. + (edit_context::apply_replace): Rename to... + (edit_context::apply_fixit): ...this. Convert to half-open range. + (edited_file::apply_insert): Delete. + (edited_file::apply_replace): Rename to... + (edited_file::apply_fixit): ...this. + (edited_line::~edited_line): Drop deletion of events. + (edited_line::apply_insert): Delete. + (edited_line::apply_replace): Rename to... + (edited_line::apply_fixit): ...this. Convert to half-open range. + Update for change to type of m_line_events. + * edit-context.h (edit_context::apply_insert): Delete. + (edit_context::apply_replace): Rename to... + (edit_context::apply_fixit): ...this. + 2017-05-01 Martin Sebor * gimple-ssa-sprintf.c (format_integer): Set knownrange when it's diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c index 5c386ae256d..3c10b69c232 100644 --- a/gcc/diagnostic-show-locus.c +++ b/gcc/diagnostic-show-locus.c @@ -941,32 +941,10 @@ layout::get_expanded_location (const line_span *line_span) const bool layout::validate_fixit_hint_p (const fixit_hint *hint) { - switch (hint->get_kind ()) - { - case fixit_hint::INSERT: - { - const fixit_insert *insert = static_cast (hint); - location_t loc = insert->get_location (); - if (LOCATION_FILE (loc) != m_exploc.file) - return false; - } - break; - - case fixit_hint::REPLACE: - { - const fixit_replace *replace - = static_cast (hint); - source_range src_range = replace->get_range (); - if (LOCATION_FILE (src_range.m_start) != m_exploc.file) - return false; - if (LOCATION_FILE (src_range.m_finish) != m_exploc.file) - return false; - } - break; - - default: - gcc_unreachable (); - } + if (LOCATION_FILE (hint->get_start_loc ()) != m_exploc.file) + return false; + if (LOCATION_FILE (hint->get_next_loc ()) != m_exploc.file) + return false; return true; } @@ -979,30 +957,8 @@ static line_span get_line_span_for_fixit_hint (const fixit_hint *hint) { gcc_assert (hint); - switch (hint->get_kind ()) - { - case fixit_hint::INSERT: - { - const fixit_insert *insert = static_cast (hint); - location_t loc = insert->get_location (); - int line = LOCATION_LINE (loc); - return line_span (line, line); - } - break; - - case fixit_hint::REPLACE: - { - const fixit_replace *replace - = static_cast (hint); - source_range src_range = replace->get_range (); - return line_span (LOCATION_LINE (src_range.m_start), - LOCATION_LINE (src_range.m_finish)); - } - break; - - default: - gcc_unreachable (); - } + return line_span (LOCATION_LINE (hint->get_start_loc ()), + LOCATION_LINE (hint->get_next_loc ())); } /* We want to print the pertinent source code at a diagnostic. The @@ -1264,62 +1220,47 @@ layout::print_any_fixits (int row) if (hint->affects_line_p (m_exploc.file, row)) { /* For now we assume each fixit hint can only touch one line. */ - switch (hint->get_kind ()) + if (hint->insertion_p ()) { - case fixit_hint::INSERT: - { - const fixit_insert *insert - = static_cast (hint); - /* This assumes the insertion just affects one line. */ - int start_column - = LOCATION_COLUMN (insert->get_location ()); - move_to_column (&column, start_column); - m_colorizer.set_fixit_insert (); - pp_string (m_pp, insert->get_string ()); - m_colorizer.set_normal_text (); - column += insert->get_length (); - } - break; + /* This assumes the insertion just affects one line. */ + int start_column = LOCATION_COLUMN (hint->get_start_loc ()); + move_to_column (&column, start_column); + m_colorizer.set_fixit_insert (); + pp_string (m_pp, hint->get_string ()); + m_colorizer.set_normal_text (); + column += hint->get_length (); + } + else + { + int line = LOCATION_LINE (hint->get_start_loc ()); + int start_column = LOCATION_COLUMN (hint->get_start_loc ()); + int finish_column = LOCATION_COLUMN (hint->get_next_loc ()) - 1; - case fixit_hint::REPLACE: - { - const fixit_replace *replace - = static_cast (hint); - source_range src_range = replace->get_range (); - int line = LOCATION_LINE (src_range.m_start); - int start_column = LOCATION_COLUMN (src_range.m_start); - int finish_column = LOCATION_COLUMN (src_range.m_finish); - - /* If the range of the replacement wasn't printed in the - annotation line, then print an extra underline to - indicate exactly what is being replaced. - Always show it for removals. */ - if (!annotation_line_showed_range_p (line, start_column, - finish_column) - || replace->get_length () == 0) - { - move_to_column (&column, start_column); - m_colorizer.set_fixit_delete (); - for (; column <= finish_column; column++) - pp_character (m_pp, '-'); - m_colorizer.set_normal_text (); - } - /* Print the replacement text. REPLACE also covers - removals, so only do this extra work (potentially starting - a new line) if we have actual replacement text. */ - if (replace->get_length () > 0) - { - move_to_column (&column, start_column); - m_colorizer.set_fixit_insert (); - pp_string (m_pp, replace->get_string ()); - m_colorizer.set_normal_text (); - column += replace->get_length (); - } - } - break; - - default: - gcc_unreachable (); + /* If the range of the replacement wasn't printed in the + annotation line, then print an extra underline to + indicate exactly what is being replaced. + Always show it for removals. */ + if (!annotation_line_showed_range_p (line, start_column, + finish_column) + || hint->get_length () == 0) + { + move_to_column (&column, start_column); + m_colorizer.set_fixit_delete (); + for (; column <= finish_column; column++) + pp_character (m_pp, '-'); + m_colorizer.set_normal_text (); + } + /* Print the replacement text. REPLACE also covers + removals, so only do this extra work (potentially starting + a new line) if we have actual replacement text. */ + if (hint->get_length () > 0) + { + move_to_column (&column, start_column); + m_colorizer.set_fixit_insert (); + pp_string (m_pp, hint->get_string ()); + m_colorizer.set_normal_text (); + column += hint->get_length (); + } } } } @@ -1852,41 +1793,45 @@ test_one_liner_fixit_validation_adhoc_locations () } } -/* Ensure that we can add an arbitrary number of fix-it hints to a - rich_location. */ +/* Test of consolidating insertions at the same location. */ static void -test_one_liner_many_fixits () +test_one_liner_many_fixits_1 () { test_diagnostic_context dc; location_t equals = linemap_position_for_column (line_table, 5); rich_location richloc (line_table, equals); for (int i = 0; i < 19; i++) richloc.add_fixit_insert_before ("a"); + ASSERT_EQ (1, richloc.get_num_fixit_hints ()); + diagnostic_show_locus (&dc, &richloc, DK_ERROR); + ASSERT_STREQ ("\n" + " foo = bar.field;\n" + " ^\n" + " aaaaaaaaaaaaaaaaaaa\n", + pp_formatted_text (dc.printer)); +} + +/* Ensure that we can add an arbitrary number of fix-it hints to a + rich_location, even if they are not consolidated. */ + +static void +test_one_liner_many_fixits_2 () +{ + test_diagnostic_context dc; + location_t equals = linemap_position_for_column (line_table, 5); + rich_location richloc (line_table, equals); + for (int i = 0; i < 19; i++) + { + location_t loc = linemap_position_for_column (line_table, i * 2); + richloc.add_fixit_insert_before (loc, "a"); + } ASSERT_EQ (19, richloc.get_num_fixit_hints ()); diagnostic_show_locus (&dc, &richloc, DK_ERROR); ASSERT_STREQ ("\n" " foo = bar.field;\n" " ^\n" - " a\n" - " a\n" - " a\n" - " a\n" - " a\n" - " a\n" - " a\n" - " a\n" - " a\n" - " a\n" - " a\n" - " a\n" - " a\n" - " a\n" - " a\n" - " a\n" - " a\n" - " a\n" - " a\n", + "a a a a a a a a a a a a a a a a a a a\n", pp_formatted_text (dc.printer)); } @@ -1924,7 +1869,8 @@ test_diagnostic_show_locus_one_liner (const line_table_case &case_) test_one_liner_fixit_replace_non_equal_range (); test_one_liner_fixit_replace_equal_secondary_range (); test_one_liner_fixit_validation_adhoc_locations (); - test_one_liner_many_fixits (); + test_one_liner_many_fixits_1 (); + test_one_liner_many_fixits_2 (); } /* Verify that we print fixits even if they only affect lines @@ -2027,6 +1973,7 @@ test_fixit_consolidation (const line_table_case &case_) const location_t c16 = linemap_position_for_column (line_table, 16); const location_t c17 = linemap_position_for_column (line_table, 17); const location_t c20 = linemap_position_for_column (line_table, 20); + const location_t c21 = linemap_position_for_column (line_table, 21); const location_t caret = c10; /* Insert + insert. */ @@ -2105,11 +2052,9 @@ test_fixit_consolidation (const line_table_case &case_) /* They should have been merged into a single "replace". */ ASSERT_EQ (1, richloc.get_num_fixit_hints ()); const fixit_hint *hint = richloc.get_fixit_hint (0); - ASSERT_EQ (fixit_hint::REPLACE, hint->get_kind ()); - const fixit_replace *replace = (const fixit_replace *)hint; - ASSERT_STREQ ("foobar", replace->get_string ()); - ASSERT_EQ (c10, replace->get_range ().m_start); - ASSERT_EQ (c20, replace->get_range ().m_finish); + ASSERT_STREQ ("foobar", hint->get_string ()); + ASSERT_EQ (c10, hint->get_start_loc ()); + ASSERT_EQ (c21, hint->get_next_loc ()); } } @@ -2129,11 +2074,9 @@ test_fixit_consolidation (const line_table_case &case_) range extended to cover that of the removal. */ ASSERT_EQ (1, richloc.get_num_fixit_hints ()); const fixit_hint *hint = richloc.get_fixit_hint (0); - ASSERT_EQ (fixit_hint::REPLACE, hint->get_kind ()); - const fixit_replace *replace = (const fixit_replace *)hint; - ASSERT_STREQ ("foo", replace->get_string ()); - ASSERT_EQ (c10, replace->get_range ().m_start); - ASSERT_EQ (c20, replace->get_range ().m_finish); + ASSERT_STREQ ("foo", hint->get_string ()); + ASSERT_EQ (c10, hint->get_start_loc ()); + ASSERT_EQ (c21, hint->get_next_loc ()); } } @@ -2151,11 +2094,9 @@ test_fixit_consolidation (const line_table_case &case_) /* They should have been merged into a single "replace-with-empty". */ ASSERT_EQ (1, richloc.get_num_fixit_hints ()); const fixit_hint *hint = richloc.get_fixit_hint (0); - ASSERT_EQ (fixit_hint::REPLACE, hint->get_kind ()); - const fixit_replace *replace = (const fixit_replace *)hint; - ASSERT_STREQ ("", replace->get_string ()); - ASSERT_EQ (c10, replace->get_range ().m_start); - ASSERT_EQ (c20, replace->get_range ().m_finish); + ASSERT_STREQ ("", hint->get_string ()); + ASSERT_EQ (c10, hint->get_start_loc ()); + ASSERT_EQ (c21, hint->get_next_loc ()); } } } diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c index dbaf8d6aed6..dc81755a6af 100644 --- a/gcc/diagnostic.c +++ b/gcc/diagnostic.c @@ -757,43 +757,13 @@ print_parseable_fixits (pretty_printer *pp, rich_location *richloc) expanded_location start_exploc = expand_location (start_loc); pp_string (pp, "fix-it:"); print_escaped_string (pp, start_exploc.file); - source_location end_loc; - /* For compatibility with clang, print as a half-open range. */ - if (hint->maybe_get_end_loc (&end_loc)) - { - expanded_location end_exploc = expand_location (end_loc); - pp_printf (pp, ":{%i:%i-%i:%i}:", - start_exploc.line, start_exploc.column, - end_exploc.line, end_exploc.column + 1); - } - else - { - pp_printf (pp, ":{%i:%i-%i:%i}:", - start_exploc.line, start_exploc.column, - start_exploc.line, start_exploc.column); - } - switch (hint->get_kind ()) - { - case fixit_hint::INSERT: - { - const fixit_insert *insert - = static_cast (hint); - print_escaped_string (pp, insert->get_string ()); - } - break; - - case fixit_hint::REPLACE: - { - const fixit_replace *replace - = static_cast (hint); - print_escaped_string (pp, replace->get_string ()); - } - break; - - default: - gcc_unreachable (); - } + source_location next_loc = hint->get_next_loc (); + expanded_location next_exploc = expand_location (next_loc); + pp_printf (pp, ":{%i:%i-%i:%i}:", + start_exploc.line, start_exploc.column, + next_exploc.line, next_exploc.column); + print_escaped_string (pp, hint->get_string ()); pp_newline (pp); } } diff --git a/gcc/edit-context.c b/gcc/edit-context.c index db8a5dc1324..bea8a8af688 100644 --- a/gcc/edit-context.c +++ b/gcc/edit-context.c @@ -45,8 +45,6 @@ class edit_context; class edited_file; class edited_line; class line_event; - class insert_event; - class replace_event; /* A struct to hold the params of a print_diff call. */ @@ -71,11 +69,10 @@ class edited_file const char *get_filename () const { return m_filename; } char *get_content (); - bool apply_insert (int line, int column, const char *str, int len); - bool apply_replace (int line, int start_column, - int finish_column, - const char *replacement_str, - int replacement_len); + bool apply_fixit (int line, int start_column, + int next_column, + const char *replacement_str, + int replacement_len); int get_effective_column (int line, int column); static int call_print_diff (const char *, edited_file *file, @@ -119,11 +116,10 @@ class edited_line int get_len () const { return m_len; } int get_effective_column (int orig_column) const; - bool apply_insert (int column, const char *str, int len); - bool apply_replace (int start_column, - int finish_column, - const char *replacement_str, - int replacement_len); + bool apply_fixit (int start_column, + int next_column, + const char *replacement_str, + int replacement_len); private: void ensure_capacity (int len); @@ -134,55 +130,23 @@ class edited_line char *m_content; int m_len; int m_alloc_sz; - auto_vec m_line_events; + auto_vec m_line_events; }; -/* Abstract base class for representing events that have occurred - on one line of one file. */ +/* Class for representing edit events that have occurred on one line of + one file: the replacement of some text betweeen some columns + on the line. + + Subsequent events will need their columns adjusting if they're + are on this line and their column is >= the start point. */ class line_event { public: - virtual ~line_event () {} - virtual int get_effective_column (int orig_column) const = 0; -}; + line_event (int start, int next, int len) : m_start (start), + m_next (next), m_delta (len - (next - start)) {} -/* Concrete subclass of line_event: an insertion of some text - at some column on the line. - - Subsequent events will need their columns adjusting if they're - are on this line and their column is >= the insertion point. */ - -class insert_event : public line_event -{ - public: - insert_event (int column, int len) : m_column (column), m_len (len) {} - int get_effective_column (int orig_column) const FINAL OVERRIDE - { - if (orig_column >= m_column) - return orig_column + m_len; - else - return orig_column; - } - - private: - int m_column; - int m_len; -}; - -/* Concrete subclass of line_event: the replacement of some text - betweeen some columns on the line. - - Subsequent events will need their columns adjusting if they're - are on this line and their column is >= the finish point. */ - -class replace_event : public line_event -{ - public: - replace_event (int start, int finish, int len) : m_start (start), - m_finish (finish), m_delta (len - (finish + 1 - start)) {} - - int get_effective_column (int orig_column) const FINAL OVERRIDE + int get_effective_column (int orig_column) const { if (orig_column >= m_start) return orig_column += m_delta; @@ -192,7 +156,7 @@ class replace_event : public line_event private: int m_start; - int m_finish; + int m_next; int m_delta; }; @@ -221,27 +185,8 @@ edit_context::add_fixits (rich_location *richloc) for (unsigned i = 0; i < richloc->get_num_fixit_hints (); i++) { const fixit_hint *hint = richloc->get_fixit_hint (i); - switch (hint->get_kind ()) - { - case fixit_hint::INSERT: - if (!apply_insert ((const fixit_insert *)hint)) - { - /* Failure. */ - m_valid = false; - return; - } - break; - case fixit_hint::REPLACE: - if (!apply_replace ((const fixit_replace *)hint)) - { - /* Failure. */ - m_valid = false; - return; - } - break; - default: - gcc_unreachable (); - } + if (!apply_fixit (hint)) + m_valid = false; } } @@ -302,45 +247,25 @@ edit_context::print_diff (pretty_printer *pp, bool show_filenames) applied, or false otherwise. */ bool -edit_context::apply_insert (const fixit_insert *insert) +edit_context::apply_fixit (const fixit_hint *hint) { - expanded_location exploc = expand_location (insert->get_location ()); - - if (exploc.column == 0) + expanded_location start = expand_location (hint->get_start_loc ()); + expanded_location next_loc = expand_location (hint->get_next_loc ()); + if (start.file != next_loc.file) return false; - - edited_file &file = get_or_insert_file (exploc.file); - if (!m_valid) - return false; - return file.apply_insert (exploc.line, exploc.column, insert->get_string (), - insert->get_length ()); -} - -/* Attempt to apply the given fixit. Return true if it can be - applied, or false otherwise. */ - -bool -edit_context::apply_replace (const fixit_replace *replace) -{ - source_range range = replace->get_range (); - - expanded_location start = expand_location (range.m_start); - expanded_location finish = expand_location (range.m_finish); - if (start.file != finish.file) - return false; - if (start.line != finish.line) + if (start.line != next_loc.line) return false; if (start.column == 0) return false; - if (finish.column == 0) + if (next_loc.column == 0) return false; edited_file &file = get_or_insert_file (start.file); if (!m_valid) return false; - return file.apply_replace (start.line, start.column, finish.column, - replace->get_string (), - replace->get_length ()); + return file.apply_fixit (start.line, start.column, next_loc.column, + hint->get_string (), + hint->get_length ()); } /* Locate the edited_file * for FILENAME, if any @@ -409,37 +334,21 @@ edited_file::get_content () return xstrdup (pp_formatted_text (&pp)); } -/* Attempt to insert the string INSERT_STR with length INSERT_LEN - at LINE and COLUMN, updating the in-memory copy of the line, and - the record of edits to the line. */ - -bool -edited_file::apply_insert (int line, int column, - const char *insert_str, - int insert_len) -{ - edited_line *el = get_or_insert_line (line); - if (!el) - return false; - return el->apply_insert (column, insert_str, insert_len); -} - -/* Attempt to replace columns START_COLUMN through FINISH_COLUMN of LINE - with the string REPLACEMENT_STR of length REPLACEMENT_LEN, +/* Attempt to replace columns START_COLUMN up to but not including NEXT_COLUMN + of LINE with the string REPLACEMENT_STR of length REPLACEMENT_LEN, updating the in-memory copy of the line, and the record of edits to the line. */ bool -edited_file::apply_replace (int line, int start_column, - int finish_column, - const char *replacement_str, - int replacement_len) +edited_file::apply_fixit (int line, int start_column, int next_column, + const char *replacement_str, + int replacement_len) { edited_line *el = get_or_insert_line (line); if (!el) return false; - return el->apply_replace (start_column, finish_column, replacement_str, - replacement_len); + return el->apply_fixit (start_column, next_column, replacement_str, + replacement_len); } /* Given line LINE, map from COLUMN in the input file to its current @@ -698,11 +607,6 @@ edited_line::edited_line (const char *filename, int line_num) edited_line::~edited_line () { free (m_content); - - int i; - line_event *event; - FOR_EACH_VEC_ELT (m_line_events, i, event) - delete event; } /* A callback for deleting edited_line *, for use as a @@ -727,85 +631,41 @@ edited_line::get_effective_column (int orig_column) const return orig_column; } -/* Attempt to insert the string INSERT_STR with length INSERT_LEN at COLUMN - of this line, updating the in-memory copy of the line, and the record - of edits to it. +/* Attempt to replace columns START_COLUMN up to but not including + NEXT_COLUMN of the line with the string REPLACEMENT_STR of + length REPLACEMENT_LEN, updating the in-memory copy of the line, + and the record of edits to the line. Return true if successful; false if an error occurred. */ bool -edited_line::apply_insert (int column, const char *insert_str, - int insert_len) -{ - column = get_effective_column (column); - - int start_offset = column - 1; - gcc_assert (start_offset >= 0); - if (start_offset > m_len) - return false; - - /* Ensure buffer is big enough. */ - size_t new_len = m_len + insert_len; - ensure_capacity (new_len); - - char *suffix = m_content + start_offset; - gcc_assert (suffix <= m_content + m_len); - size_t len_suffix = (m_content + m_len) - suffix; - - /* Move successor content into position. They overlap, so use memmove. */ - memmove (m_content + start_offset + insert_len, - suffix, len_suffix); - - /* Replace target content. They don't overlap, so use memcpy. */ - memcpy (m_content + start_offset, - insert_str, - insert_len); - - m_len = new_len; - - ensure_terminated (); - - /* Record the insertion, so that future changes to the line can have - their column information adjusted accordingly. */ - m_line_events.safe_push (new insert_event (column, insert_len)); - - return true; -} - -/* Attempt to replace columns START_COLUMN through FINISH_COLUMN of the line - with the string REPLACEMENT_STR of length REPLACEMENT_LEN, - updating the in-memory copy of the line, and the record of edits to - the line. - Return true if successful; false if an error occurred. */ - -bool -edited_line::apply_replace (int start_column, - int finish_column, - const char *replacement_str, - int replacement_len) +edited_line::apply_fixit (int start_column, + int next_column, + const char *replacement_str, + int replacement_len) { start_column = get_effective_column (start_column); - finish_column = get_effective_column (finish_column); + next_column = get_effective_column (next_column); int start_offset = start_column - 1; - int end_offset = finish_column - 1; + int next_offset = next_column - 1; gcc_assert (start_offset >= 0); - gcc_assert (end_offset >= 0); + gcc_assert (next_offset >= 0); - if (start_column > finish_column) + if (start_column > next_column) return false; - if (start_offset >= m_len) + if (start_offset >= (m_len + 1)) return false; - if (end_offset >= m_len) + if (next_offset >= (m_len + 1)) return false; - size_t victim_len = end_offset - start_offset + 1; + size_t victim_len = next_offset - start_offset; /* Ensure buffer is big enough. */ size_t new_len = m_len + replacement_len - victim_len; ensure_capacity (new_len); - char *suffix = m_content + end_offset + 1; + char *suffix = m_content + next_offset; gcc_assert (suffix <= m_content + m_len); size_t len_suffix = (m_content + m_len) - suffix; @@ -824,8 +684,8 @@ edited_line::apply_replace (int start_column, /* Record the replacement, so that future changes to the line can have their column information adjusted accordingly. */ - m_line_events.safe_push (new replace_event (start_column, finish_column, - replacement_len)); + m_line_events.safe_push (line_event (start_column, next_column, + replacement_len)); return true; } diff --git a/gcc/edit-context.h b/gcc/edit-context.h index 0d79f07d8bf..402b8b1cffe 100644 --- a/gcc/edit-context.h +++ b/gcc/edit-context.h @@ -56,8 +56,7 @@ class edit_context void print_diff (pretty_printer *pp, bool show_filenames); private: - bool apply_insert (const fixit_insert *insert); - bool apply_replace (const fixit_replace *replace); + bool apply_fixit (const fixit_hint *hint); edited_file *get_file (const char *filename); edited_file &get_or_insert_file (const char *filename); diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 9d69bf7561c..dc80b563f5f 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2017-05-01 David Malcolm + + * gcc.dg/Wmissing-braces-fixits.c: Update expected output to + reflect insertion fix-it hints at the same location now being + consolidated. + 2017-05-01 Martin Sebor * tree-ssa/builtin-sprintf-warn-18.c: Adjust to avoid failures diff --git a/gcc/testsuite/gcc.dg/Wmissing-braces-fixits.c b/gcc/testsuite/gcc.dg/Wmissing-braces-fixits.c index 37998d04adf..b7d66cd6c3d 100644 --- a/gcc/testsuite/gcc.dg/Wmissing-braces-fixits.c +++ b/gcc/testsuite/gcc.dg/Wmissing-braces-fixits.c @@ -48,13 +48,9 @@ int arr_2_2_3[2][2][3] = \ /* { dg-begin-multiline-output "" } { 0, 1, 2, 3, 4, 5, ^ - { - { } { } - } + {{ } { }} 6, 7, 8, 9, 10, 11}; - { - { } { } - } + {{ } { }} { dg-end-multiline-output "" } */ int arr_2_3_2[2][3][2] = \ @@ -63,13 +59,9 @@ int arr_2_3_2[2][3][2] = \ /* { dg-begin-multiline-output "" } { 0, 1, 2, 3, 4, 5, ^ - { - { } { } { } - } + {{ } { } { }} 6, 7, 8, 9, 10, 11}; - { - { } { } { } - } + {{ } { } { }} { dg-end-multiline-output "" } */ int arr_6_2[6][2] = \ @@ -89,15 +81,9 @@ int arr_3_2_2[3][2][2] = \ /* { dg-begin-multiline-output "" } { 0, 1, 2, 3, 4, 5, ^ - { - { } { } - } { - { } + {{ } { }}{{ } 6, 7, 8, 9, 10, 11}; - { } - } { - { } { } - } + { }}{{ } { }} { dg-end-multiline-output "" } */ int arr_3_4[3][4] = \ @@ -128,13 +114,9 @@ int arr_2_1_6[2][1][6] = \ /* { dg-begin-multiline-output "" } { 0, 1, 2, 3, 4, 5, ^ - { - { } - } + {{ }} 6, 7, 8, 9, 10, 11}; - { - { } - } + {{ }} { dg-end-multiline-output "" } */ struct sf2 arr_6_sf2[6] = \ @@ -165,21 +147,9 @@ struct sa2 arr_6_sa2[6] = \ /* { dg-begin-multiline-output "" } { 0, 1, 2, 3, 4, 5, ^ - { - { } - } { - { } - } { - { } - } + {{ }}{{ }}{{ }} 6, 7, 8, 9, 10, 11}; - { - { } - } { - { } - } { - { } - } + {{ }}{{ }}{{ }} { dg-end-multiline-output "" } */ struct sa3 arr_4_sa3[4] = \ @@ -188,15 +158,7 @@ struct sa3 arr_4_sa3[4] = \ /* { dg-begin-multiline-output "" } { 0, 1, 2, 3, 4, 5, ^ - { - { } - } { - { } - } + {{ }}{{ }} 6, 7, 8, 9, 10, 11}; - { - { } - } { - { } - } + {{ }}{{ }} { dg-end-multiline-output "" } */ diff --git a/libcpp/ChangeLog b/libcpp/ChangeLog index 63cd5a52a33..ab225d53519 100644 --- a/libcpp/ChangeLog +++ b/libcpp/ChangeLog @@ -1,3 +1,35 @@ +2017-05-01 David Malcolm + + * include/line-map.h (source_range::intersects_line_p): Delete. + (rich_location::add_fixit): Delete. + (rich_location::maybe_add_fixit): New method. + (class fixit_hint): Reimplement in terms of... + (class fixit_replace): ...this. + (class fixit_insert): Delete. + * line-map.c (linemap_position_for_loc_and_offset): Drop overzealous + linemap_assert_fails. + (source_range::intersects_line_p): Rename to... + (fixit_hint::affects_line_p): New function. + (rich_location::add_fixit_insert_before): Reimplement in terms of + maybe_add_fixit, moving validation there. + (rich_location::add_fixit_insert_after): Likewise. + (column_before_p): Delete. + (rich_location::add_fixit_replace): Reimplement in terms of + maybe_add_fixit, moving validation there. Convert closed input range + to half-open range. + (rich_location::add_fixit): Delete. + (rich_location::maybe_add_fixit): New function. + (fixit_insert::fixit_insert): Delete. + (fixit_insert::~fixit_insert): Delete. + (fixit_insert::affects_line_p): Delete. + (fixit_insert::maybe_append_replace): Delete. + (fixit_replace::fixit_replace): Rename to... + (fixit_hint::fixit_hint): ...this, rewriting as necessary. + (fixit_replace::~fixit_replace): Delete. + (fixit_replace::affects_line_p): Delete. + (fixit_replace::maybe_append_replace): Rename to... + (fixit_hint::maybe_append): ...this, rewriting as necessary. + 2017-04-03 Jonathan Wakely * include/line-map.h (LINEMAPS_MACRO_MAPS): Fix typo in comment. diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h index 522e8bb17eb..0c44f01b3bd 100644 --- a/libcpp/include/line-map.h +++ b/libcpp/include/line-map.h @@ -305,9 +305,6 @@ struct GTY(()) source_range result.m_finish = finish; return result; } - - /* Is there any part of this range on the given line? */ - bool intersects_line_p (const char *file, int line) const; }; /* Memory allocation function typedef. Works like xrealloc. */ @@ -1416,8 +1413,6 @@ semi_embedded_vec::truncate (int len) } class fixit_hint; - class fixit_insert; - class fixit_replace; /* A "rich" source code location, for use when printing diagnostics. A rich_location has one or more carets&ranges, where the carets @@ -1667,7 +1662,9 @@ class rich_location private: bool reject_impossible_fixit (source_location where); void stop_supporting_fixits (); - void add_fixit (fixit_hint *hint); + void maybe_add_fixit (source_location start, + source_location next_loc, + const char *new_content); public: static const int STATICALLY_ALLOCATED_RANGES = 3; @@ -1687,72 +1684,41 @@ protected: bool m_seen_impossible_fixit; }; +/* A fix-it hint: a suggested insertion, replacement, or deletion of text. + We handle these three types of edit with one class, by representing + them as replacement of a half-open range: + [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. */ + class fixit_hint -{ -public: - enum kind {INSERT, REPLACE}; - - virtual ~fixit_hint () {} - - virtual enum kind get_kind () const = 0; - virtual bool affects_line_p (const char *file, int line) const = 0; - virtual source_location get_start_loc () const = 0; - virtual bool maybe_get_end_loc (source_location *out) const = 0; - /* Vfunc for consolidating successor fixits. */ - virtual bool maybe_append_replace (line_maps *set, - source_range src_range, - const char *new_content) = 0; -}; - -class fixit_insert : public fixit_hint { public: - fixit_insert (source_location where, - const char *new_content); - ~fixit_insert (); - enum kind get_kind () const { return INSERT; } - bool affects_line_p (const char *file, int line) const; - source_location get_start_loc () const { return m_where; } - bool maybe_get_end_loc (source_location *) const { return false; } - bool maybe_append_replace (line_maps *set, - source_range src_range, - const char *new_content); + fixit_hint (source_location start, + source_location next_loc, + const char *new_content); + ~fixit_hint () { free (m_bytes); } + + bool affects_line_p (const char *file, int line) const; + source_location get_start_loc () const { return m_start; } + source_location get_next_loc () const { return m_next_loc; } + bool maybe_append (source_location start, + source_location next_loc, + const char *new_content); - source_location get_location () const { return m_where; } const char *get_string () const { return m_bytes; } size_t get_length () const { return m_len; } - private: - source_location m_where; - char *m_bytes; - size_t m_len; -}; - -class fixit_replace : public fixit_hint -{ - public: - fixit_replace (source_range src_range, - const char *new_content); - ~fixit_replace (); - - enum kind get_kind () const { return REPLACE; } - bool affects_line_p (const char *file, int line) const; - source_location get_start_loc () const { return m_src_range.m_start; } - bool maybe_get_end_loc (source_location *out) const - { - *out = m_src_range.m_finish; - return true; - } - bool maybe_append_replace (line_maps *set, - source_range src_range, - const char *new_content); - - source_range get_range () const { return m_src_range; } - const char *get_string () const { return m_bytes; } - size_t get_length () const { return m_len; } + bool insertion_p () const { return m_start == m_next_loc; } private: - source_range m_src_range; + /* We don't use source_range here since, unlike most places, + this is a half-open/half-closed range: + [start, next_loc) + so that we can support insertion via start == next_loc. */ + source_location m_start; + source_location m_next_loc; char *m_bytes; size_t m_len; }; diff --git a/libcpp/line-map.c b/libcpp/line-map.c index 949489eb1a1..176e58d2545 100644 --- a/libcpp/line-map.c +++ b/libcpp/line-map.c @@ -881,8 +881,7 @@ linemap_position_for_loc_and_offset (struct line_maps *set, loc = set->location_adhoc_data_map.data[loc & MAX_SOURCE_LOCATION].locus; /* This function does not support virtual locations yet. */ - if (linemap_assert_fails - (!linemap_location_from_macro_expansion_p (set, loc))) + if (linemap_location_from_macro_expansion_p (set, loc)) return loc; if (column_offset == 0 @@ -2003,28 +2002,6 @@ line_table_dump (FILE *stream, struct line_maps *set, unsigned int num_ordinary, } } -/* struct source_range. */ - -/* Is there any part of this range on the given line? */ - -bool -source_range::intersects_line_p (const char *file, int line) const -{ - expanded_location exploc_start - = linemap_client_expand_location_to_spelling_point (m_start); - if (file != exploc_start.file) - return false; - if (line < exploc_start.line) - return false; - expanded_location exploc_finish - = linemap_client_expand_location_to_spelling_point (m_finish); - if (file != exploc_finish.file) - return false; - if (line > exploc_finish.line) - return false; - return true; -} - /* class rich_location. */ /* Construct a rich_location with location LOC as its initial range. */ @@ -2173,16 +2150,7 @@ rich_location::add_fixit_insert_before (source_location where, const char *new_content) { source_location start = get_range_from_loc (m_line_table, where).m_start; - - if (reject_impossible_fixit (start)) - return; - /* We do not yet support newlines within fix-it hints. */ - if (strchr (new_content, '\n')) - { - stop_supporting_fixits (); - return; - } - add_fixit (new fixit_insert (start, new_content)); + maybe_add_fixit (start, start, new_content); } /* Add a fixit-hint, suggesting insertion of NEW_CONTENT @@ -2202,10 +2170,6 @@ rich_location::add_fixit_insert_after (source_location where, const char *new_content) { source_location finish = get_range_from_loc (m_line_table, where).m_finish; - - if (reject_impossible_fixit (finish)) - return; - source_location next_loc = linemap_position_for_loc_and_offset (m_line_table, finish, 1); @@ -2217,7 +2181,7 @@ rich_location::add_fixit_insert_after (source_location where, return; } - add_fixit (new fixit_insert (next_loc, new_content)); + maybe_add_fixit (next_loc, next_loc, new_content); } /* Methods for adding removal fix-it hints. */ @@ -2250,44 +2214,6 @@ rich_location::add_fixit_remove (source_range src_range) add_fixit_replace (src_range, ""); } -/* Return true iff A is in the column directly before B, on the - same line of the same source file. */ - -static bool -column_before_p (line_maps *set, source_location a, source_location b) -{ - if (IS_ADHOC_LOC (a)) - a = get_location_from_adhoc_loc (set, a); - if (IS_ADHOC_LOC (b)) - b = get_location_from_adhoc_loc (set, b); - - /* They must both be in ordinary maps. */ - const struct line_map *linemap_a = linemap_lookup (set, a); - if (linemap_macro_expansion_map_p (linemap_a)) - return false; - const struct line_map *linemap_b = linemap_lookup (set, b); - if (linemap_macro_expansion_map_p (linemap_b)) - return false; - - /* To be on the same line, they must be in the same ordinary map. */ - if (linemap_a != linemap_b) - return false; - - linenum_type line_a - = SOURCE_LINE (linemap_check_ordinary (linemap_a), a); - linenum_type line_b - = SOURCE_LINE (linemap_check_ordinary (linemap_b), b); - if (line_a != line_b) - return false; - - linenum_type column_a - = SOURCE_COLUMN (linemap_check_ordinary (linemap_a), a); - linenum_type column_b - = SOURCE_COLUMN (linemap_check_ordinary (linemap_b), b); - - return column_b == column_a + 1; -} - /* Add a fixit-hint, suggesting replacement of the content covered by range 0 with NEW_CONTENT. */ @@ -2317,28 +2243,22 @@ void rich_location::add_fixit_replace (source_range src_range, const char *new_content) { - src_range.m_start = get_pure_location (m_line_table, src_range.m_start); - src_range.m_finish = get_pure_location (m_line_table, src_range.m_finish); + source_location start = get_pure_location (m_line_table, src_range.m_start); + source_location finish = get_pure_location (m_line_table, src_range.m_finish); - if (reject_impossible_fixit (src_range.m_start)) - return; - if (reject_impossible_fixit (src_range.m_finish)) - return; - - /* We do not yet support newlines within fix-it hints. */ - if (strchr (new_content, '\n')) + /* Fix-it hints use half-closed ranges, so attempt to offset the endpoint. */ + source_location next_loc + = linemap_position_for_loc_and_offset (m_line_table, finish, 1); + /* linemap_position_for_loc_and_offset can fail, if so, it returns + its input value. */ + if (next_loc == finish) { stop_supporting_fixits (); return; } + finish = next_loc; - /* Consolidate neighboring fixits. */ - fixit_hint *prev = get_last_fixit_hint (); - if (prev) - if (prev->maybe_append_replace (m_line_table, src_range, new_content)) - return; - - add_fixit (new fixit_replace (src_range, new_content)); + maybe_add_fixit (start, finish, new_content); } /* Get the last fix-it hint within this rich_location, or NULL if none. */ @@ -2392,89 +2312,85 @@ rich_location::stop_supporting_fixits () m_fixit_hints.truncate (0); } -/* Add HINT to the fix-it hints in this rich_location. */ +/* Add HINT to the fix-it hints in this rich_location, + consolidating into the prior fixit if possible. */ void -rich_location::add_fixit (fixit_hint *hint) +rich_location::maybe_add_fixit (source_location start, + source_location next_loc, + const char *new_content) { - m_fixit_hints.push (hint); + if (reject_impossible_fixit (start)) + return; + if (reject_impossible_fixit (next_loc)) + return; + + /* We do not yet support newlines within fix-it hints. */ + if (strchr (new_content, '\n')) + { + stop_supporting_fixits (); + return; + } + + /* Consolidate neighboring fixits. */ + fixit_hint *prev = get_last_fixit_hint (); + if (prev) + if (prev->maybe_append (start, next_loc, new_content)) + return; + + m_fixit_hints.push (new fixit_hint (start, next_loc, new_content)); } -/* class fixit_insert. */ +/* class fixit_hint. */ -fixit_insert::fixit_insert (source_location where, - const char *new_content) -: m_where (where), +fixit_hint::fixit_hint (source_location start, + source_location next_loc, + const char *new_content) +: m_start (start), + m_next_loc (next_loc), m_bytes (xstrdup (new_content)), m_len (strlen (new_content)) { } -fixit_insert::~fixit_insert () -{ - free (m_bytes); -} - -/* Implementation of fixit_hint::affects_line_p for fixit_insert. */ +/* Does this fix-it hint affect the given line? */ bool -fixit_insert::affects_line_p (const char *file, int line) const +fixit_hint::affects_line_p (const char *file, int line) const { - expanded_location exploc - = linemap_client_expand_location_to_spelling_point (m_where); - if (file == exploc.file) - if (line == exploc.line) - return true; - return false; + expanded_location exploc_start + = linemap_client_expand_location_to_spelling_point (m_start); + if (file != exploc_start.file) + return false; + if (line < exploc_start.line) + return false; + expanded_location exploc_next_loc + = linemap_client_expand_location_to_spelling_point (m_next_loc); + if (file != exploc_next_loc.file) + return false; + if (line > exploc_next_loc.line) + return false; + return true; } -/* Implementation of maybe_append_replace for fixit_insert. Reject - the attempt to consolidate fix-its. */ - -bool -fixit_insert::maybe_append_replace (line_maps *, source_range, const char *) -{ - return false; -} - -/* class fixit_replace. */ - -fixit_replace::fixit_replace (source_range src_range, - const char *new_content) -: m_src_range (src_range), - m_bytes (xstrdup (new_content)), - m_len (strlen (new_content)) -{ -} - -fixit_replace::~fixit_replace () -{ - free (m_bytes); -} - -/* Implementation of fixit_hint::affects_line_p for fixit_replace. */ - -bool -fixit_replace::affects_line_p (const char *file, int line) const -{ - return m_src_range.intersects_line_p (file, line); -} - -/* Implementation of maybe_append_replace for fixit_replace. If - possible, merge the new replacement into this one and return true. +/* Method for consolidating fix-it hints, for use by + rich_location::maybe_add_fixit. + If possible, merge a pending fix-it hint with the given params + into this one and return true. Otherwise return false. */ bool -fixit_replace::maybe_append_replace (line_maps *set, - source_range src_range, - const char *new_content) +fixit_hint::maybe_append (source_location start, + source_location next_loc, + const char *new_content) { - /* Does SRC_RANGE start immediately after this one finishes? */ - if (!column_before_p (set, m_src_range.m_finish, src_range.m_start)) + /* For consolidation to be possible, START must be at this hint's + m_next_loc. */ + if (start != m_next_loc) return false; - /* We have neighboring replacements; merge them. */ - m_src_range.m_finish = src_range.m_finish; + /* If so, we have neighboring replacements; merge them. */ + m_next_loc = next_loc; size_t extra_len = strlen (new_content); m_bytes = (char *)xrealloc (m_bytes, m_len + extra_len + 1); memcpy (m_bytes + m_len, new_content, extra_len);