[PATCH 1/2] gdb/linespec.c: simplify condition

[PATCH 1/2] gdb/linespec.c: simplify condition

[Simon Marchi]

We can remove the empty check: if the vector has size 1, it is obviously
not empty.  This code ended up like this because the empty check used to
be a NULL check.

Change-Id: I1571bd0228818ca93f6a6b444e9b010dc2da4c08
---
 gdb/linespec.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 56bfaede9d9a..44134665ac7c 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2085,8 +2085,7 @@ canonicalize_linespec (struct linespec_state *state, const linespec *ls)
       if (explicit_loc->function_name == NULL)
 	{
 	  /* No function was specified, so add the symbol name.  */
-	  gdb_assert (!ls->labels.function_symbols->empty ()
-		      && (ls->labels.function_symbols->size () == 1));
+	  gdb_assert (ls->labels.function_symbols->size () == 1);
 	  block_symbol s = ls->labels.function_symbols->front ();
 	  explicit_loc->function_name = xstrdup (s.symbol->natural_name ());
 	}
-- 
2.33.1

[PATCH 2/2] gdb: make struct linespect contain vectors, not pointers to vectors

[Simon Marchi]

struct linespec contains pointers to vectors, instead of containing
vectors directly.  This is probably historical, when linespec_parser
(which contains a struct linespec field) was not C++-ified yet.  But it
seems easy to change the pointers to vectors to just vectors today.
This simplifies the code, we don't need to manually allocate and delete
the vectors and there's no pointer that can be NULL.

As far as I understand, there was not meaningful distinction between a
NULL pointer to vector and an empty vector.  So all NULL checks are
changed for !empty checks.

Change-Id: Ie759707da14d9d984169b93233343a86e2de9ee6
---
 gdb/linespec.c | 222 +++++++++++++++++++++++--------------------------
 1 file changed, 105 insertions(+), 117 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 44134665ac7c..0d3820b97853 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -95,26 +95,27 @@ struct address_entry
 struct linespec
 {
   /* An explicit location describing the SaLs.  */
-  struct explicit_location explicit_loc;
+  struct explicit_location explicit_loc {};
 
-  /* The list of symtabs to search to which to limit the search.  May not
-     be NULL.  If explicit.SOURCE_FILENAME is NULL (no user-specified
-     filename), FILE_SYMTABS should contain one single NULL member.  This
-     will cause the code to use the default symtab.  */
-  std::vector<symtab *> *file_symtabs;
+  /* The list of symtabs to search to which to limit the search.
+
+     If explicit.SOURCE_FILENAME is NULL (no user-specified filename),
+     FILE_SYMTABS should contain one single NULL member.  This will cause the
+     code to use the default symtab.  */
+  std::vector<symtab *> file_symtabs;
 
   /* A list of matching function symbols and minimal symbols.  Both lists
-     may be NULL (or empty) if no matching symbols were found.  */
-  std::vector<block_symbol> *function_symbols;
-  std::vector<bound_minimal_symbol> *minimal_symbols;
+     may be empty if no matching symbols were found.  */
+  std::vector<block_symbol> function_symbols;
+  std::vector<bound_minimal_symbol> minimal_symbols;
 
   /* A structure of matching label symbols and the corresponding
-     function symbol in which the label was found.  Both may be NULL
-     or both must be non-NULL.  */
+     function symbol in which the label was found.  Both may be empty
+     or both must be non-empty.  */
   struct
   {
-    std::vector<block_symbol> *label_symbols;
-    std::vector<block_symbol> *function_symbols;
+    std::vector<block_symbol> label_symbols;
+    std::vector<block_symbol> function_symbols;
   } labels;
 };
 
@@ -185,7 +186,7 @@ struct collect_info
   struct linespec_state *state;
 
   /* A list of symtabs to which to restrict matches.  */
-  std::vector<symtab *> *file_symtabs;
+  const std::vector<symtab *> *file_symtabs;
 
   /* The result being accumulated.  */
   struct
@@ -315,7 +316,7 @@ struct linespec_parser
 #define PARSER_STATE(PPTR) (&(PPTR)->state)
 
   /* The result of the parse.  */
-  struct linespec result {};
+  linespec result;
 #define PARSER_RESULT(PPTR) (&(PPTR)->result)
 
   /* What the parser believes the current word point should complete
@@ -365,13 +366,14 @@ static std::vector<symtab_and_line> decode_objc (struct linespec_state *self,
 static std::vector<symtab *> symtabs_from_filename
   (const char *, struct program_space *pspace);
 
-static std::vector<block_symbol> *find_label_symbols
-  (struct linespec_state *self, std::vector<block_symbol> *function_symbols,
-   std::vector<block_symbol> *label_funcs_ret, const char *name,
-   bool completion_mode = false);
+static std::vector<block_symbol> find_label_symbols
+  (struct linespec_state *self,
+   const std::vector<block_symbol> &function_symbols,
+   std::vector<block_symbol> *label_funcs_ret,
+   const char *name, bool completion_mode = false);
 
 static void find_linespec_symbols (struct linespec_state *self,
-				   std::vector<symtab *> *file_symtabs,
+				   const std::vector<symtab *> &file_symtabs,
 				   const char *name,
 				   symbol_name_match_type name_match_type,
 				   std::vector<block_symbol> *symbols,
@@ -1787,9 +1789,6 @@ linespec_parse_basic (linespec_parser *parser)
 {
   gdb::unique_xmalloc_ptr<char> name;
   linespec_token token;
-  std::vector<block_symbol> symbols;
-  std::vector<block_symbol> *labels;
-  std::vector<bound_minimal_symbol> minimal_symbols;
 
   /* Get the next token.  */
   token = linespec_lexer_lex_one (parser);
@@ -1886,6 +1885,9 @@ linespec_parse_basic (linespec_parser *parser)
     }
   else
     {
+      std::vector<block_symbol> symbols;
+      std::vector<bound_minimal_symbol> minimal_symbols;
+
       /* Try looking it up as a function/method.  */
       find_linespec_symbols (PARSER_STATE (parser),
 			     PARSER_RESULT (parser)->file_symtabs, name.get (),
@@ -1894,24 +1896,23 @@ linespec_parse_basic (linespec_parser *parser)
 
       if (!symbols.empty () || !minimal_symbols.empty ())
 	{
-	  PARSER_RESULT (parser)->function_symbols
-	    = new std::vector<block_symbol> (std::move (symbols));
-	  PARSER_RESULT (parser)->minimal_symbols
-	    = new std::vector<bound_minimal_symbol>
-		(std::move (minimal_symbols));
+	  PARSER_RESULT (parser)->function_symbols = std::move (symbols);
+	  PARSER_RESULT (parser)->minimal_symbols = std::move (minimal_symbols);
 	  PARSER_EXPLICIT (parser)->function_name = name.release ();
 	}
       else
 	{
 	  /* NAME was not a function or a method.  So it must be a label
 	     name or user specified variable like "break foo.c:$zippo".  */
-	  labels = find_label_symbols (PARSER_STATE (parser), NULL,
-				       &symbols, name.get ());
-	  if (labels != NULL)
+	  std::vector<block_symbol> labels
+	    = find_label_symbols (PARSER_STATE (parser), {}, &symbols,
+				  name.get ());
+
+	  if (!labels.empty ())
 	    {
-	      PARSER_RESULT (parser)->labels.label_symbols = labels;
+	      PARSER_RESULT (parser)->labels.label_symbols = std::move (labels);
 	      PARSER_RESULT (parser)->labels.function_symbols
-		= new std::vector<block_symbol> (std::move (symbols));
+		  = std::move (symbols);
 	      PARSER_EXPLICIT (parser)->label_name = name.release ();
 	    }
 	  else if (token.type == LSTOKEN_STRING
@@ -2006,18 +2007,21 @@ linespec_parse_basic (linespec_parser *parser)
 	    }
 	  else
 	    {
+	      std::vector<block_symbol> symbols;
+
 	      /* Grab a copy of the label's name and look it up.  */
 	      name = copy_token_string (token);
-	      labels
+	      std::vector<block_symbol> labels
 		= find_label_symbols (PARSER_STATE (parser),
 				      PARSER_RESULT (parser)->function_symbols,
 				      &symbols, name.get ());
 
-	      if (labels != NULL)
+	      if (!labels.empty ())
 		{
-		  PARSER_RESULT (parser)->labels.label_symbols = labels;
+		  PARSER_RESULT (parser)->labels.label_symbols
+		    = std::move (labels);
 		  PARSER_RESULT (parser)->labels.function_symbols
-		    = new std::vector<block_symbol> (std::move (symbols));
+		    = std::move (symbols);
 		  PARSER_EXPLICIT (parser)->label_name = name.release ();
 		}
 	      else
@@ -2085,8 +2089,8 @@ canonicalize_linespec (struct linespec_state *state, const linespec *ls)
       if (explicit_loc->function_name == NULL)
 	{
 	  /* No function was specified, so add the symbol name.  */
-	  gdb_assert (ls->labels.function_symbols->size () == 1);
-	  block_symbol s = ls->labels.function_symbols->front ();
+	  gdb_assert (ls->labels.function_symbols.size () == 1);
+	  block_symbol s = ls->labels.function_symbols.front ();
 	  explicit_loc->function_name = xstrdup (s.symbol->natural_name ());
 	}
     }
@@ -2116,15 +2120,15 @@ create_sals_line_offset (struct linespec_state *self,
      set_default_source_symtab_and_line uses
      select_source_symtab that calls us with such an argument.  */
 
-  if (ls->file_symtabs->size () == 1
-      && ls->file_symtabs->front () == nullptr)
+  if (ls->file_symtabs.size () == 1
+      && ls->file_symtabs.front () == nullptr)
     {
       set_current_program_space (self->program_space);
 
       /* Make sure we have at least a default source line.  */
       set_default_source_symtab_and_line ();
       initialize_defaults (&self->default_symtab, &self->default_line);
-      *ls->file_symtabs
+      ls->file_symtabs
 	= collect_symtabs_from_filename (self->default_symtab->filename,
 					 self->search_pspace);
       use_default = 1;
@@ -2257,12 +2261,12 @@ convert_linespec_to_sals (struct linespec_state *state, linespec *ls)
 {
   std::vector<symtab_and_line> sals;
 
-  if (ls->labels.label_symbols != NULL)
+  if (!ls->labels.label_symbols.empty ())
     {
       /* We have just a bunch of functions/methods or labels.  */
       struct symtab_and_line sal;
 
-      for (const auto &sym : *ls->labels.label_symbols)
+      for (const auto &sym : ls->labels.label_symbols)
 	{
 	  struct program_space *pspace
 	    = SYMTAB_PSPACE (symbol_symtab (sym.symbol));
@@ -2273,18 +2277,18 @@ convert_linespec_to_sals (struct linespec_state *state, linespec *ls)
 			     sym.symbol->natural_name (), 0);
 	}
     }
-  else if (ls->function_symbols != NULL || ls->minimal_symbols != NULL)
+  else if (!ls->function_symbols.empty () || !ls->minimal_symbols.empty ())
     {
       /* We have just a bunch of functions and/or methods.  */
-      if (ls->function_symbols != NULL)
+      if (!ls->function_symbols.empty ())
 	{
 	  /* Sort symbols so that symbols with the same program space are next
 	     to each other.  */
-	  std::sort (ls->function_symbols->begin (),
-		     ls->function_symbols->end (),
+	  std::sort (ls->function_symbols.begin (),
+		     ls->function_symbols.end (),
 		     compare_symbols);
 
-	  for (const auto &sym : *ls->function_symbols)
+	  for (const auto &sym : ls->function_symbols)
 	    {
 	      program_space *pspace
 		= SYMTAB_PSPACE (symbol_symtab (sym.symbol));
@@ -2297,13 +2301,13 @@ convert_linespec_to_sals (struct linespec_state *state, linespec *ls)
 	      bool found_ifunc = false;
 
 	      if (state->funfirstline
-		   && ls->minimal_symbols != NULL
+		   && !ls->minimal_symbols.empty ()
 		   && SYMBOL_CLASS (sym.symbol) == LOC_BLOCK)
 		{
 		  const CORE_ADDR addr
 		    = BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym.symbol));
 
-		  for (const auto &elem : *ls->minimal_symbols)
+		  for (const auto &elem : ls->minimal_symbols)
 		    {
 		      if (MSYMBOL_TYPE (elem.minsym) == mst_text_gnu_ifunc
 			  || MSYMBOL_TYPE (elem.minsym) == mst_data_gnu_ifunc)
@@ -2340,14 +2344,14 @@ convert_linespec_to_sals (struct linespec_state *state, linespec *ls)
 	    }
 	}
 
-      if (ls->minimal_symbols != NULL)
+      if (!ls->minimal_symbols.empty ())
 	{
 	  /* Sort minimal symbols by program space, too  */
-	  std::sort (ls->minimal_symbols->begin (),
-		     ls->minimal_symbols->end (),
+	  std::sort (ls->minimal_symbols.begin (),
+		     ls->minimal_symbols.end (),
 		     compare_msymbols);
 
-	  for (const auto &elem : *ls->minimal_symbols)
+	  for (const auto &elem : ls->minimal_symbols)
 	    {
 	      program_space *pspace = elem.objfile->pspace;
 	      set_current_program_space (pspace);
@@ -2398,8 +2402,6 @@ convert_explicit_location_to_linespec (struct linespec_state *self,
 				       const char *label_name,
 				       struct line_offset line_offset)
 {
-  std::vector<block_symbol> symbols;
-  std::vector<block_symbol> *labels;
   std::vector<bound_minimal_symbol> minimal_symbols;
 
   result->explicit_loc.func_name_match_type = fname_match_type;
@@ -2408,7 +2410,7 @@ convert_explicit_location_to_linespec (struct linespec_state *self,
     {
       try
 	{
-	  *result->file_symtabs
+	  result->file_symtabs
 	    = symtabs_from_filename (source_filename, self->search_pspace);
 	}
       catch (const gdb_exception_error &except)
@@ -2420,11 +2422,13 @@ convert_explicit_location_to_linespec (struct linespec_state *self,
   else
     {
       /* A NULL entry means to use the default symtab.  */
-      result->file_symtabs->push_back (nullptr);
+      result->file_symtabs.push_back (nullptr);
     }
 
   if (function_name != NULL)
     {
+      std::vector<block_symbol> symbols;
+
       find_linespec_symbols (self, result->file_symtabs,
 			     function_name, fname_match_type,
 			     &symbols, &minimal_symbols);
@@ -2434,25 +2438,24 @@ convert_explicit_location_to_linespec (struct linespec_state *self,
 				result->explicit_loc.source_filename);
 
       result->explicit_loc.function_name = xstrdup (function_name);
-      result->function_symbols
-	= new std::vector<block_symbol> (std::move (symbols));
-      result->minimal_symbols
-	= new std::vector<bound_minimal_symbol> (std::move (minimal_symbols));
+      result->function_symbols = std::move (symbols);
+      result->minimal_symbols = std::move (minimal_symbols);
     }
 
   if (label_name != NULL)
     {
-      labels = find_label_symbols (self, result->function_symbols,
-				   &symbols, label_name);
+      std::vector<block_symbol> symbols;
+      std::vector<block_symbol> labels
+	= find_label_symbols (self, result->function_symbols,
+			      &symbols, label_name);
 
-      if (labels == NULL)
+      if (labels.empty ())
 	undefined_label_error (result->explicit_loc.function_name,
 			       label_name);
 
       result->explicit_loc.label_name = xstrdup (label_name);
       result->labels.label_symbols = labels;
-      result->labels.function_symbols
-	= new std::vector<block_symbol> (std::move (symbols));
+      result->labels.function_symbols = std::move (symbols);
     }
 
   if (line_offset.sign != LINE_OFFSET_UNKNOWN)
@@ -2591,7 +2594,7 @@ parse_linespec (linespec_parser *parser, const char *arg,
     {
       /* A NULL entry means to use GLOBAL_DEFAULT_SYMTAB.  */
       if (parser->completion_tracker == NULL)
-	PARSER_RESULT (parser)->file_symtabs->push_back (nullptr);
+	PARSER_RESULT (parser)->file_symtabs.push_back (nullptr);
 
       /* User specified a convenience variable or history value.  */
       gdb::unique_xmalloc_ptr<char> var = copy_token_string (token);
@@ -2632,7 +2635,7 @@ parse_linespec (linespec_parser *parser, const char *arg,
       /* Check if the input is a filename.  */
       try
 	{
-	  *PARSER_RESULT (parser)->file_symtabs
+	  PARSER_RESULT (parser)->file_symtabs
 	    = symtabs_from_filename (user_filename.get (),
 				     PARSER_STATE (parser)->search_pspace);
 	}
@@ -2655,7 +2658,7 @@ parse_linespec (linespec_parser *parser, const char *arg,
       else
 	{
 	  /* A NULL entry means to use GLOBAL_DEFAULT_SYMTAB.  */
-	  PARSER_RESULT (parser)->file_symtabs->push_back (nullptr);
+	  PARSER_RESULT (parser)->file_symtabs.push_back (nullptr);
 	}
     }
   /* If the next token is not EOI, KEYWORD, or COMMA, issue an error.  */
@@ -2671,17 +2674,17 @@ parse_linespec (linespec_parser *parser, const char *arg,
   else
     {
       /* A NULL entry means to use GLOBAL_DEFAULT_SYMTAB.  */
-      PARSER_RESULT (parser)->file_symtabs->push_back (nullptr);
+      PARSER_RESULT (parser)->file_symtabs.push_back (nullptr);
     }
 
   /* Parse the rest of the linespec.  */
   linespec_parse_basic (parser);
 
   if (parser->completion_tracker == NULL
-      && PARSER_RESULT (parser)->function_symbols == NULL
-      && PARSER_RESULT (parser)->labels.label_symbols == NULL
+      && PARSER_RESULT (parser)->function_symbols.empty ()
+      && PARSER_RESULT (parser)->labels.label_symbols.empty ()
       && PARSER_EXPLICIT (parser)->line_offset.sign == LINE_OFFSET_UNKNOWN
-      && PARSER_RESULT (parser)->minimal_symbols == NULL)
+      && PARSER_RESULT (parser)->minimal_symbols.empty ())
     {
       /* The linespec didn't parse.  Re-throw the file exception if
 	 there was one.  */
@@ -2753,7 +2756,6 @@ linespec_parser::linespec_parser (int flags,
 				  struct linespec_result *canonical)
 {
   lexer.current.type = LSTOKEN_CONSUMED;
-  PARSER_RESULT (this)->file_symtabs = new std::vector<symtab *> ();
   PARSER_EXPLICIT (this)->func_name_match_type
     = symbol_name_match_type::WILD;
   PARSER_EXPLICIT (this)->line_offset.sign = LINE_OFFSET_UNKNOWN;
@@ -2779,12 +2781,6 @@ linespec_parser::~linespec_parser ()
   xfree (PARSER_EXPLICIT (this)->label_name);
   xfree (PARSER_EXPLICIT (this)->function_name);
 
-  delete PARSER_RESULT (this)->file_symtabs;
-  delete PARSER_RESULT (this)->function_symbols;
-  delete PARSER_RESULT (this)->minimal_symbols;
-  delete PARSER_RESULT (this)->labels.label_symbols;
-  delete PARSER_RESULT (this)->labels.function_symbols;
-
   linespec_state_destructor (PARSER_STATE (this));
 }
 
@@ -2913,20 +2909,16 @@ complete_label (completion_tracker &tracker,
 		const char *label_name)
 {
   std::vector<block_symbol> label_function_symbols;
-  std::vector<block_symbol> *labels
+  std::vector<block_symbol> labels
     = find_label_symbols (PARSER_STATE (parser),
 			  PARSER_RESULT (parser)->function_symbols,
 			  &label_function_symbols,
 			  label_name, true);
 
-  if (labels != nullptr)
+  for (const auto &label : labels)
     {
-      for (const auto &label : *labels)
-	{
-	  char *match = xstrdup (label.symbol->search_name ());
-	  tracker.add_completion (gdb::unique_xmalloc_ptr<char> (match));
-	}
-      delete labels;
+      char *match = xstrdup (label.symbol->search_name ());
+      tracker.add_completion (gdb::unique_xmalloc_ptr<char> (match));
     }
 }
 
@@ -3031,10 +3023,8 @@ linespec_complete (completion_tracker &tracker, const char *text,
 			     func_name, match_type,
 			     &function_symbols, &minimal_symbols);
 
-      PARSER_RESULT (&parser)->function_symbols
-	= new std::vector<block_symbol> (std::move (function_symbols));
-      PARSER_RESULT (&parser)->minimal_symbols
-	= new std::vector<bound_minimal_symbol> (std::move (minimal_symbols));
+      PARSER_RESULT (&parser)->function_symbols = std::move (function_symbols);
+      PARSER_RESULT (&parser)->minimal_symbols = std::move (minimal_symbols);
 
       complete_label (tracker, &parser, parser.completion_word);
     }
@@ -3436,10 +3426,8 @@ decode_objc (struct linespec_state *self, linespec *ls, const char *arg)
       saved_arg[new_argptr - arg] = '\0';
 
       ls->explicit_loc.function_name = xstrdup (saved_arg);
-      ls->function_symbols
-	= new std::vector<block_symbol> (std::move (symbols));
-      ls->minimal_symbols
-	= new std::vector<bound_minimal_symbol> (std::move (minimal_symbols));
+      ls->function_symbols = std::move (symbols);
+      ls->minimal_symbols = std::move (minimal_symbols);
       values = convert_linespec_to_sals (self, ls);
 
       if (self->canonical)
@@ -3533,14 +3521,14 @@ decode_compound_collector::operator () (block_symbol *bsym)
 
 static std::vector<block_symbol>
 lookup_prefix_sym (struct linespec_state *state,
-		   std::vector<symtab *> *file_symtabs,
+		   const std::vector<symtab *> &file_symtabs,
 		   const char *class_name)
 {
   decode_compound_collector collector;
 
   lookup_name_info lookup_name (class_name, symbol_name_match_type::FULL);
 
-  for (const auto &elt : *file_symtabs)
+  for (const auto &elt : file_symtabs)
     {
       if (elt == nullptr)
 	{
@@ -3658,7 +3646,8 @@ find_superclass_methods (std::vector<struct type *> &&superclasses,
    in SYMBOLS (for debug symbols) and MINSYMS (for minimal symbols).  */
 
 static void
-find_method (struct linespec_state *self, std::vector<symtab *> *file_symtabs,
+find_method (struct linespec_state *self,
+	     const std::vector<symtab *> &file_symtabs,
 	     const char *class_name, const char *method_name,
 	     std::vector<block_symbol> *sym_classes,
 	     std::vector<block_symbol> *symbols,
@@ -3675,7 +3664,7 @@ find_method (struct linespec_state *self, std::vector<symtab *> *file_symtabs,
 	     compare_symbols);
 
   info.state = self;
-  info.file_symtabs = file_symtabs;
+  info.file_symtabs = &file_symtabs;
   info.result.symbols = symbols;
   info.result.minimal_symbols = minsyms;
 
@@ -3875,7 +3864,7 @@ symbol_searcher::find_all_symbols (const std::string &name,
 
 static void
 find_function_symbols (struct linespec_state *state,
-		       std::vector<symtab *> *file_symtabs, const char *name,
+		       const std::vector<symtab *> &file_symtabs, const char *name,
 		       symbol_name_match_type name_match_type,
 		       std::vector<block_symbol> *symbols,
 		       std::vector<bound_minimal_symbol> *minsyms)
@@ -3886,7 +3875,7 @@ find_function_symbols (struct linespec_state *state,
   info.state = state;
   info.result.symbols = symbols;
   info.result.minimal_symbols = minsyms;
-  info.file_symtabs = file_symtabs;
+  info.file_symtabs = &file_symtabs;
 
   /* Try NAME as an Objective-C selector.  */
   find_imps (name, &symbol_names);
@@ -3903,7 +3892,7 @@ find_function_symbols (struct linespec_state *state,
 
 static void
 find_linespec_symbols (struct linespec_state *state,
-		       std::vector<symtab *> *file_symtabs,
+		       const std::vector<symtab *> &file_symtabs,
 		       const char *lookup_name,
 		       symbol_name_match_type name_match_type,
 		       std::vector <block_symbol> *symbols,
@@ -4031,8 +4020,7 @@ find_label_symbols_in_block (const struct block *block,
     }
 }
 
-/* Return all labels that match name NAME in FUNCTION_SYMBOLS or NULL
-   if no matches were found.
+/* Return all labels that match name NAME in FUNCTION_SYMBOLS.
 
    Return the actual function symbol in which the label was found in
    LABEL_FUNC_RET.  If COMPLETION_MODE is true, then NAME is
@@ -4040,9 +4028,9 @@ find_label_symbols_in_block (const struct block *block,
    exactly NAME match.  */
 
 
-static std::vector<block_symbol> *
+static std::vector<block_symbol>
 find_label_symbols (struct linespec_state *self,
-		    std::vector<block_symbol> *function_symbols,
+		    const std::vector<block_symbol> &function_symbols,
 		    std::vector<block_symbol> *label_funcs_ret,
 		    const char *name,
 		    bool completion_mode)
@@ -4051,7 +4039,7 @@ find_label_symbols (struct linespec_state *self,
   struct symbol *fn_sym;
   std::vector<block_symbol> result;
 
-  if (function_symbols == NULL)
+  if (function_symbols.empty ())
     {
       set_current_program_space (self->program_space);
       block = get_current_search_block ();
@@ -4060,8 +4048,10 @@ find_label_symbols (struct linespec_state *self,
 	   block && !BLOCK_FUNCTION (block);
 	   block = BLOCK_SUPERBLOCK (block))
 	;
+
       if (!block)
-	return NULL;
+	return {};
+
       fn_sym = BLOCK_FUNCTION (block);
 
       find_label_symbols_in_block (block, name, fn_sym, completion_mode,
@@ -4069,7 +4059,7 @@ find_label_symbols (struct linespec_state *self,
     }
   else
     {
-      for (const auto &elt : *function_symbols)
+      for (const auto &elt : function_symbols)
 	{
 	  fn_sym = elt.symbol;
 	  set_current_program_space (SYMTAB_PSPACE (symbol_symtab (fn_sym)));
@@ -4080,9 +4070,7 @@ find_label_symbols (struct linespec_state *self,
 	}
     }
 
-  if (!result.empty ())
-    return new std::vector<block_symbol> (std::move (result));
-  return nullptr;
+  return result;
 }
 
 \f
@@ -4098,7 +4086,7 @@ decode_digits_list_mode (struct linespec_state *self,
 
   std::vector<symtab_and_line> values;
 
-  for (const auto &elt : *ls->file_symtabs)
+  for (const auto &elt : ls->file_symtabs)
     {
       /* The logic above should ensure this.  */
       gdb_assert (elt != NULL);
@@ -4130,7 +4118,7 @@ decode_digits_ordinary (struct linespec_state *self,
 			struct linetable_entry **best_entry)
 {
   std::vector<symtab_and_line> sals;
-  for (const auto &elt : *ls->file_symtabs)
+  for (const auto &elt : ls->file_symtabs)
     {
       std::vector<CORE_ADDR> pcs;
 
-- 
2.33.1

Re: [PATCH 1/2] gdb/linespec.c: simplify condition

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> We can remove the empty check: if the vector has size 1, it is obviously
Simon> not empty.  This code ended up like this because the empty check used to
Simon> be a NULL check.

This one seems obvious.

Tom

Re: [PATCH 2/2] gdb: make struct linespect contain vectors, not pointers to vectors

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> struct linespec contains pointers to vectors, instead of containing
Simon> vectors directly.  This is probably historical, when linespec_parser
Simon> (which contains a struct linespec field) was not C++-ified yet.  But it
Simon> seems easy to change the pointers to vectors to just vectors today.
Simon> This simplifies the code, we don't need to manually allocate and delete
Simon> the vectors and there's no pointer that can be NULL.

Simon> As far as I understand, there was not meaningful distinction between a
Simon> NULL pointer to vector and an empty vector.  So all NULL checks are
Simon> changed for !empty checks.

I think the main danger is that a vector would be copied, but I looked
through the patch and didn't see any instances of this, so I think this
is ok.

Tom

Re: [PATCH 1/2] gdb/linespec.c: simplify condition

[Simon Marchi]

On 2021-12-07 2:32 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> We can remove the empty check: if the vector has size 1, it is obviously
> Simon> not empty.  This code ended up like this because the empty check used to
> Simon> be a NULL check.
> 
> This one seems obvious.
> 
> Tom
> 

Thanks, I pushed this one, and I will look at the feedback for the second one a
bit later.

Simon

Re: [PATCH 2/2] gdb: make struct linespect contain vectors, not pointers to vectors

[Simon Marchi]

On 2021-12-07 14:35, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> struct linespec contains pointers to vectors, instead of containing
> Simon> vectors directly.  This is probably historical, when linespec_parser
> Simon> (which contains a struct linespec field) was not C++-ified yet.  But it
> Simon> seems easy to change the pointers to vectors to just vectors today.
> Simon> This simplifies the code, we don't need to manually allocate and delete
> Simon> the vectors and there's no pointer that can be NULL.
> 
> Simon> As far as I understand, there was not meaningful distinction between a
> Simon> NULL pointer to vector and an empty vector.  So all NULL checks are
> Simon> changed for !empty checks.
> 
> I think the main danger is that a vector would be copied, but I looked
> through the patch and didn't see any instances of this, so I think this
> is ok.

Ok, thanks for checking.  I gave it another look, and it seems fine.
Whenever we assign one of the vectors, it's the return value of a
function (which results in a move) or an explicit move.  When passing
one of the vectors to a function, it's by const-ref

I'll push the patch.

Simon