From 7761dfbee17cb7a4bb3539a381bec63d31af7c28 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Mon, 30 Apr 2018 15:01:56 +0000 Subject: [PATCH] Use char_span for return type of location_get_source_line location_get_source_line returns a const char * that isn't 0-terminated, writing back a length through an int * param. This is error-prone, as all call-sites have to take into account the lack of 0-termination, and respect the length of the buffer. It's cleaner to bundle together this pointer+length state into a class, so this patch does so, reusing the "char_span" class that I introduced in r250187 (as part of the fix for PR c/81405). The patch also adds assertions to all access to the char_span. gcc/c-family/ChangeLog: * c-format.c (get_corrected_substring): Update for location_get_source_line returning a char_span. Use a char_span when handling the prefix of the correction. * c-indentation.c (get_visual_column): Update for location_get_source_line returning a char_span. (get_first_nws_vis_column): Likewise. gcc/ChangeLog: * diagnostic-show-locus.c (layout::layout): Update for location_get_source_line returning a char_span. (struct char_span): Move to input.h. (struct correction): Update for fields in char_span becoming private. (struct source_line): Update for location_get_source_line returning a char_span. (layout::print_line): Likewise. * edit-context.c (edited_file::print_content): Likewise. (edited_file::print_diff_hunk): Likewise. (edited_file::print_run_of_changed_lines): Likewise. (edited_file::get_num_lines): Likewise. (edited_line::edited_line): Likewise. * final.c (asm_show_source): Likewise. * input.c (location_get_source_line): Convert return type from const char * to char_span, losing the final "line_len" param. (dump_location_info): Update for the above. (get_substring_ranges_for_loc): Likewise. Use a char_span when handling the literal within the line. (test_reading_source_line): Update for location_get_source_line returning a char_span. * input.h (class char_span): Move here from diagnostic-show-locus.c, converting from a struct to a class. Make data members private. (char_span::operator bool): New. (char_span::length): New. (char_span::get_buffer): New. (char_span::operator[]): New. (char_span::subspan): Make const. (char_span::xstrdup): New. (location_get_source_line): Convert return type from const char * to char_span, losing the final "line_size" param. gcc/testsuite/ChangeLog: * gcc.dg/plugin/diagnostic_plugin_test_show_locus.c (test_show_locus): Update for location_get_source_line returning a char_span. Use char_span for handling words in the "test_many_nested_locations" fix-it example. From-SVN: r259768 --- gcc/ChangeLog | 36 +++++++++ gcc/c-family/ChangeLog | 9 +++ gcc/c-family/c-format.c | 10 +-- gcc/c-family/c-indentation.c | 9 +-- gcc/diagnostic-show-locus.c | 52 ++++--------- gcc/edit-context.c | 31 +++----- gcc/final.c | 7 +- gcc/input.c | 76 +++++++++---------- gcc/input.h | 48 +++++++++++- gcc/testsuite/ChangeLog | 7 ++ .../diagnostic_plugin_test_show_locus.c | 14 ++-- 11 files changed, 176 insertions(+), 123 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index a3ea6420dfa..74ac8b441f1 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,39 @@ +2018-04-30 David Malcolm + + * diagnostic-show-locus.c (layout::layout): Update for + location_get_source_line returning a char_span. + (struct char_span): Move to input.h. + (struct correction): Update for fields in char_span becoming + private. + (struct source_line): Update for location_get_source_line + returning a char_span. + (layout::print_line): Likewise. + * edit-context.c (edited_file::print_content): Likewise. + (edited_file::print_diff_hunk): Likewise. + (edited_file::print_run_of_changed_lines): Likewise. + (edited_file::get_num_lines): Likewise. + (edited_line::edited_line): Likewise. + * final.c (asm_show_source): Likewise. + * input.c (location_get_source_line): Convert return type + from const char * to char_span, losing the final "line_len" + param. + (dump_location_info): Update for the above. + (get_substring_ranges_for_loc): Likewise. Use a char_span + when handling the literal within the line. + (test_reading_source_line): Update for location_get_source_line + returning a char_span. + * input.h (class char_span): Move here from + diagnostic-show-locus.c, converting from a struct to a class. + Make data members private. + (char_span::operator bool): New. + (char_span::length): New. + (char_span::get_buffer): New. + (char_span::operator[]): New. + (char_span::subspan): Make const. + (char_span::xstrdup): New. + (location_get_source_line): Convert return type from const char * + to char_span, losing the final "line_size" param. + 2018-04-30 Jan Hubicka * lto-wrapper.c (ltrans_priorities): New static var. diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog index 27245b7e60d..ab85a73bd1b 100644 --- a/gcc/c-family/ChangeLog +++ b/gcc/c-family/ChangeLog @@ -1,3 +1,12 @@ +2018-04-30 David Malcolm + + * c-format.c (get_corrected_substring): Update for + location_get_source_line returning a char_span. Use a char_span + when handling the prefix of the correction. + * c-indentation.c (get_visual_column): Update for + location_get_source_line returning a char_span. + (get_first_nws_vis_column): Likewise. + 2018-03-29 David Malcolm PR c++/84269 diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c index 3f4f83af219..ee7c33d97f2 100644 --- a/gcc/c-family/c-format.c +++ b/gcc/c-family/c-format.c @@ -3499,10 +3499,8 @@ get_corrected_substring (const substring_loc &fmt_loc, if (caret.column > finish.column) return NULL; - int line_width; - const char *line = location_get_source_line (start.file, start.line, - &line_width); - if (line == NULL) + char_span line = location_get_source_line (start.file, start.line); + if (!line) return NULL; /* If we got this far, then we have the line containing the @@ -3511,9 +3509,9 @@ get_corrected_substring (const substring_loc &fmt_loc, Generate a trimmed copy, containing the prefix part of the conversion specification, up to the (but not including) the length modifier. In the above example, this would be "%-+*.*". */ - const char *current_content = line + start.column - 1; int length_up_to_type = caret.column - start.column; - char *prefix = xstrndup (current_content, length_up_to_type); + char_span prefix_span = line.subspan (start.column - 1, length_up_to_type); + char *prefix = prefix_span.xstrdup (); /* Now attempt to generate a suggestion for the rest of the specification (length modifier and conversion char), based on ARG_TYPE and diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c index acca44472e8..44b1e1e361f 100644 --- a/gcc/c-family/c-indentation.c +++ b/gcc/c-family/c-indentation.c @@ -70,9 +70,7 @@ get_visual_column (expanded_location exploc, location_t loc, return false; } - int line_len; - const char *line = location_get_source_line (exploc.file, exploc.line, - &line_len); + char_span line = location_get_source_line (exploc.file, exploc.line); if (!line) return false; unsigned int vis_column = 0; @@ -112,12 +110,11 @@ get_first_nws_vis_column (const char *file, int line_num, { gcc_assert (first_nws); - int line_len; - const char *line = location_get_source_line (file, line_num, &line_len); + char_span line = location_get_source_line (file, line_num); if (!line) return false; unsigned int vis_column = 0; - for (int i = 1; i < line_len; i++) + for (size_t i = 1; i < line.length (); i++) { unsigned char ch = line[i - 1]; diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c index bdf608a08bc..f188ee9938b 100644 --- a/gcc/diagnostic-show-locus.c +++ b/gcc/diagnostic-show-locus.c @@ -857,17 +857,15 @@ layout::layout (diagnostic_context * context, /* Adjust m_x_offset. Center the primary caret to fit in max_width; all columns will be adjusted accordingly. */ - int max_width = m_context->caret_max_width; - int line_width; - const char *line = location_get_source_line (m_exploc.file, m_exploc.line, - &line_width); - if (line && m_exploc.column <= line_width) + size_t max_width = m_context->caret_max_width; + char_span line = location_get_source_line (m_exploc.file, m_exploc.line); + if (line && (size_t)m_exploc.column <= line.length ()) { - int right_margin = CARET_LINE_MARGIN; - int column = m_exploc.column; - right_margin = MIN (line_width - column, right_margin); + size_t right_margin = CARET_LINE_MARGIN; + size_t column = m_exploc.column; + right_margin = MIN (line.length () - column, right_margin); right_margin = max_width - right_margin; - if (line_width >= max_width && column > right_margin) + if (line.length () >= max_width && column > right_margin) m_x_offset = column - right_margin; gcc_assert (m_x_offset >= 0); } @@ -1483,26 +1481,6 @@ get_printed_columns (const fixit_hint *hint) } } -/* A struct capturing the bounds of a buffer, to allow for run-time - bounds-checking in a checked build. */ - -struct char_span -{ - char_span (const char *ptr, size_t n_elts) : m_ptr (ptr), m_n_elts (n_elts) {} - - char_span subspan (int offset, int n_elts) - { - gcc_assert (offset >= 0); - gcc_assert (offset < (int)m_n_elts); - gcc_assert (n_elts >= 0); - gcc_assert (offset + n_elts <= (int)m_n_elts); - return char_span (m_ptr + offset, n_elts); - } - - const char *m_ptr; - size_t m_n_elts; -}; - /* A correction on a particular line. This describes a plan for how to print one or more fixit_hint instances that affected the line, potentially consolidating hints @@ -1534,9 +1512,9 @@ struct correction void overwrite (int dst_offset, const char_span &src_span) { gcc_assert (dst_offset >= 0); - gcc_assert (dst_offset + src_span.m_n_elts < m_alloc_sz); - memcpy (m_text + dst_offset, src_span.m_ptr, - src_span.m_n_elts); + gcc_assert (dst_offset + src_span.length () < m_alloc_sz); + memcpy (m_text + dst_offset, src_span.get_buffer (), + src_span.length ()); } /* If insert, then start: the column before which the text @@ -1627,7 +1605,9 @@ struct source_line source_line::source_line (const char *filename, int line) { - chars = location_get_source_line (filename, line, &width); + char_span span = location_get_source_line (filename, line); + chars = span.get_buffer (); + width = span.length (); } /* Add HINT to the corrections for this line. @@ -1935,15 +1915,13 @@ layout::show_ruler (int max_column) const void layout::print_line (linenum_type row) { - int line_width; - const char *line = location_get_source_line (m_exploc.file, row, - &line_width); + char_span line = location_get_source_line (m_exploc.file, row); if (!line) return; line_bounds lbounds; print_leading_fixits (row); - print_source_line (row, line, line_width, &lbounds); + print_source_line (row, line.get_buffer (), line.length (), &lbounds); if (should_print_annotation_line_p (row)) print_annotation_line (row, lbounds); print_trailing_fixits (row); diff --git a/gcc/edit-context.c b/gcc/edit-context.c index 9ac2dfc451c..3cdb88d565c 100644 --- a/gcc/edit-context.c +++ b/gcc/edit-context.c @@ -422,12 +422,10 @@ edited_file::print_content (pretty_printer *pp) el->print_content (pp); else { - int len; - const char *line - = location_get_source_line (m_filename, line_num, &len); + char_span line = location_get_source_line (m_filename, line_num); if (!line) return false; - for (int i = 0; i < len; i++) + for (size_t i = 0; i < line.length (); i++) pp_character (pp, line[i]); } if (line_num < line_count) @@ -543,10 +541,8 @@ edited_file::print_diff_hunk (pretty_printer *pp, int old_start_of_hunk, else { /* Unchanged line. */ - 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); + char_span old_line = location_get_source_line (m_filename, line_num); + print_diff_line (pp, ' ', old_line.get_buffer (), old_line.length ()); line_num++; } } @@ -574,10 +570,9 @@ edited_file::print_run_of_changed_lines (pretty_printer *pp, 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); + char_span old_line = location_get_source_line (m_filename, line_num); + print_diff_line (pp, '-', old_line.get_buffer (), + old_line.length ()); } } pp_string (pp, colorize_stop (pp_show_color (pp))); @@ -671,10 +666,8 @@ edited_file::get_num_lines (bool *missing_trailing_newline) m_num_lines = 0; while (true) { - int line_size; - const char *line - = location_get_source_line (m_filename, m_num_lines + 1, - &line_size); + char_span line + = location_get_source_line (m_filename, m_num_lines + 1); if (line) m_num_lines++; else @@ -695,12 +688,12 @@ edited_line::edited_line (const char *filename, int line_num) m_line_events (), m_predecessors () { - const char *line = location_get_source_line (filename, line_num, - &m_len); + char_span line = location_get_source_line (filename, line_num); if (!line) return; + m_len = line.length (); ensure_capacity (m_len); - memcpy (m_content, line, m_len); + memcpy (m_content, line.get_buffer (), m_len); ensure_terminated (); } diff --git a/gcc/final.c b/gcc/final.c index 19817e240da..4c600f0edf2 100644 --- a/gcc/final.c +++ b/gcc/final.c @@ -2216,14 +2216,13 @@ asm_show_source (const char *filename, int linenum) if (!filename) return; - int line_size; - const char *line = location_get_source_line (filename, linenum, &line_size); + char_span line = location_get_source_line (filename, linenum); if (!line) return; fprintf (asm_out_file, "%s %s:%i: ", ASM_COMMENT_START, filename, linenum); - /* "line" is not 0-terminated, so we must use line_size. */ - fwrite (line, 1, line_size, asm_out_file); + /* "line" is not 0-terminated, so we must use its length. */ + fwrite (line.get_buffer (), 1, line.length (), asm_out_file); fputc ('\n', asm_out_file); } diff --git a/gcc/input.c b/gcc/input.c index b6675768722..d65a82dc26e 100644 --- a/gcc/input.c +++ b/gcc/input.c @@ -741,29 +741,27 @@ read_line_num (fcache *c, size_t line_num, The line is not nul-terminated. The returned pointer is only valid until the next call of location_get_source_line. Note that the line can contain several null characters, - so LINE_LEN, if non-null, points to the actual length of the line. - If the function fails, NULL is returned. */ + so the returned value's length has the actual length of the line. + If the function fails, a NULL char_span is returned. */ -const char * -location_get_source_line (const char *file_path, int line, - int *line_len) +char_span +location_get_source_line (const char *file_path, int line) { char *buffer = NULL; ssize_t len; if (line == 0) - return NULL; + return char_span (NULL, 0); fcache *c = lookup_or_add_file_to_cache_tab (file_path); if (c == NULL) - return NULL; + return char_span (NULL, 0); bool read = read_line_num (c, line, &buffer, &len); + if (!read) + return char_span (NULL, 0); - if (read && line_len) - *line_len = len; - - return read ? buffer : NULL; + return char_span (buffer, len); } /* Determine if FILE_PATH missing a trailing newline on its final line. @@ -1121,25 +1119,23 @@ dump_location_info (FILE *stream) { /* Beginning of a new source line: draw the line. */ - int line_size; - const char *line_text = location_get_source_line (exploc.file, - exploc.line, - &line_size); + char_span line_text = location_get_source_line (exploc.file, + exploc.line); if (!line_text) break; fprintf (stream, "%s:%3i|loc:%5i|%.*s\n", exploc.file, exploc.line, loc, - line_size, line_text); + (int)line_text.length (), line_text.get_buffer ()); /* "loc" is at column 0, which means "the whole line". Render the locations *within* the line, by underlining it, showing the source_location numeric values at each column. */ - int max_col = (1 << map->m_column_and_range_bits) - 1; - if (max_col > line_size) - max_col = line_size + 1; + size_t max_col = (1 << map->m_column_and_range_bits) - 1; + if (max_col > line_text.length ()) + max_col = line_text.length () + 1; int indent = 14 + strlen (exploc.file); @@ -1426,28 +1422,27 @@ get_substring_ranges_for_loc (cpp_reader *pfile, if (start.column > finish.column) return "range endpoints are reversed"; - int line_width; - const char *line = location_get_source_line (start.file, start.line, - &line_width); - if (line == NULL) + char_span line = location_get_source_line (start.file, start.line); + if (!line) return "unable to read source line"; /* Determine the location of the literal (including quotes and leading prefix chars, such as the 'u' in a u"" token). */ - const char *literal = line + start.column - 1; - int literal_length = finish.column - start.column + 1; + size_t literal_length = finish.column - start.column + 1; /* Ensure that we don't crash if we got the wrong location. */ - if (line_width < (start.column - 1 + literal_length)) + if (line.length () < (start.column - 1 + literal_length)) return "line is not wide enough"; + char_span literal = line.subspan (start.column - 1, literal_length); + cpp_string from; from.len = literal_length; /* Make a copy of the literal, to avoid having to rely on the lifetime of the copy of the line within the cache. This will be released by the auto_cpp_string_vec dtor. */ - from.text = XDUPVEC (unsigned char, literal, literal_length); + from.text = (unsigned char *)literal.xstrdup (); strs.safe_push (from); /* For very long lines, a new linemap could have started @@ -1908,24 +1903,23 @@ test_reading_source_line () "This is the 3rd line"); /* Read back a specific line from the tempfile. */ - int line_size; - const char *source_line = location_get_source_line (tmp.get_filename (), - 3, &line_size); - ASSERT_TRUE (source_line != NULL); - ASSERT_EQ (20, line_size); + char_span source_line = location_get_source_line (tmp.get_filename (), 3); + ASSERT_TRUE (source_line); + ASSERT_TRUE (source_line.get_buffer () != NULL); + ASSERT_EQ (20, source_line.length ()); ASSERT_TRUE (!strncmp ("This is the 3rd line", - source_line, line_size)); + source_line.get_buffer (), source_line.length ())); - source_line = location_get_source_line (tmp.get_filename (), - 2, &line_size); - ASSERT_TRUE (source_line != NULL); - ASSERT_EQ (21, line_size); + source_line = location_get_source_line (tmp.get_filename (), 2); + ASSERT_TRUE (source_line); + ASSERT_TRUE (source_line.get_buffer () != NULL); + ASSERT_EQ (21, source_line.length ()); ASSERT_TRUE (!strncmp ("This is the test text", - source_line, line_size)); + source_line.get_buffer (), source_line.length ())); - source_line = location_get_source_line (tmp.get_filename (), - 4, &line_size); - ASSERT_TRUE (source_line == NULL); + source_line = location_get_source_line (tmp.get_filename (), 4); + ASSERT_FALSE (source_line); + ASSERT_TRUE (source_line.get_buffer () == NULL); } /* Tests of lexing. */ diff --git a/gcc/input.h b/gcc/input.h index cec922f9c8d..4619663519a 100644 --- a/gcc/input.h +++ b/gcc/input.h @@ -38,8 +38,52 @@ STATIC_ASSERT (BUILTINS_LOCATION < RESERVED_LOCATION_COUNT); extern bool is_location_from_builtin_token (source_location); extern expanded_location expand_location (source_location); -extern const char *location_get_source_line (const char *file_path, int line, - int *line_size); + +/* A class capturing the bounds of a buffer, to allow for run-time + bounds-checking in a checked build. */ + +class char_span +{ + public: + char_span (const char *ptr, size_t n_elts) : m_ptr (ptr), m_n_elts (n_elts) {} + + /* Test for a non-NULL pointer. */ + operator bool() const { return m_ptr; } + + /* Get length, not including any 0-terminator (which may not be, + in fact, present). */ + size_t length () const { return m_n_elts; } + + const char *get_buffer () const { return m_ptr; } + + char operator[] (int idx) const + { + gcc_assert (idx >= 0); + gcc_assert ((size_t)idx < m_n_elts); + return m_ptr[idx]; + } + + char_span subspan (int offset, int n_elts) const + { + gcc_assert (offset >= 0); + gcc_assert (offset < (int)m_n_elts); + gcc_assert (n_elts >= 0); + gcc_assert (offset + n_elts <= (int)m_n_elts); + return char_span (m_ptr + offset, n_elts); + } + + char *xstrdup () const + { + return ::xstrndup (m_ptr, m_n_elts); + } + + private: + const char *m_ptr; + size_t m_n_elts; +}; + +extern char_span location_get_source_line (const char *file_path, int line); + extern bool location_missing_trailing_newline (const char *file_path); extern expanded_location expand_location_to_spelling_point (source_location); extern source_location expansion_point_location_if_in_system_header (source_location); diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index a274532f815..557ac8db20f 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2018-04-30 David Malcolm + + * gcc.dg/plugin/diagnostic_plugin_test_show_locus.c + (test_show_locus): Update for location_get_source_line returning a + char_span. Use char_span for handling words in the + "test_many_nested_locations" fix-it example. + 2018-04-30 Claudiu Zissulescu * gcc.target/arc/interrupt-8.c: Update test. 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 3908b922b26..dabc0e42130 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 @@ -377,19 +377,17 @@ test_show_locus (function *fun) rich_location richloc (line_table, loc); for (int line = start_line; line <= finish_line; line++) { - int line_size; - const char *content = location_get_source_line (file, line, - &line_size); + char_span content = location_get_source_line (file, line); gcc_assert (content); /* Split line up into words. */ - for (int idx = 0; idx < line_size; idx++) + for (int idx = 0; idx < content.length (); idx++) { if (ISALPHA (content[idx])) { int start_idx = idx; - while (idx < line_size && ISALPHA (content[idx])) + while (idx < content.length () && ISALPHA (content[idx])) idx++; - if (idx == line_size || !ISALPHA (content[idx])) + if (idx == content.length () || !ISALPHA (content[idx])) { location_t start_of_word = get_loc (line, start_idx); location_t end_of_word = get_loc (line, idx - 1); @@ -399,8 +397,8 @@ test_show_locus (function *fun) richloc.add_range (word, true); /* Add a fixit, converting to upper case. */ - char *copy = xstrndup (content + start_idx, - idx - start_idx); + char_span word_span = content.subspan (start_idx, idx - start_idx); + char *copy = word_span.xstrdup (); for (char *ch = copy; *ch; ch++) *ch = TOUPPER (*ch); richloc.add_fixit_replace (word, copy); -- 2.30.2