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

+2018-04-30  David Malcolm  <dmalcolm@redhat.com>
+
+	* 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  <jh@suse.cz>
 
 	* lto-wrapper.c (ltrans_priorities): New static var.

gcc/c-family/ChangeLog

+2018-04-30  David Malcolm  <dmalcolm@redhat.com>
+
+	* 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  <dmalcolm@redhat.com>
 
 	PR c++/84269

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

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];
 

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);

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 ();
 }
 

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);
 }
 

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.  */

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);

gcc/testsuite/ChangeLog

+2018-04-30  David Malcolm  <dmalcolm@redhat.com>
+
+	* 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  <claziss@synopsys.com>
 
 	* gcc.target/arc/interrupt-8.c: Update test.

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);