[PATCH 0/4] Minor C++-ifications for Ada

[PATCH 0/4] Minor C++-ifications for Ada

[Tom Tromey]

This short series C++-ifies the Ada code a bit.  The main motivation
for this is simplification; overall this series reduces the number of
lines of code in gdb.  It also replaces some manual management with
automatic management.

Tested on x86-64 Fedora 32.

Tom

[PATCH 1/4] Use new for ada_symbol_cache

[Tom Tromey]

This changes the ada_symbol_cache to be allocated with 'new' and
managed via unique_ptr.  This simplifies the code somewhat.  Also,
ada_clear_symbol_cache is changed so that it does not allocate a
symbol cache just to clear it.

gdb/ChangeLog
2021-02-18  Tom Tromey  <tromey@adacore.com>

	* ada-lang.c (struct ada_symbol_cache) <cache_space>: Now an
	auto_obstack.
	<root>: Initialize.
	(ada_pspace_data): Remove destructor.
	<sym_cache>: Now a unique_ptr.
	(ada_init_symbol_cache, ada_free_symbol_cache): Remove.
	(ada_get_symbol_cache): Use 'new'.
	(ada_clear_symbol_cache): Rewrite.
---
 gdb/ChangeLog  | 11 +++++++++++
 gdb/ada-lang.c | 51 +++++++++++---------------------------------------
 2 files changed, 22 insertions(+), 40 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 416a45be58e..e2b2e6105b0 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -283,14 +283,12 @@ struct cache_entry
 struct ada_symbol_cache
 {
   /* An obstack used to store the entries in our cache.  */
-  struct obstack cache_space;
+  struct auto_obstack cache_space;
 
   /* The root of the hash table used to implement our symbol cache.  */
-  struct cache_entry *root[HASH_SIZE];
+  struct cache_entry *root[HASH_SIZE] {};
 };
 
-static void ada_free_symbol_cache (struct ada_symbol_cache *sym_cache);
-
 /* Maximum-sized dynamic type.  */
 static unsigned int varsize_limit;
 
@@ -385,14 +383,8 @@ ada_inferior_exit (struct inferior *inf)
 /* This module's per-program-space data.  */
 struct ada_pspace_data
 {
-  ~ada_pspace_data ()
-  {
-    if (sym_cache != NULL)
-      ada_free_symbol_cache (sym_cache);
-  }
-
   /* The Ada symbol cache.  */
-  struct ada_symbol_cache *sym_cache = nullptr;
+  std::unique_ptr<ada_symbol_cache> sym_cache;
 };
 
 /* Key to our per-program-space data.  */
@@ -4604,24 +4596,6 @@ make_array_descriptor (struct type *type, struct value *arr)
    even in this case, some expensive name-based symbol searches are still
    sometimes necessary - to find an XVZ variable, mostly.  */
 
-/* Initialize the contents of SYM_CACHE.  */
-
-static void
-ada_init_symbol_cache (struct ada_symbol_cache *sym_cache)
-{
-  obstack_init (&sym_cache->cache_space);
-  memset (sym_cache->root, '\000', sizeof (sym_cache->root));
-}
-
-/* Free the memory used by SYM_CACHE.  */
-
-static void
-ada_free_symbol_cache (struct ada_symbol_cache *sym_cache)
-{
-  obstack_free (&sym_cache->cache_space, NULL);
-  xfree (sym_cache);
-}
-
 /* Return the symbol cache associated to the given program space PSPACE.
    If not allocated for this PSPACE yet, allocate and initialize one.  */
 
@@ -4630,25 +4604,22 @@ ada_get_symbol_cache (struct program_space *pspace)
 {
   struct ada_pspace_data *pspace_data = get_ada_pspace_data (pspace);
 
-  if (pspace_data->sym_cache == NULL)
-    {
-      pspace_data->sym_cache = XCNEW (struct ada_symbol_cache);
-      ada_init_symbol_cache (pspace_data->sym_cache);
-    }
+  if (pspace_data->sym_cache == nullptr)
+    pspace_data->sym_cache.reset (new ada_symbol_cache);
 
-  return pspace_data->sym_cache;
+  return pspace_data->sym_cache.get ();
 }
 
 /* Clear all entries from the symbol cache.  */
 
 static void
-ada_clear_symbol_cache (void)
+ada_clear_symbol_cache ()
 {
-  struct ada_symbol_cache *sym_cache
-    = ada_get_symbol_cache (current_program_space);
+  struct ada_pspace_data *pspace_data
+    = get_ada_pspace_data (current_program_space);
 
-  obstack_free (&sym_cache->cache_space, NULL);
-  ada_init_symbol_cache (sym_cache);
+  if (pspace_data->sym_cache != nullptr)
+    pspace_data->sym_cache.reset ();
 }
 
 /* Search our cache for an entry matching NAME and DOMAIN.
-- 
2.26.2

Re: [PATCH 1/4] Use new for ada_symbol_cache

[Andrew Burgess]

* Tom Tromey <tromey@adacore.com> [2021-02-18 11:04:27 -0700]:

> This changes the ada_symbol_cache to be allocated with 'new' and
> managed via unique_ptr.  This simplifies the code somewhat.  Also,
> ada_clear_symbol_cache is changed so that it does not allocate a
> symbol cache just to clear it.
> 
> gdb/ChangeLog
> 2021-02-18  Tom Tromey  <tromey@adacore.com>
> 
> 	* ada-lang.c (struct ada_symbol_cache) <cache_space>: Now an
> 	auto_obstack.
> 	<root>: Initialize.
> 	(ada_pspace_data): Remove destructor.
> 	<sym_cache>: Now a unique_ptr.
> 	(ada_init_symbol_cache, ada_free_symbol_cache): Remove.
> 	(ada_get_symbol_cache): Use 'new'.
> 	(ada_clear_symbol_cache): Rewrite.

LGTM.

Thanks,
Andrew

> ---
>  gdb/ChangeLog  | 11 +++++++++++
>  gdb/ada-lang.c | 51 +++++++++++---------------------------------------
>  2 files changed, 22 insertions(+), 40 deletions(-)
> 
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 416a45be58e..e2b2e6105b0 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -283,14 +283,12 @@ struct cache_entry
>  struct ada_symbol_cache
>  {
>    /* An obstack used to store the entries in our cache.  */
> -  struct obstack cache_space;
> +  struct auto_obstack cache_space;
>  
>    /* The root of the hash table used to implement our symbol cache.  */
> -  struct cache_entry *root[HASH_SIZE];
> +  struct cache_entry *root[HASH_SIZE] {};
>  };
>  
> -static void ada_free_symbol_cache (struct ada_symbol_cache *sym_cache);
> -
>  /* Maximum-sized dynamic type.  */
>  static unsigned int varsize_limit;
>  
> @@ -385,14 +383,8 @@ ada_inferior_exit (struct inferior *inf)
>  /* This module's per-program-space data.  */
>  struct ada_pspace_data
>  {
> -  ~ada_pspace_data ()
> -  {
> -    if (sym_cache != NULL)
> -      ada_free_symbol_cache (sym_cache);
> -  }
> -
>    /* The Ada symbol cache.  */
> -  struct ada_symbol_cache *sym_cache = nullptr;
> +  std::unique_ptr<ada_symbol_cache> sym_cache;
>  };
>  
>  /* Key to our per-program-space data.  */
> @@ -4604,24 +4596,6 @@ make_array_descriptor (struct type *type, struct value *arr)
>     even in this case, some expensive name-based symbol searches are still
>     sometimes necessary - to find an XVZ variable, mostly.  */
>  
> -/* Initialize the contents of SYM_CACHE.  */
> -
> -static void
> -ada_init_symbol_cache (struct ada_symbol_cache *sym_cache)
> -{
> -  obstack_init (&sym_cache->cache_space);
> -  memset (sym_cache->root, '\000', sizeof (sym_cache->root));
> -}
> -
> -/* Free the memory used by SYM_CACHE.  */
> -
> -static void
> -ada_free_symbol_cache (struct ada_symbol_cache *sym_cache)
> -{
> -  obstack_free (&sym_cache->cache_space, NULL);
> -  xfree (sym_cache);
> -}
> -
>  /* Return the symbol cache associated to the given program space PSPACE.
>     If not allocated for this PSPACE yet, allocate and initialize one.  */
>  
> @@ -4630,25 +4604,22 @@ ada_get_symbol_cache (struct program_space *pspace)
>  {
>    struct ada_pspace_data *pspace_data = get_ada_pspace_data (pspace);
>  
> -  if (pspace_data->sym_cache == NULL)
> -    {
> -      pspace_data->sym_cache = XCNEW (struct ada_symbol_cache);
> -      ada_init_symbol_cache (pspace_data->sym_cache);
> -    }
> +  if (pspace_data->sym_cache == nullptr)
> +    pspace_data->sym_cache.reset (new ada_symbol_cache);
>  
> -  return pspace_data->sym_cache;
> +  return pspace_data->sym_cache.get ();
>  }
>  
>  /* Clear all entries from the symbol cache.  */
>  
>  static void
> -ada_clear_symbol_cache (void)
> +ada_clear_symbol_cache ()
>  {
> -  struct ada_symbol_cache *sym_cache
> -    = ada_get_symbol_cache (current_program_space);
> +  struct ada_pspace_data *pspace_data
> +    = get_ada_pspace_data (current_program_space);
>  
> -  obstack_free (&sym_cache->cache_space, NULL);
> -  ada_init_symbol_cache (sym_cache);
> +  if (pspace_data->sym_cache != nullptr)
> +    pspace_data->sym_cache.reset ();
>  }
>  
>  /* Search our cache for an entry matching NAME and DOMAIN.
> -- 
> 2.26.2
> 

[PATCH 2/4] Simplify resolve_subexp by using C++ algorithms

[Tom Tromey]

This changes resolve_subexp to use any_of and the erase-remove idiom
to simplify the code somewhat.  This simplifies the next patch a bit.

gdb/ChangeLog
2021-02-18  Tom Tromey  <tromey@adacore.com>

	* ada-lang.c (resolve_subexp): Use any_of and erase-remove idiom.
---
 gdb/ChangeLog  |  4 ++++
 gdb/ada-lang.c | 57 +++++++++++++++++++++++++-------------------------
 2 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index e2b2e6105b0..db4f3591131 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -3660,41 +3660,40 @@ resolve_subexp (expression_up *expp, int *pos, int deprocedure_p,
 	    ada_lookup_symbol_list (exp->elts[pc + 2].symbol->linkage_name (),
 				    exp->elts[pc + 1].block, VAR_DOMAIN,
 				    &candidates);
+	  /* Paranoia.  */
+	  candidates.resize (n_candidates);
 
-	  if (n_candidates > 1)
+	  if (std::any_of (candidates.begin (),
+			   candidates.end (),
+			   [] (block_symbol &sym)
+			   {
+			     switch (SYMBOL_CLASS (sym.symbol))
+			       {
+			       case LOC_REGISTER:
+			       case LOC_ARG:
+			       case LOC_REF_ARG:
+			       case LOC_REGPARM_ADDR:
+			       case LOC_LOCAL:
+			       case LOC_COMPUTED:
+				 return true;
+			       default:
+				 return false;
+			       }
+			   }))
 	    {
 	      /* Types tend to get re-introduced locally, so if there
 		 are any local symbols that are not types, first filter
 		 out all types.  */
-	      int j;
-	      for (j = 0; j < n_candidates; j += 1)
-		switch (SYMBOL_CLASS (candidates[j].symbol))
+	      candidates.erase
+		(std::remove_if
+		 (candidates.begin (),
+		  candidates.end (),
+		  [] (block_symbol &sym)
 		  {
-		  case LOC_REGISTER:
-		  case LOC_ARG:
-		  case LOC_REF_ARG:
-		  case LOC_REGPARM_ADDR:
-		  case LOC_LOCAL:
-		  case LOC_COMPUTED:
-		    goto FoundNonType;
-		  default:
-		    break;
-		  }
-	    FoundNonType:
-	      if (j < n_candidates)
-		{
-		  j = 0;
-		  while (j < n_candidates)
-		    {
-		      if (SYMBOL_CLASS (candidates[j].symbol) == LOC_TYPEDEF)
-			{
-			  candidates[j] = candidates[n_candidates - 1];
-			  n_candidates -= 1;
-			}
-		      else
-			j += 1;
-		    }
-		}
+		    return SYMBOL_CLASS (sym.symbol) == LOC_TYPEDEF;
+		  }),
+		 candidates.end ());
+	      n_candidates = candidates.size ();
 	    }
 
 	  if (n_candidates == 0)
-- 
2.26.2

Re: [PATCH 2/4] Simplify resolve_subexp by using C++ algorithms

[Andrew Burgess]

* Tom Tromey <tromey@adacore.com> [2021-02-18 11:04:28 -0700]:

> This changes resolve_subexp to use any_of and the erase-remove idiom
> to simplify the code somewhat.  This simplifies the next patch a bit.
> 
> gdb/ChangeLog
> 2021-02-18  Tom Tromey  <tromey@adacore.com>
> 
> 	* ada-lang.c (resolve_subexp): Use any_of and erase-remove idiom.
> ---
>  gdb/ChangeLog  |  4 ++++
>  gdb/ada-lang.c | 57 +++++++++++++++++++++++++-------------------------
>  2 files changed, 32 insertions(+), 29 deletions(-)
> 
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index e2b2e6105b0..db4f3591131 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -3660,41 +3660,40 @@ resolve_subexp (expression_up *expp, int *pos, int deprocedure_p,
>  	    ada_lookup_symbol_list (exp->elts[pc + 2].symbol->linkage_name (),
>  				    exp->elts[pc + 1].block, VAR_DOMAIN,
>  				    &candidates);
> +	  /* Paranoia.  */
> +	  candidates.resize (n_candidates);

It's not clear what you're being paranoid about here?  I would have
expected that:

   gdb_assert (candidates.size () == n_candidates);

based on the comments on ada_lookup_symbol_list.  But even if that's
not true I can't see what harm would occur if the size and
n_candidates were out of sync.

Could you expand the command here to help those of us who are slower
to catch on :)

Otherwise, I didn't try to confirm the code does exactly as before, I
figure testing will do that, but the new code look very readable, so
LGTM.

Thanks,
Andrew

>  
> -	  if (n_candidates > 1)
> +	  if (std::any_of (candidates.begin (),
> +			   candidates.end (),
> +			   [] (block_symbol &sym)
> +			   {
> +			     switch (SYMBOL_CLASS (sym.symbol))
> +			       {
> +			       case LOC_REGISTER:
> +			       case LOC_ARG:
> +			       case LOC_REF_ARG:
> +			       case LOC_REGPARM_ADDR:
> +			       case LOC_LOCAL:
> +			       case LOC_COMPUTED:
> +				 return true;
> +			       default:
> +				 return false;
> +			       }
> +			   }))
>  	    {
>  	      /* Types tend to get re-introduced locally, so if there
>  		 are any local symbols that are not types, first filter
>  		 out all types.  */
> -	      int j;
> -	      for (j = 0; j < n_candidates; j += 1)
> -		switch (SYMBOL_CLASS (candidates[j].symbol))
> +	      candidates.erase
> +		(std::remove_if
> +		 (candidates.begin (),
> +		  candidates.end (),
> +		  [] (block_symbol &sym)
>  		  {
> -		  case LOC_REGISTER:
> -		  case LOC_ARG:
> -		  case LOC_REF_ARG:
> -		  case LOC_REGPARM_ADDR:
> -		  case LOC_LOCAL:
> -		  case LOC_COMPUTED:
> -		    goto FoundNonType;
> -		  default:
> -		    break;
> -		  }
> -	    FoundNonType:
> -	      if (j < n_candidates)
> -		{
> -		  j = 0;
> -		  while (j < n_candidates)
> -		    {
> -		      if (SYMBOL_CLASS (candidates[j].symbol) == LOC_TYPEDEF)
> -			{
> -			  candidates[j] = candidates[n_candidates - 1];
> -			  n_candidates -= 1;
> -			}
> -		      else
> -			j += 1;
> -		    }
> -		}
> +		    return SYMBOL_CLASS (sym.symbol) == LOC_TYPEDEF;
> +		  }),
> +		 candidates.end ());
> +	      n_candidates = candidates.size ();
>  	    }
>  
>  	  if (n_candidates == 0)
> -- 
> 2.26.2
> 

Re: [PATCH 2/4] Simplify resolve_subexp by using C++ algorithms

[Andrew Burgess]

* Andrew Burgess <andrew.burgess@embecosm.com> [2021-02-19 17:55:58 +0000]:

> * Tom Tromey <tromey@adacore.com> [2021-02-18 11:04:28 -0700]:
> 
> > This changes resolve_subexp to use any_of and the erase-remove idiom
> > to simplify the code somewhat.  This simplifies the next patch a bit.
> > 
> > gdb/ChangeLog
> > 2021-02-18  Tom Tromey  <tromey@adacore.com>
> > 
> > 	* ada-lang.c (resolve_subexp): Use any_of and erase-remove idiom.
> > ---
> >  gdb/ChangeLog  |  4 ++++
> >  gdb/ada-lang.c | 57 +++++++++++++++++++++++++-------------------------
> >  2 files changed, 32 insertions(+), 29 deletions(-)
> > 
> > diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> > index e2b2e6105b0..db4f3591131 100644
> > --- a/gdb/ada-lang.c
> > +++ b/gdb/ada-lang.c
> > @@ -3660,41 +3660,40 @@ resolve_subexp (expression_up *expp, int *pos, int deprocedure_p,
> >  	    ada_lookup_symbol_list (exp->elts[pc + 2].symbol->linkage_name (),
> >  				    exp->elts[pc + 1].block, VAR_DOMAIN,
> >  				    &candidates);
> > +	  /* Paranoia.  */
> > +	  candidates.resize (n_candidates);
> 
> It's not clear what you're being paranoid about here?  I would have
> expected that:
> 
>    gdb_assert (candidates.size () == n_candidates);
> 
> based on the comments on ada_lookup_symbol_list.  But even if that's
> not true I can't see what harm would occur if the size and
> n_candidates were out of sync.
> 
> Could you expand the command here to help those of us who are slower
> to catch on :)

OK, just looked at the next patch and saw that these get deleted
anyway, so don't waste your time.

This LGTM as is.

Thanks,
Andrew



> 
> Otherwise, I didn't try to confirm the code does exactly as before, I
> figure testing will do that, but the new code look very readable, so
> LGTM.
> 
> Thanks,
> Andrew
> 
> >  
> > -	  if (n_candidates > 1)
> > +	  if (std::any_of (candidates.begin (),
> > +			   candidates.end (),
> > +			   [] (block_symbol &sym)
> > +			   {
> > +			     switch (SYMBOL_CLASS (sym.symbol))
> > +			       {
> > +			       case LOC_REGISTER:
> > +			       case LOC_ARG:
> > +			       case LOC_REF_ARG:
> > +			       case LOC_REGPARM_ADDR:
> > +			       case LOC_LOCAL:
> > +			       case LOC_COMPUTED:
> > +				 return true;
> > +			       default:
> > +				 return false;
> > +			       }
> > +			   }))
> >  	    {
> >  	      /* Types tend to get re-introduced locally, so if there
> >  		 are any local symbols that are not types, first filter
> >  		 out all types.  */
> > -	      int j;
> > -	      for (j = 0; j < n_candidates; j += 1)
> > -		switch (SYMBOL_CLASS (candidates[j].symbol))
> > +	      candidates.erase
> > +		(std::remove_if
> > +		 (candidates.begin (),
> > +		  candidates.end (),
> > +		  [] (block_symbol &sym)
> >  		  {
> > -		  case LOC_REGISTER:
> > -		  case LOC_ARG:
> > -		  case LOC_REF_ARG:
> > -		  case LOC_REGPARM_ADDR:
> > -		  case LOC_LOCAL:
> > -		  case LOC_COMPUTED:
> > -		    goto FoundNonType;
> > -		  default:
> > -		    break;
> > -		  }
> > -	    FoundNonType:
> > -	      if (j < n_candidates)
> > -		{
> > -		  j = 0;
> > -		  while (j < n_candidates)
> > -		    {
> > -		      if (SYMBOL_CLASS (candidates[j].symbol) == LOC_TYPEDEF)
> > -			{
> > -			  candidates[j] = candidates[n_candidates - 1];
> > -			  n_candidates -= 1;
> > -			}
> > -		      else
> > -			j += 1;
> > -		    }
> > -		}
> > +		    return SYMBOL_CLASS (sym.symbol) == LOC_TYPEDEF;
> > +		  }),
> > +		 candidates.end ());
> > +	      n_candidates = candidates.size ();
> >  	    }
> >  
> >  	  if (n_candidates == 0)
> > -- 
> > 2.26.2
> > 

[PATCH 3/4] Return a vector from ada_lookup_symbol_list

[Tom Tromey]

This changes ada_lookup_symbol_list to return a std::vector, and
changes various other helper functions to follow.  This simplifies the
code, and makes it more type-safe (by using a vector where an obstack
had been used).

gdb/ChangeLog
2021-02-18  Tom Tromey  <tromey@adacore.com>

	* ada-lang.h (ada_lookup_symbol_list): Return a vector.
	* ada-lang.c (resolve_subexp): Update.
	(ada_resolve_function): Accept a vector.
	(is_nonfunction, add_defn_to_vec)
	(add_symbols_from_enclosing_procs): Likewise.
	(num_defns_collected, defns_collected): Remove.
	(remove_extra_symbols): Return a vector.
	(remove_irrelevant_renamings): Return void.
	(ada_add_local_symbols): Accept a vector.
	(struct match_data) <obstackp>: Remove.
	<resultp>: New member.
	(aux_add_nonlocal_symbols): Update.
	(ada_add_block_renamings, add_nonlocal_symbols)
	(ada_add_all_symbols): Accept a vector.
	(ada_lookup_symbol_list_worker, ada_lookup_symbol_list): Return a
	vector.
	(ada_lookup_symbol): Update.
	(ada_add_block_symbols): Accept a vector.
	(get_var_value, iterate_over_symbols): Update.
	* ada-exp.y (block_lookup, write_var_or_type, write_name_assoc):
	Update.
---
 gdb/ChangeLog  |  24 ++++
 gdb/ada-exp.y  |  35 +++---
 gdb/ada-lang.c | 299 +++++++++++++++++++------------------------------
 gdb/ada-lang.h |   5 +-
 4 files changed, 158 insertions(+), 205 deletions(-)

diff --git a/gdb/ada-exp.y b/gdb/ada-exp.y
index b55a0b434f7..a7e0ccbe5b1 100644
--- a/gdb/ada-exp.y
+++ b/gdb/ada-exp.y
@@ -948,8 +948,6 @@ static const struct block*
 block_lookup (const struct block *context, const char *raw_name)
 {
   const char *name;
-  std::vector<struct block_symbol> syms;
-  int nsyms;
   struct symtab *symtab;
   const struct block *result = NULL;
 
@@ -965,17 +963,18 @@ block_lookup (const struct block *context, const char *raw_name)
       name = name_storage.c_str ();
     }
 
-  nsyms = ada_lookup_symbol_list (name, context, VAR_DOMAIN, &syms);
+  std::vector<struct block_symbol> syms
+    = ada_lookup_symbol_list (name, context, VAR_DOMAIN);
 
   if (context == NULL
-      && (nsyms == 0 || SYMBOL_CLASS (syms[0].symbol) != LOC_BLOCK))
+      && (syms.empty () || SYMBOL_CLASS (syms[0].symbol) != LOC_BLOCK))
     symtab = lookup_symtab (name);
   else
     symtab = NULL;
 
   if (symtab != NULL)
     result = BLOCKVECTOR_BLOCK (SYMTAB_BLOCKVECTOR (symtab), STATIC_BLOCK);
-  else if (nsyms == 0 || SYMBOL_CLASS (syms[0].symbol) != LOC_BLOCK)
+  else if (syms.empty () || SYMBOL_CLASS (syms[0].symbol) != LOC_BLOCK)
     {
       if (context == NULL)
 	error (_("No file or function \"%s\"."), raw_name);
@@ -984,7 +983,7 @@ block_lookup (const struct block *context, const char *raw_name)
     }
   else
     {
-      if (nsyms > 1)
+      if (syms.size () > 1)
 	warning (_("Function name \"%s\" ambiguous here"), raw_name);
       result = SYMBOL_BLOCK_VALUE (syms[0].symbol);
     }
@@ -1216,8 +1215,6 @@ write_var_or_type (struct parser_state *par_state,
       tail_index = name_len;
       while (tail_index > 0)
 	{
-	  int nsyms;
-	  std::vector<struct block_symbol> syms;
 	  struct symbol *type_sym;
 	  struct symbol *renaming_sym;
 	  const char* renaming;
@@ -1226,15 +1223,15 @@ write_var_or_type (struct parser_state *par_state,
 	  int terminator = encoded_name[tail_index];
 
 	  encoded_name[tail_index] = '\0';
-	  nsyms = ada_lookup_symbol_list (encoded_name, block,
-					  VAR_DOMAIN, &syms);
+	  std::vector<struct block_symbol> syms
+	    = ada_lookup_symbol_list (encoded_name, block, VAR_DOMAIN);
 	  encoded_name[tail_index] = terminator;
 
 	  type_sym = select_possible_type_sym (syms);
 
 	  if (type_sym != NULL)
 	    renaming_sym = type_sym;
-	  else if (nsyms == 1)
+	  else if (syms.size () == 1)
 	    renaming_sym = syms[0].symbol;
 	  else 
 	    renaming_sym = NULL;
@@ -1285,7 +1282,7 @@ write_var_or_type (struct parser_state *par_state,
 		error (_("Invalid attempt to select from type: \"%s\"."),
 		       name0.ptr);
 	    }
-	  else if (tail_index == name_len && nsyms == 0)
+	  else if (tail_index == name_len && syms.empty ())
 	    {
 	      struct type *type = find_primitive_type (par_state,
 						       encoded_name);
@@ -1294,13 +1291,13 @@ write_var_or_type (struct parser_state *par_state,
 		return type;
 	    }
 
-	  if (nsyms == 1)
+	  if (syms.size () == 1)
 	    {
 	      write_var_from_sym (par_state, syms[0].block, syms[0].symbol);
 	      write_selectors (par_state, encoded_name + tail_index);
 	      return NULL;
 	    }
-	  else if (nsyms == 0) 
+	  else if (syms.empty ()) 
 	    {
 	      struct bound_minimal_symbol msym
 		= ada_lookup_simple_minsym (encoded_name);
@@ -1362,12 +1359,12 @@ write_name_assoc (struct parser_state *par_state, struct stoken name)
 {
   if (strchr (name.ptr, '.') == NULL)
     {
-      std::vector<struct block_symbol> syms;
-      int nsyms = ada_lookup_symbol_list (name.ptr,
-					  par_state->expression_context_block,
-					  VAR_DOMAIN, &syms);
+      std::vector<struct block_symbol> syms
+	= ada_lookup_symbol_list (name.ptr,
+				  par_state->expression_context_block,
+				  VAR_DOMAIN);
 
-      if (nsyms != 1 || SYMBOL_CLASS (syms[0].symbol) == LOC_TYPEDEF)
+      if (syms.size () != 1 || SYMBOL_CLASS (syms[0].symbol) == LOC_TYPEDEF)
 	write_exp_op_with_string (par_state, OP_NAME, name);
       else
 	write_var_from_sym (par_state, syms[0].block, syms[0].symbol);
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index db4f3591131..1bc7b912e91 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -100,24 +100,22 @@ static int ada_args_match (struct symbol *, struct value **, int);
 
 static struct value *make_array_descriptor (struct type *, struct value *);
 
-static void ada_add_block_symbols (struct obstack *,
+static void ada_add_block_symbols (std::vector<struct block_symbol> &,
 				   const struct block *,
 				   const lookup_name_info &lookup_name,
 				   domain_enum, struct objfile *);
 
-static void ada_add_all_symbols (struct obstack *, const struct block *,
+static void ada_add_all_symbols (std::vector<struct block_symbol> &,
+				 const struct block *,
 				 const lookup_name_info &lookup_name,
 				 domain_enum, int, int *);
 
-static int is_nonfunction (struct block_symbol *, int);
+static int is_nonfunction (const std::vector<struct block_symbol> &);
 
-static void add_defn_to_vec (struct obstack *, struct symbol *,
+static void add_defn_to_vec (std::vector<struct block_symbol> &,
+			     struct symbol *,
 			     const struct block *);
 
-static int num_defns_collected (struct obstack *);
-
-static struct block_symbol *defns_collected (struct obstack *, int);
-
 static struct value *resolve_subexp (expression_up *, int *, int,
 				     struct type *, int,
 				     innermost_block_tracker *);
@@ -205,7 +203,7 @@ static struct value *ada_search_struct_field (const char *, struct value *, int,
 static int find_struct_field (const char *, struct type *, int,
 			      struct type **, int *, int *, int *, int *);
 
-static int ada_resolve_function (struct block_symbol *, int,
+static int ada_resolve_function (std::vector<struct block_symbol> &,
 				 struct value **, int, const char *,
 				 struct type *, int);
 
@@ -3653,15 +3651,9 @@ resolve_subexp (expression_up *expp, int *pos, int deprocedure_p,
     case OP_VAR_VALUE:
       if (SYMBOL_DOMAIN (exp->elts[pc + 2].symbol) == UNDEF_DOMAIN)
 	{
-	  std::vector<struct block_symbol> candidates;
-	  int n_candidates;
-
-	  n_candidates =
-	    ada_lookup_symbol_list (exp->elts[pc + 2].symbol->linkage_name (),
-				    exp->elts[pc + 1].block, VAR_DOMAIN,
-				    &candidates);
-	  /* Paranoia.  */
-	  candidates.resize (n_candidates);
+	  std::vector<struct block_symbol> candidates
+	    = ada_lookup_symbol_list (exp->elts[pc + 2].symbol->linkage_name (),
+				      exp->elts[pc + 1].block, VAR_DOMAIN);
 
 	  if (std::any_of (candidates.begin (),
 			   candidates.end (),
@@ -3693,19 +3685,17 @@ resolve_subexp (expression_up *expp, int *pos, int deprocedure_p,
 		    return SYMBOL_CLASS (sym.symbol) == LOC_TYPEDEF;
 		  }),
 		 candidates.end ());
-	      n_candidates = candidates.size ();
 	    }
 
-	  if (n_candidates == 0)
+	  if (candidates.empty ())
 	    error (_("No definition found for %s"),
 		   exp->elts[pc + 2].symbol->print_name ());
-	  else if (n_candidates == 1)
+	  else if (candidates.size () == 1)
 	    i = 0;
-	  else if (deprocedure_p
-		   && !is_nonfunction (candidates.data (), n_candidates))
+	  else if (deprocedure_p && !is_nonfunction (candidates))
 	    {
 	      i = ada_resolve_function
-		(candidates.data (), n_candidates, NULL, 0,
+		(candidates, NULL, 0,
 		 exp->elts[pc + 2].symbol->linkage_name (),
 		 context_type, parse_completion);
 	      if (i < 0)
@@ -3716,7 +3706,7 @@ resolve_subexp (expression_up *expp, int *pos, int deprocedure_p,
 	    {
 	      printf_filtered (_("Multiple matches for %s\n"),
 			       exp->elts[pc + 2].symbol->print_name ());
-	      user_select_syms (candidates.data (), n_candidates, 1);
+	      user_select_syms (candidates.data (), candidates.size (), 1);
 	      i = 0;
 	    }
 
@@ -3741,20 +3731,16 @@ resolve_subexp (expression_up *expp, int *pos, int deprocedure_p,
 	if (exp->elts[pc + 3].opcode == OP_VAR_VALUE
 	    && SYMBOL_DOMAIN (exp->elts[pc + 5].symbol) == UNDEF_DOMAIN)
 	  {
-	    std::vector<struct block_symbol> candidates;
-	    int n_candidates;
+	    std::vector<struct block_symbol> candidates
+	      = ada_lookup_symbol_list (exp->elts[pc + 5].symbol->linkage_name (),
+					exp->elts[pc + 4].block, VAR_DOMAIN);
 
-	    n_candidates =
-	      ada_lookup_symbol_list (exp->elts[pc + 5].symbol->linkage_name (),
-				      exp->elts[pc + 4].block, VAR_DOMAIN,
-				      &candidates);
-
-	    if (n_candidates == 1)
+	    if (candidates.size () == 1)
 	      i = 0;
 	    else
 	      {
 		i = ada_resolve_function
-		  (candidates.data (), n_candidates,
+		  (candidates,
 		   argvec, nargs,
 		   exp->elts[pc + 5].symbol->linkage_name (),
 		   context_type, parse_completion);
@@ -3792,15 +3778,11 @@ resolve_subexp (expression_up *expp, int *pos, int deprocedure_p,
     case UNOP_ABS:
       if (possible_user_operator_p (op, argvec))
 	{
-	  std::vector<struct block_symbol> candidates;
-	  int n_candidates;
-
-	  n_candidates =
-	    ada_lookup_symbol_list (ada_decoded_op_name (op),
-				    NULL, VAR_DOMAIN,
-				    &candidates);
+	  std::vector<struct block_symbol> candidates
+	    = ada_lookup_symbol_list (ada_decoded_op_name (op),
+				      NULL, VAR_DOMAIN);
 
-	  i = ada_resolve_function (candidates.data (), n_candidates, argvec,
+	  i = ada_resolve_function (candidates, argvec,
 				    nargs, ada_decoded_op_name (op), NULL,
 				    parse_completion);
 	  if (i < 0)
@@ -3966,8 +3948,8 @@ return_match (struct type *func_type, struct type *context_type)
    the process; the index returned is for the modified vector.  */
 
 static int
-ada_resolve_function (struct block_symbol syms[],
-		      int nsyms, struct value **args, int nargs,
+ada_resolve_function (std::vector<struct block_symbol> &syms,
+		      struct value **args, int nargs,
 		      const char *name, struct type *context_type,
 		      int parse_completion)
 {
@@ -3981,7 +3963,7 @@ ada_resolve_function (struct block_symbol syms[],
      where every function is accepted.  */
   for (fallback = 0; m == 0 && fallback < 2; fallback++)
     {
-      for (k = 0; k < nsyms; k += 1)
+      for (k = 0; k < syms.size (); k += 1)
 	{
 	  struct type *type = ada_check_typedef (SYMBOL_TYPE (syms[k].symbol));
 
@@ -4003,7 +3985,7 @@ ada_resolve_function (struct block_symbol syms[],
   else if (m > 1 && !parse_completion)
     {
       printf_filtered (_("Multiple matches for %s\n"), name);
-      user_select_syms (syms, m, 1);
+      user_select_syms (syms.data (), m, 1);
       return 0;
     }
   return 0;
@@ -4737,14 +4719,12 @@ standard_lookup (const char *name, const struct block *block,
    in the symbol fields of SYMS[0..N-1].  We treat enumerals as functions, 
    since they contend in overloading in the same way.  */
 static int
-is_nonfunction (struct block_symbol syms[], int n)
+is_nonfunction (const std::vector<struct block_symbol> &syms)
 {
-  int i;
-
-  for (i = 0; i < n; i += 1)
-    if (SYMBOL_TYPE (syms[i].symbol)->code () != TYPE_CODE_FUNC
-	&& (SYMBOL_TYPE (syms[i].symbol)->code () != TYPE_CODE_ENUM
-	    || SYMBOL_CLASS (syms[i].symbol) != LOC_CONST))
+  for (const block_symbol &sym : syms)
+    if (SYMBOL_TYPE (sym.symbol)->code () != TYPE_CODE_FUNC
+	&& (SYMBOL_TYPE (sym.symbol)->code () != TYPE_CODE_ENUM
+	    || SYMBOL_CLASS (sym.symbol) != LOC_CONST))
       return 1;
 
   return 0;
@@ -4817,17 +4797,14 @@ lesseq_defined_than (struct symbol *sym0, struct symbol *sym1)
     }
 }
 
-/* Append (SYM,BLOCK,SYMTAB) to the end of the array of struct block_symbol
-   records in OBSTACKP.  Do nothing if SYM is a duplicate.  */
+/* Append (SYM,BLOCK) to the end of the array of struct block_symbol
+   records in RESULT.  Do nothing if SYM is a duplicate.  */
 
 static void
-add_defn_to_vec (struct obstack *obstackp,
+add_defn_to_vec (std::vector<struct block_symbol> &result,
 		 struct symbol *sym,
 		 const struct block *block)
 {
-  int i;
-  struct block_symbol *prevDefns = defns_collected (obstackp, 0);
-
   /* Do not try to complete stub types, as the debugger is probably
      already scanning all symbols matching a certain name at the
      time when this function is called.  Trying to replace the stub
@@ -4837,46 +4814,22 @@ add_defn_to_vec (struct obstack *obstackp,
      matches, with at least one of them complete.  It can then filter
      out the stub ones if needed.  */
 
-  for (i = num_defns_collected (obstackp) - 1; i >= 0; i -= 1)
+  for (int i = result.size () - 1; i >= 0; i -= 1)
     {
-      if (lesseq_defined_than (sym, prevDefns[i].symbol))
+      if (lesseq_defined_than (sym, result[i].symbol))
 	return;
-      else if (lesseq_defined_than (prevDefns[i].symbol, sym))
+      else if (lesseq_defined_than (result[i].symbol, sym))
 	{
-	  prevDefns[i].symbol = sym;
-	  prevDefns[i].block = block;
+	  result[i].symbol = sym;
+	  result[i].block = block;
 	  return;
 	}
     }
 
-  {
-    struct block_symbol info;
-
-    info.symbol = sym;
-    info.block = block;
-    obstack_grow (obstackp, &info, sizeof (struct block_symbol));
-  }
-}
-
-/* Number of block_symbol structures currently collected in current vector in
-   OBSTACKP.  */
-
-static int
-num_defns_collected (struct obstack *obstackp)
-{
-  return obstack_object_size (obstackp) / sizeof (struct block_symbol);
-}
-
-/* Vector of block_symbol structures currently collected in current vector in
-   OBSTACKP.  If FINISH, close off the vector and return its final address.  */
-
-static struct block_symbol *
-defns_collected (struct obstack *obstackp, int finish)
-{
-  if (finish)
-    return (struct block_symbol *) obstack_finish (obstackp);
-  else
-    return (struct block_symbol *) obstack_base (obstackp);
+  struct block_symbol info;
+  info.symbol = sym;
+  info.block = block;
+  result.push_back (info);
 }
 
 /* Return a bound minimal symbol matching NAME according to Ada
@@ -4922,7 +4875,7 @@ ada_lookup_simple_minsym (const char *name)
    with a wildcard prefix.  */
 
 static void
-add_symbols_from_enclosing_procs (struct obstack *obstackp,
+add_symbols_from_enclosing_procs (std::vector<struct block_symbol> &result,
 				  const lookup_name_info &lookup_name,
 				  domain_enum domain)
 {
@@ -5048,7 +5001,7 @@ symbols_are_identical_enums (const std::vector<struct block_symbol> &syms)
    debugging symbols)).  Modifies SYMS to squeeze out deleted entries.
    Returns the number of items in the modified list.  */
 
-static int
+static void
 remove_extra_symbols (std::vector<struct block_symbol> *syms)
 {
   int i, j;
@@ -5057,7 +5010,7 @@ remove_extra_symbols (std::vector<struct block_symbol> *syms)
      cannot be any extra symbol in that case.  But it's easy to
      handle, since we have nothing to do in that case.  */
   if (syms->size () < 2)
-    return syms->size ();
+    return;
 
   i = 0;
   while (i < syms->size ())
@@ -5122,8 +5075,6 @@ remove_extra_symbols (std::vector<struct block_symbol> *syms)
      isn't missing some choices that were identical and yet distinct.  */
   if (symbols_are_identical_enums (*syms))
     syms->resize (1);
-
-  return syms->size ();
 }
 
 /* Given a type that corresponds to a renaming entity, use the type name
@@ -5215,8 +5166,8 @@ old_renaming_is_invisible (const struct symbol *sym, const char *function_name)
    is not visible from the function associated with CURRENT_BLOCK or
    that is superfluous due to the presence of more specific renaming
    information.  Places surviving symbols in the initial entries of
-   SYMS and returns the number of surviving symbols.
-   
+   SYMS.
+
    Rationale:
    First, in cases where an object renaming is implemented as a
    reference variable, GNAT may produce both the actual reference
@@ -5248,7 +5199,7 @@ old_renaming_is_invisible (const struct symbol *sym, const char *function_name)
 	has been changed by an "Export" pragma.  As a consequence,
 	the user will be unable to print such rename entities.  */
 
-static int
+static void
 remove_irrelevant_renamings (std::vector<struct block_symbol> *syms,
 			     const struct block *current_block)
 {
@@ -5297,22 +5248,23 @@ remove_irrelevant_renamings (std::vector<struct block_symbol> *syms,
 	      (*syms)[k] = (*syms)[j];
 	      k += 1;
 	    }
-      return k;
+      syms->resize (k);
+      return;
     }
 
   /* Extract the function name associated to CURRENT_BLOCK.
      Abort if unable to do so.  */
 
   if (current_block == NULL)
-    return syms->size ();
+    return;
 
   current_function = block_linkage_function (current_block);
   if (current_function == NULL)
-    return syms->size ();
+    return;
 
   current_function_name = current_function->linkage_name ();
   if (current_function_name == NULL)
-    return syms->size ();
+    return;
 
   /* Check each of the symbols, and remove it from the list if it is
      a type corresponding to a renaming that is out of the scope of
@@ -5329,11 +5281,9 @@ remove_irrelevant_renamings (std::vector<struct block_symbol> *syms,
       else
 	i += 1;
     }
-
-  return syms->size ();
 }
 
-/* Add to OBSTACKP all symbols from BLOCK (and its super-blocks)
+/* Add to RESULT all symbols from BLOCK (and its super-blocks)
    whose name and domain match NAME and DOMAIN respectively.
    If no match was found, then extend the search to "enclosing"
    routines (in other words, if we're inside a nested function,
@@ -5341,10 +5291,10 @@ remove_irrelevant_renamings (std::vector<struct block_symbol> *syms,
    If WILD_MATCH_P is nonzero, perform the naming matching in
    "wild" mode (see function "wild_match" for more info).
 
-   Note: This function assumes that OBSTACKP has 0 (zero) element in it.  */
+   Note: This function assumes that RESULT has 0 (zero) element in it.  */
 
 static void
-ada_add_local_symbols (struct obstack *obstackp,
+ada_add_local_symbols (std::vector<struct block_symbol> &result,
 		       const lookup_name_info &lookup_name,
 		       const struct block *block, domain_enum domain)
 {
@@ -5353,11 +5303,10 @@ ada_add_local_symbols (struct obstack *obstackp,
   while (block != NULL)
     {
       block_depth += 1;
-      ada_add_block_symbols (obstackp, block, lookup_name, domain, NULL);
+      ada_add_block_symbols (result, block, lookup_name, domain, NULL);
 
       /* If we found a non-function match, assume that's the one.  */
-      if (is_nonfunction (defns_collected (obstackp, 0),
-			  num_defns_collected (obstackp)))
+      if (is_nonfunction (result))
 	return;
 
       block = BLOCK_SUPERBLOCK (block);
@@ -5365,8 +5314,8 @@ ada_add_local_symbols (struct obstack *obstackp,
 
   /* If no luck so far, try to find NAME as a local symbol in some lexically
      enclosing subprogram.  */
-  if (num_defns_collected (obstackp) == 0 && block_depth > 2)
-    add_symbols_from_enclosing_procs (obstackp, lookup_name, domain);
+  if (result.empty () && block_depth > 2)
+    add_symbols_from_enclosing_procs (result, lookup_name, domain);
 }
 
 /* An object of this type is used as the user_data argument when
@@ -5375,7 +5324,7 @@ ada_add_local_symbols (struct obstack *obstackp,
 struct match_data
 {
   struct objfile *objfile;
-  struct obstack *obstackp;
+  std::vector<struct block_symbol> *resultp;
   struct symbol *arg_sym;
   int found_sym;
 };
@@ -5399,7 +5348,7 @@ aux_add_nonlocal_symbols (struct block_symbol *bsym,
   if (sym == NULL)
     {
       if (!data->found_sym && data->arg_sym != NULL) 
-	add_defn_to_vec (data->obstackp,
+	add_defn_to_vec (*data->resultp,
 			 fixup_symbol_section (data->arg_sym, data->objfile),
 			 block);
       data->found_sym = 0;
@@ -5414,7 +5363,7 @@ aux_add_nonlocal_symbols (struct block_symbol *bsym,
       else
 	{
 	  data->found_sym = 1;
-	  add_defn_to_vec (data->obstackp,
+	  add_defn_to_vec (*data->resultp,
 			   fixup_symbol_section (sym, data->objfile),
 			   block);
 	}
@@ -5427,13 +5376,13 @@ aux_add_nonlocal_symbols (struct block_symbol *bsym,
    symbols to OBSTACKP.  Return whether we found such symbols.  */
 
 static int
-ada_add_block_renamings (struct obstack *obstackp,
+ada_add_block_renamings (std::vector<struct block_symbol> &result,
 			 const struct block *block,
 			 const lookup_name_info &lookup_name,
 			 domain_enum domain)
 {
   struct using_direct *renaming;
-  int defns_mark = num_defns_collected (obstackp);
+  int defns_mark = result.size ();
 
   symbol_name_matcher_ftype *name_match
     = ada_get_symbol_name_matcher (lookup_name);
@@ -5471,12 +5420,12 @@ ada_add_block_renamings (struct obstack *obstackp,
 	{
 	  lookup_name_info decl_lookup_name (renaming->declaration,
 					     lookup_name.match_type ());
-	  ada_add_all_symbols (obstackp, block, decl_lookup_name, domain,
+	  ada_add_all_symbols (result, block, decl_lookup_name, domain,
 			       1, NULL);
 	}
       renaming->searched = 0;
     }
-  return num_defns_collected (obstackp) != defns_mark;
+  return result.size () != defns_mark;
 }
 
 /* Implements compare_names, but only applying the comparision using
@@ -5579,14 +5528,14 @@ ada_lookup_name (const lookup_name_info &lookup_name)
    symbols otherwise.  */
 
 static void
-add_nonlocal_symbols (struct obstack *obstackp,
+add_nonlocal_symbols (std::vector<struct block_symbol> &result,
 		      const lookup_name_info &lookup_name,
 		      domain_enum domain, int global)
 {
   struct match_data data;
 
   memset (&data, 0, sizeof data);
-  data.obstackp = obstackp;
+  data.resultp = &result;
 
   bool is_wild_match = lookup_name.ada ().wild_match_p ();
 
@@ -5609,13 +5558,13 @@ add_nonlocal_symbols (struct obstack *obstackp,
 	  const struct block *global_block
 	    = BLOCKVECTOR_BLOCK (COMPUNIT_BLOCKVECTOR (cu), GLOBAL_BLOCK);
 
-	  if (ada_add_block_renamings (obstackp, global_block, lookup_name,
+	  if (ada_add_block_renamings (result, global_block, lookup_name,
 				       domain))
 	    data.found_sym = 1;
 	}
     }
 
-  if (num_defns_collected (obstackp) == 0 && global && !is_wild_match)
+  if (result.empty () && global && !is_wild_match)
     {
       const char *name = ada_lookup_name (lookup_name);
       std::string bracket_name = std::string ("<_ada_") + name + '>';
@@ -5649,7 +5598,7 @@ add_nonlocal_symbols (struct obstack *obstackp,
    to lookup global symbols.  */
 
 static void
-ada_add_all_symbols (struct obstack *obstackp,
+ada_add_all_symbols (std::vector<struct block_symbol> &result,
 		     const struct block *block,
 		     const lookup_name_info &lookup_name,
 		     domain_enum domain,
@@ -5676,15 +5625,15 @@ ada_add_all_symbols (struct obstack *obstackp,
   if (block != NULL)
     {
       if (full_search)
-	ada_add_local_symbols (obstackp, lookup_name, block, domain);
+	ada_add_local_symbols (result, lookup_name, block, domain);
       else
 	{
 	  /* In the !full_search case we're are being called by
 	     iterate_over_symbols, and we don't want to search
 	     superblocks.  */
-	  ada_add_block_symbols (obstackp, block, lookup_name, domain, NULL);
+	  ada_add_block_symbols (result, block, lookup_name, domain, NULL);
 	}
-      if (num_defns_collected (obstackp) > 0 || !full_search)
+      if (!result.empty () || !full_search)
 	return;
     }
 
@@ -5696,7 +5645,7 @@ ada_add_all_symbols (struct obstack *obstackp,
 			    domain, &sym, &block))
     {
       if (sym != NULL)
-	add_defn_to_vec (obstackp, sym, block);
+	add_defn_to_vec (result, sym, block);
       return;
     }
 
@@ -5705,21 +5654,20 @@ ada_add_all_symbols (struct obstack *obstackp,
 
   /* Search symbols from all global blocks.  */
  
-  add_nonlocal_symbols (obstackp, lookup_name, domain, 1);
+  add_nonlocal_symbols (result, lookup_name, domain, 1);
 
   /* Now add symbols from all per-file blocks if we've gotten no hits
      (not strictly correct, but perhaps better than an error).  */
 
-  if (num_defns_collected (obstackp) == 0)
-    add_nonlocal_symbols (obstackp, lookup_name, domain, 0);
+  if (result.empty ())
+    add_nonlocal_symbols (result, lookup_name, domain, 0);
 }
 
 /* Find symbols in DOMAIN matching LOOKUP_NAME, in BLOCK and, if FULL_SEARCH
-   is non-zero, enclosing scope and in global scopes, returning the number of
-   matches.
-   Fills *RESULTS with (SYM,BLOCK) tuples, indicating the symbols
-   found and the blocks and symbol tables (if any) in which they were
-   found.
+   is non-zero, enclosing scope and in global scopes.
+
+   Returns (SYM,BLOCK) tuples, indicating the symbols found and the
+   blocks and symbol tables (if any) in which they were found.
 
    When full_search is non-zero, any non-function/non-enumeral
    symbol match within the nest of blocks whose innermost member is BLOCK,
@@ -5730,55 +5678,44 @@ ada_add_all_symbols (struct obstack *obstackp,
    Names prefixed with "standard__" are handled specially: "standard__"
    is first stripped off, and only static and global symbols are searched.  */
 
-static int
+static std::vector<struct block_symbol>
 ada_lookup_symbol_list_worker (const lookup_name_info &lookup_name,
 			       const struct block *block,
 			       domain_enum domain,
-			       std::vector<struct block_symbol> *results,
 			       int full_search)
 {
   int syms_from_global_search;
-  int ndefns;
-  auto_obstack obstack;
+  std::vector<struct block_symbol> results;
 
-  ada_add_all_symbols (&obstack, block, lookup_name,
+  ada_add_all_symbols (results, block, lookup_name,
 		       domain, full_search, &syms_from_global_search);
 
-  ndefns = num_defns_collected (&obstack);
-
-  struct block_symbol *base = defns_collected (&obstack, 1);
-  for (int i = 0; i < ndefns; ++i)
-    results->push_back (base[i]);
+  remove_extra_symbols (&results);
 
-  ndefns = remove_extra_symbols (results);
-
-  if (ndefns == 0 && full_search && syms_from_global_search)
+  if (results.empty () && full_search && syms_from_global_search)
     cache_symbol (ada_lookup_name (lookup_name), domain, NULL, NULL);
 
-  if (ndefns == 1 && full_search && syms_from_global_search)
+  if (results.size () == 1 && full_search && syms_from_global_search)
     cache_symbol (ada_lookup_name (lookup_name), domain,
-		  (*results)[0].symbol, (*results)[0].block);
-
-  ndefns = remove_irrelevant_renamings (results, block);
+		  results[0].symbol, results[0].block);
 
-  return ndefns;
+  remove_irrelevant_renamings (&results, block);
+  return results;
 }
 
 /* Find symbols in DOMAIN matching NAME, in BLOCK and enclosing scope and
-   in global scopes, returning the number of matches, and filling *RESULTS
-   with (SYM,BLOCK) tuples.
+   in global scopes, returning (SYM,BLOCK) tuples.
 
    See ada_lookup_symbol_list_worker for further details.  */
 
-int
+std::vector<struct block_symbol>
 ada_lookup_symbol_list (const char *name, const struct block *block,
-			domain_enum domain,
-			std::vector<struct block_symbol> *results)
+			domain_enum domain)
 {
   symbol_name_match_type name_match_type = name_match_type_from_name (name);
   lookup_name_info lookup_name (name, name_match_type);
 
-  return ada_lookup_symbol_list_worker (lookup_name, block, domain, results, 1);
+  return ada_lookup_symbol_list_worker (lookup_name, block, domain, 1);
 }
 
 /* The result is as for ada_lookup_symbol_list with FULL_SEARCH set
@@ -5814,12 +5751,10 @@ struct block_symbol
 ada_lookup_symbol (const char *name, const struct block *block0,
 		   domain_enum domain)
 {
-  std::vector<struct block_symbol> candidates;
-  int n_candidates;
+  std::vector<struct block_symbol> candidates
+    = ada_lookup_symbol_list (name, block0, domain);
 
-  n_candidates = ada_lookup_symbol_list (name, block0, domain, &candidates);
-
-  if (n_candidates == 0)
+  if (candidates.empty ())
     return {};
 
   block_symbol info = candidates[0];
@@ -6078,12 +6013,11 @@ wild_match (const char *name, const char *patn)
     }
 }
 
-/* Add symbols from BLOCK matching LOOKUP_NAME in DOMAIN to vector
-   *defn_symbols, updating the list of symbols in OBSTACKP (if
+/* Add symbols from BLOCK matching LOOKUP_NAME in DOMAIN to RESULT (if
    necessary).  OBJFILE is the section containing BLOCK.  */
 
 static void
-ada_add_block_symbols (struct obstack *obstackp,
+ada_add_block_symbols (std::vector<struct block_symbol> &result,
 		       const struct block *block,
 		       const lookup_name_info &lookup_name,
 		       domain_enum domain, struct objfile *objfile)
@@ -6110,7 +6044,7 @@ ada_add_block_symbols (struct obstack *obstackp,
 	      else
 		{
 		  found_sym = 1;
-		  add_defn_to_vec (obstackp,
+		  add_defn_to_vec (result,
 				   fixup_symbol_section (sym, objfile),
 				   block);
 		}
@@ -6120,12 +6054,12 @@ ada_add_block_symbols (struct obstack *obstackp,
 
   /* Handle renamings.  */
 
-  if (ada_add_block_renamings (obstackp, block, lookup_name, domain))
+  if (ada_add_block_renamings (result, block, lookup_name, domain))
     found_sym = 1;
 
   if (!found_sym && arg_sym != NULL)
     {
-      add_defn_to_vec (obstackp,
+      add_defn_to_vec (result,
 		       fixup_symbol_section (arg_sym, objfile),
 		       block);
     }
@@ -6164,7 +6098,7 @@ ada_add_block_symbols (struct obstack *obstackp,
 		    else
 		      {
 			found_sym = 1;
-			add_defn_to_vec (obstackp,
+			add_defn_to_vec (result,
 					 fixup_symbol_section (sym, objfile),
 					 block);
 		      }
@@ -6177,7 +6111,7 @@ ada_add_block_symbols (struct obstack *obstackp,
 	 They aren't parameters, right?  */
       if (!found_sym && arg_sym != NULL)
 	{
-	  add_defn_to_vec (obstackp,
+	  add_defn_to_vec (result,
 			   fixup_symbol_section (arg_sym, objfile),
 			   block);
 	}
@@ -11317,12 +11251,12 @@ get_var_value (const char *name, const char *err_msg)
 
   lookup_name_info lookup_name (quoted_name, symbol_name_match_type::FULL);
 
-  std::vector<struct block_symbol> syms;
-  int nsyms = ada_lookup_symbol_list_worker (lookup_name,
-					     get_selected_block (0),
-					     VAR_DOMAIN, &syms, 1);
+  std::vector<struct block_symbol> syms
+    = ada_lookup_symbol_list_worker (lookup_name,
+				     get_selected_block (0),
+				     VAR_DOMAIN, 1);
 
-  if (nsyms != 1)
+  if (syms.size () != 1)
     {
       if (err_msg == NULL)
 	return 0;
@@ -13852,9 +13786,8 @@ class ada_language : public language_defn
 	 domain_enum domain,
 	 gdb::function_view<symbol_found_callback_ftype> callback) const override
   {
-    std::vector<struct block_symbol> results;
-
-    ada_lookup_symbol_list_worker (name, block, domain, &results, 0);
+    std::vector<struct block_symbol> results
+      = ada_lookup_symbol_list_worker (name, block, domain, 0);
     for (block_symbol &sym : results)
       {
 	if (!callback (&sym))
diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index dbf45a84928..8be4bf4ba69 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -218,9 +218,8 @@ extern const char *ada_decode_symbol (const struct general_symbol_info *);
 
 extern std::string ada_decode (const char*);
 
-extern int ada_lookup_symbol_list (const char *, const struct block *,
-				   domain_enum,
-				   std::vector<struct block_symbol> *);
+extern std::vector<struct block_symbol> ada_lookup_symbol_list
+     (const char *, const struct block *, domain_enum);
 
 extern struct block_symbol ada_lookup_symbol (const char *,
 					      const struct block *,
-- 
2.26.2

Re: [PATCH 3/4] Return a vector from ada_lookup_symbol_list

[Andrew Burgess]

* Tom Tromey <tromey@adacore.com> [2021-02-18 11:04:29 -0700]:

> This changes ada_lookup_symbol_list to return a std::vector, and
> changes various other helper functions to follow.  This simplifies the
> code, and makes it more type-safe (by using a vector where an obstack
> had been used).
> 
> gdb/ChangeLog
> 2021-02-18  Tom Tromey  <tromey@adacore.com>
> 
> 	* ada-lang.h (ada_lookup_symbol_list): Return a vector.
> 	* ada-lang.c (resolve_subexp): Update.
> 	(ada_resolve_function): Accept a vector.
> 	(is_nonfunction, add_defn_to_vec)
> 	(add_symbols_from_enclosing_procs): Likewise.
> 	(num_defns_collected, defns_collected): Remove.
> 	(remove_extra_symbols): Return a vector.
> 	(remove_irrelevant_renamings): Return void.
> 	(ada_add_local_symbols): Accept a vector.
> 	(struct match_data) <obstackp>: Remove.
> 	<resultp>: New member.
> 	(aux_add_nonlocal_symbols): Update.
> 	(ada_add_block_renamings, add_nonlocal_symbols)
> 	(ada_add_all_symbols): Accept a vector.
> 	(ada_lookup_symbol_list_worker, ada_lookup_symbol_list): Return a
> 	vector.
> 	(ada_lookup_symbol): Update.
> 	(ada_add_block_symbols): Accept a vector.
> 	(get_var_value, iterate_over_symbols): Update.
> 	* ada-exp.y (block_lookup, write_var_or_type, write_name_assoc):
> 	Update.
> ---
>  gdb/ChangeLog  |  24 ++++
>  gdb/ada-exp.y  |  35 +++---
>  gdb/ada-lang.c | 299 +++++++++++++++++++------------------------------
>  gdb/ada-lang.h |   5 +-
>  4 files changed, 158 insertions(+), 205 deletions(-)
> 
> diff --git a/gdb/ada-exp.y b/gdb/ada-exp.y
> index b55a0b434f7..a7e0ccbe5b1 100644
> --- a/gdb/ada-exp.y
> +++ b/gdb/ada-exp.y
> @@ -948,8 +948,6 @@ static const struct block*
>  block_lookup (const struct block *context, const char *raw_name)
>  {
>    const char *name;
> -  std::vector<struct block_symbol> syms;
> -  int nsyms;
>    struct symtab *symtab;
>    const struct block *result = NULL;
>  
> @@ -965,17 +963,18 @@ block_lookup (const struct block *context, const char *raw_name)
>        name = name_storage.c_str ();
>      }
>  
> -  nsyms = ada_lookup_symbol_list (name, context, VAR_DOMAIN, &syms);
> +  std::vector<struct block_symbol> syms
> +    = ada_lookup_symbol_list (name, context, VAR_DOMAIN);
>  
>    if (context == NULL
> -      && (nsyms == 0 || SYMBOL_CLASS (syms[0].symbol) != LOC_BLOCK))
> +      && (syms.empty () || SYMBOL_CLASS (syms[0].symbol) != LOC_BLOCK))
>      symtab = lookup_symtab (name);
>    else
>      symtab = NULL;
>  
>    if (symtab != NULL)
>      result = BLOCKVECTOR_BLOCK (SYMTAB_BLOCKVECTOR (symtab), STATIC_BLOCK);
> -  else if (nsyms == 0 || SYMBOL_CLASS (syms[0].symbol) != LOC_BLOCK)
> +  else if (syms.empty () || SYMBOL_CLASS (syms[0].symbol) != LOC_BLOCK)
>      {
>        if (context == NULL)
>  	error (_("No file or function \"%s\"."), raw_name);
> @@ -984,7 +983,7 @@ block_lookup (const struct block *context, const char *raw_name)
>      }
>    else
>      {
> -      if (nsyms > 1)
> +      if (syms.size () > 1)
>  	warning (_("Function name \"%s\" ambiguous here"), raw_name);
>        result = SYMBOL_BLOCK_VALUE (syms[0].symbol);
>      }
> @@ -1216,8 +1215,6 @@ write_var_or_type (struct parser_state *par_state,
>        tail_index = name_len;
>        while (tail_index > 0)
>  	{
> -	  int nsyms;
> -	  std::vector<struct block_symbol> syms;
>  	  struct symbol *type_sym;
>  	  struct symbol *renaming_sym;
>  	  const char* renaming;
> @@ -1226,15 +1223,15 @@ write_var_or_type (struct parser_state *par_state,
>  	  int terminator = encoded_name[tail_index];
>  
>  	  encoded_name[tail_index] = '\0';
> -	  nsyms = ada_lookup_symbol_list (encoded_name, block,
> -					  VAR_DOMAIN, &syms);
> +	  std::vector<struct block_symbol> syms
> +	    = ada_lookup_symbol_list (encoded_name, block, VAR_DOMAIN);
>  	  encoded_name[tail_index] = terminator;
>  
>  	  type_sym = select_possible_type_sym (syms);
>  
>  	  if (type_sym != NULL)
>  	    renaming_sym = type_sym;
> -	  else if (nsyms == 1)
> +	  else if (syms.size () == 1)
>  	    renaming_sym = syms[0].symbol;
>  	  else 
>  	    renaming_sym = NULL;
> @@ -1285,7 +1282,7 @@ write_var_or_type (struct parser_state *par_state,
>  		error (_("Invalid attempt to select from type: \"%s\"."),
>  		       name0.ptr);
>  	    }
> -	  else if (tail_index == name_len && nsyms == 0)
> +	  else if (tail_index == name_len && syms.empty ())
>  	    {
>  	      struct type *type = find_primitive_type (par_state,
>  						       encoded_name);
> @@ -1294,13 +1291,13 @@ write_var_or_type (struct parser_state *par_state,
>  		return type;
>  	    }
>  
> -	  if (nsyms == 1)
> +	  if (syms.size () == 1)
>  	    {
>  	      write_var_from_sym (par_state, syms[0].block, syms[0].symbol);
>  	      write_selectors (par_state, encoded_name + tail_index);
>  	      return NULL;
>  	    }
> -	  else if (nsyms == 0) 
> +	  else if (syms.empty ()) 

Could you clear up the trailing white space here please?

>  	    {
>  	      struct bound_minimal_symbol msym
>  		= ada_lookup_simple_minsym (encoded_name);
> @@ -1362,12 +1359,12 @@ write_name_assoc (struct parser_state *par_state, struct stoken name)
>  {
>    if (strchr (name.ptr, '.') == NULL)
>      {
> -      std::vector<struct block_symbol> syms;
> -      int nsyms = ada_lookup_symbol_list (name.ptr,
> -					  par_state->expression_context_block,
> -					  VAR_DOMAIN, &syms);
> +      std::vector<struct block_symbol> syms
> +	= ada_lookup_symbol_list (name.ptr,
> +				  par_state->expression_context_block,
> +				  VAR_DOMAIN);
>  
> -      if (nsyms != 1 || SYMBOL_CLASS (syms[0].symbol) == LOC_TYPEDEF)
> +      if (syms.size () != 1 || SYMBOL_CLASS (syms[0].symbol) == LOC_TYPEDEF)
>  	write_exp_op_with_string (par_state, OP_NAME, name);
>        else
>  	write_var_from_sym (par_state, syms[0].block, syms[0].symbol);
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index db4f3591131..1bc7b912e91 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -100,24 +100,22 @@ static int ada_args_match (struct symbol *, struct value **, int);
>  
>  static struct value *make_array_descriptor (struct type *, struct value *);
>  
> -static void ada_add_block_symbols (struct obstack *,
> +static void ada_add_block_symbols (std::vector<struct block_symbol> &,
>  				   const struct block *,
>  				   const lookup_name_info &lookup_name,
>  				   domain_enum, struct objfile *);
>  
> -static void ada_add_all_symbols (struct obstack *, const struct block *,
> +static void ada_add_all_symbols (std::vector<struct block_symbol> &,
> +				 const struct block *,
>  				 const lookup_name_info &lookup_name,
>  				 domain_enum, int, int *);
>  
> -static int is_nonfunction (struct block_symbol *, int);
> +static int is_nonfunction (const std::vector<struct block_symbol> &);
>  
> -static void add_defn_to_vec (struct obstack *, struct symbol *,
> +static void add_defn_to_vec (std::vector<struct block_symbol> &,
> +			     struct symbol *,
>  			     const struct block *);
>  
> -static int num_defns_collected (struct obstack *);
> -
> -static struct block_symbol *defns_collected (struct obstack *, int);
> -
>  static struct value *resolve_subexp (expression_up *, int *, int,
>  				     struct type *, int,
>  				     innermost_block_tracker *);
> @@ -205,7 +203,7 @@ static struct value *ada_search_struct_field (const char *, struct value *, int,
>  static int find_struct_field (const char *, struct type *, int,
>  			      struct type **, int *, int *, int *, int *);
>  
> -static int ada_resolve_function (struct block_symbol *, int,
> +static int ada_resolve_function (std::vector<struct block_symbol> &,
>  				 struct value **, int, const char *,
>  				 struct type *, int);
>  
> @@ -3653,15 +3651,9 @@ resolve_subexp (expression_up *expp, int *pos, int deprocedure_p,
>      case OP_VAR_VALUE:
>        if (SYMBOL_DOMAIN (exp->elts[pc + 2].symbol) == UNDEF_DOMAIN)
>  	{
> -	  std::vector<struct block_symbol> candidates;
> -	  int n_candidates;
> -
> -	  n_candidates =
> -	    ada_lookup_symbol_list (exp->elts[pc + 2].symbol->linkage_name (),
> -				    exp->elts[pc + 1].block, VAR_DOMAIN,
> -				    &candidates);
> -	  /* Paranoia.  */
> -	  candidates.resize (n_candidates);
> +	  std::vector<struct block_symbol> candidates
> +	    = ada_lookup_symbol_list (exp->elts[pc + 2].symbol->linkage_name (),
> +				      exp->elts[pc + 1].block, VAR_DOMAIN);
>  
>  	  if (std::any_of (candidates.begin (),
>  			   candidates.end (),
> @@ -3693,19 +3685,17 @@ resolve_subexp (expression_up *expp, int *pos, int deprocedure_p,
>  		    return SYMBOL_CLASS (sym.symbol) == LOC_TYPEDEF;
>  		  }),
>  		 candidates.end ());
> -	      n_candidates = candidates.size ();
>  	    }
>  
> -	  if (n_candidates == 0)
> +	  if (candidates.empty ())
>  	    error (_("No definition found for %s"),
>  		   exp->elts[pc + 2].symbol->print_name ());
> -	  else if (n_candidates == 1)
> +	  else if (candidates.size () == 1)
>  	    i = 0;
> -	  else if (deprocedure_p
> -		   && !is_nonfunction (candidates.data (), n_candidates))
> +	  else if (deprocedure_p && !is_nonfunction (candidates))
>  	    {
>  	      i = ada_resolve_function
> -		(candidates.data (), n_candidates, NULL, 0,
> +		(candidates, NULL, 0,
>  		 exp->elts[pc + 2].symbol->linkage_name (),
>  		 context_type, parse_completion);
>  	      if (i < 0)
> @@ -3716,7 +3706,7 @@ resolve_subexp (expression_up *expp, int *pos, int deprocedure_p,
>  	    {
>  	      printf_filtered (_("Multiple matches for %s\n"),
>  			       exp->elts[pc + 2].symbol->print_name ());
> -	      user_select_syms (candidates.data (), n_candidates, 1);
> +	      user_select_syms (candidates.data (), candidates.size (), 1);
>  	      i = 0;
>  	    }
>  
> @@ -3741,20 +3731,16 @@ resolve_subexp (expression_up *expp, int *pos, int deprocedure_p,
>  	if (exp->elts[pc + 3].opcode == OP_VAR_VALUE
>  	    && SYMBOL_DOMAIN (exp->elts[pc + 5].symbol) == UNDEF_DOMAIN)
>  	  {
> -	    std::vector<struct block_symbol> candidates;
> -	    int n_candidates;
> +	    std::vector<struct block_symbol> candidates
> +	      = ada_lookup_symbol_list (exp->elts[pc + 5].symbol->linkage_name (),
> +					exp->elts[pc + 4].block, VAR_DOMAIN);
>  
> -	    n_candidates =
> -	      ada_lookup_symbol_list (exp->elts[pc + 5].symbol->linkage_name (),
> -				      exp->elts[pc + 4].block, VAR_DOMAIN,
> -				      &candidates);
> -
> -	    if (n_candidates == 1)
> +	    if (candidates.size () == 1)
>  	      i = 0;
>  	    else
>  	      {
>  		i = ada_resolve_function
> -		  (candidates.data (), n_candidates,
> +		  (candidates,
>  		   argvec, nargs,
>  		   exp->elts[pc + 5].symbol->linkage_name (),
>  		   context_type, parse_completion);
> @@ -3792,15 +3778,11 @@ resolve_subexp (expression_up *expp, int *pos, int deprocedure_p,
>      case UNOP_ABS:
>        if (possible_user_operator_p (op, argvec))
>  	{
> -	  std::vector<struct block_symbol> candidates;
> -	  int n_candidates;
> -
> -	  n_candidates =
> -	    ada_lookup_symbol_list (ada_decoded_op_name (op),
> -				    NULL, VAR_DOMAIN,
> -				    &candidates);
> +	  std::vector<struct block_symbol> candidates
> +	    = ada_lookup_symbol_list (ada_decoded_op_name (op),
> +				      NULL, VAR_DOMAIN);
>  
> -	  i = ada_resolve_function (candidates.data (), n_candidates, argvec,
> +	  i = ada_resolve_function (candidates, argvec,
>  				    nargs, ada_decoded_op_name (op), NULL,
>  				    parse_completion);
>  	  if (i < 0)
> @@ -3966,8 +3948,8 @@ return_match (struct type *func_type, struct type *context_type)
>     the process; the index returned is for the modified vector.  */
>  
>  static int
> -ada_resolve_function (struct block_symbol syms[],
> -		      int nsyms, struct value **args, int nargs,
> +ada_resolve_function (std::vector<struct block_symbol> &syms,
> +		      struct value **args, int nargs,
>  		      const char *name, struct type *context_type,
>  		      int parse_completion)

The header comment for this function needs updating to remove the
reference to NSYMS.

>  {
> @@ -3981,7 +3963,7 @@ ada_resolve_function (struct block_symbol syms[],
>       where every function is accepted.  */
>    for (fallback = 0; m == 0 && fallback < 2; fallback++)
>      {
> -      for (k = 0; k < nsyms; k += 1)
> +      for (k = 0; k < syms.size (); k += 1)
>  	{
>  	  struct type *type = ada_check_typedef (SYMBOL_TYPE (syms[k].symbol));
>  
> @@ -4003,7 +3985,7 @@ ada_resolve_function (struct block_symbol syms[],
>    else if (m > 1 && !parse_completion)
>      {
>        printf_filtered (_("Multiple matches for %s\n"), name);
> -      user_select_syms (syms, m, 1);
> +      user_select_syms (syms.data (), m, 1);
>        return 0;
>      }
>    return 0;
> @@ -4737,14 +4719,12 @@ standard_lookup (const char *name, const struct block *block,
>     in the symbol fields of SYMS[0..N-1].  We treat enumerals as functions, 
>     since they contend in overloading in the same way.  */
>  static int
> -is_nonfunction (struct block_symbol syms[], int n)
> +is_nonfunction (const std::vector<struct block_symbol> &syms)

Again, the comment needs updating.

>  {
> -  int i;
> -
> -  for (i = 0; i < n; i += 1)
> -    if (SYMBOL_TYPE (syms[i].symbol)->code () != TYPE_CODE_FUNC
> -	&& (SYMBOL_TYPE (syms[i].symbol)->code () != TYPE_CODE_ENUM
> -	    || SYMBOL_CLASS (syms[i].symbol) != LOC_CONST))
> +  for (const block_symbol &sym : syms)
> +    if (SYMBOL_TYPE (sym.symbol)->code () != TYPE_CODE_FUNC
> +	&& (SYMBOL_TYPE (sym.symbol)->code () != TYPE_CODE_ENUM
> +	    || SYMBOL_CLASS (sym.symbol) != LOC_CONST))
>        return 1;
>  
>    return 0;
> @@ -4817,17 +4797,14 @@ lesseq_defined_than (struct symbol *sym0, struct symbol *sym1)
>      }
>  }
>  
> -/* Append (SYM,BLOCK,SYMTAB) to the end of the array of struct block_symbol
> -   records in OBSTACKP.  Do nothing if SYM is a duplicate.  */
> +/* Append (SYM,BLOCK) to the end of the array of struct block_symbol
> +   records in RESULT.  Do nothing if SYM is a duplicate.  */
>  
>  static void
> -add_defn_to_vec (struct obstack *obstackp,
> +add_defn_to_vec (std::vector<struct block_symbol> &result,
>  		 struct symbol *sym,
>  		 const struct block *block)
>  {
> -  int i;
> -  struct block_symbol *prevDefns = defns_collected (obstackp, 0);
> -
>    /* Do not try to complete stub types, as the debugger is probably
>       already scanning all symbols matching a certain name at the
>       time when this function is called.  Trying to replace the stub
> @@ -4837,46 +4814,22 @@ add_defn_to_vec (struct obstack *obstackp,
>       matches, with at least one of them complete.  It can then filter
>       out the stub ones if needed.  */
>  
> -  for (i = num_defns_collected (obstackp) - 1; i >= 0; i -= 1)
> +  for (int i = result.size () - 1; i >= 0; i -= 1)
>      {
> -      if (lesseq_defined_than (sym, prevDefns[i].symbol))
> +      if (lesseq_defined_than (sym, result[i].symbol))
>  	return;
> -      else if (lesseq_defined_than (prevDefns[i].symbol, sym))
> +      else if (lesseq_defined_than (result[i].symbol, sym))
>  	{
> -	  prevDefns[i].symbol = sym;
> -	  prevDefns[i].block = block;
> +	  result[i].symbol = sym;
> +	  result[i].block = block;
>  	  return;
>  	}
>      }
>  
> -  {
> -    struct block_symbol info;
> -
> -    info.symbol = sym;
> -    info.block = block;
> -    obstack_grow (obstackp, &info, sizeof (struct block_symbol));
> -  }
> -}
> -
> -/* Number of block_symbol structures currently collected in current vector in
> -   OBSTACKP.  */
> -
> -static int
> -num_defns_collected (struct obstack *obstackp)
> -{
> -  return obstack_object_size (obstackp) / sizeof (struct block_symbol);
> -}
> -
> -/* Vector of block_symbol structures currently collected in current vector in
> -   OBSTACKP.  If FINISH, close off the vector and return its final address.  */
> -
> -static struct block_symbol *
> -defns_collected (struct obstack *obstackp, int finish)
> -{
> -  if (finish)
> -    return (struct block_symbol *) obstack_finish (obstackp);
> -  else
> -    return (struct block_symbol *) obstack_base (obstackp);
> +  struct block_symbol info;
> +  info.symbol = sym;
> +  info.block = block;
> +  result.push_back (info);
>  }
>  
>  /* Return a bound minimal symbol matching NAME according to Ada
> @@ -4922,7 +4875,7 @@ ada_lookup_simple_minsym (const char *name)
>     with a wildcard prefix.  */
>  
>  static void
> -add_symbols_from_enclosing_procs (struct obstack *obstackp,
> +add_symbols_from_enclosing_procs (std::vector<struct block_symbol> &result,
>  				  const lookup_name_info &lookup_name,
>  				  domain_enum domain)

Comment again.


>  {
> @@ -5048,7 +5001,7 @@ symbols_are_identical_enums (const std::vector<struct block_symbol> &syms)
>     debugging symbols)).  Modifies SYMS to squeeze out deleted entries.
>     Returns the number of items in the modified list.  */
>  
> -static int
> +static void
>  remove_extra_symbols (std::vector<struct block_symbol> *syms)

And again.

>  {
>    int i, j;
> @@ -5057,7 +5010,7 @@ remove_extra_symbols (std::vector<struct block_symbol> *syms)
>       cannot be any extra symbol in that case.  But it's easy to
>       handle, since we have nothing to do in that case.  */
>    if (syms->size () < 2)
> -    return syms->size ();
> +    return;
>  
>    i = 0;
>    while (i < syms->size ())
> @@ -5122,8 +5075,6 @@ remove_extra_symbols (std::vector<struct block_symbol> *syms)
>       isn't missing some choices that were identical and yet distinct.  */
>    if (symbols_are_identical_enums (*syms))
>      syms->resize (1);
> -
> -  return syms->size ();
>  }
>  
>  /* Given a type that corresponds to a renaming entity, use the type name
> @@ -5215,8 +5166,8 @@ old_renaming_is_invisible (const struct symbol *sym, const char *function_name)
>     is not visible from the function associated with CURRENT_BLOCK or
>     that is superfluous due to the presence of more specific renaming
>     information.  Places surviving symbols in the initial entries of
> -   SYMS and returns the number of surviving symbols.
> -   
> +   SYMS.
> +
>     Rationale:
>     First, in cases where an object renaming is implemented as a
>     reference variable, GNAT may produce both the actual reference
> @@ -5248,7 +5199,7 @@ old_renaming_is_invisible (const struct symbol *sym, const char *function_name)
>  	has been changed by an "Export" pragma.  As a consequence,
>  	the user will be unable to print such rename entities.  */
>  
> -static int
> +static void
>  remove_irrelevant_renamings (std::vector<struct block_symbol> *syms,
>  			     const struct block *current_block)
>  {
> @@ -5297,22 +5248,23 @@ remove_irrelevant_renamings (std::vector<struct block_symbol> *syms,
>  	      (*syms)[k] = (*syms)[j];
>  	      k += 1;
>  	    }
> -      return k;
> +      syms->resize (k);
> +      return;
>      }
>  
>    /* Extract the function name associated to CURRENT_BLOCK.
>       Abort if unable to do so.  */
>  
>    if (current_block == NULL)
> -    return syms->size ();
> +    return;
>  
>    current_function = block_linkage_function (current_block);
>    if (current_function == NULL)
> -    return syms->size ();
> +    return;
>  
>    current_function_name = current_function->linkage_name ();
>    if (current_function_name == NULL)
> -    return syms->size ();
> +    return;
>  
>    /* Check each of the symbols, and remove it from the list if it is
>       a type corresponding to a renaming that is out of the scope of
> @@ -5329,11 +5281,9 @@ remove_irrelevant_renamings (std::vector<struct block_symbol> *syms,
>        else
>  	i += 1;
>      }
> -
> -  return syms->size ();
>  }
>  
> -/* Add to OBSTACKP all symbols from BLOCK (and its super-blocks)
> +/* Add to RESULT all symbols from BLOCK (and its super-blocks)
>     whose name and domain match NAME and DOMAIN respectively.
>     If no match was found, then extend the search to "enclosing"
>     routines (in other words, if we're inside a nested function,
> @@ -5341,10 +5291,10 @@ remove_irrelevant_renamings (std::vector<struct block_symbol> *syms,
>     If WILD_MATCH_P is nonzero, perform the naming matching in
>     "wild" mode (see function "wild_match" for more info).
>  
> -   Note: This function assumes that OBSTACKP has 0 (zero) element in it.  */
> +   Note: This function assumes that RESULT has 0 (zero) element in it.  */
>  
>  static void
> -ada_add_local_symbols (struct obstack *obstackp,
> +ada_add_local_symbols (std::vector<struct block_symbol> &result,
>  		       const lookup_name_info &lookup_name,
>  		       const struct block *block, domain_enum domain)
>  {
> @@ -5353,11 +5303,10 @@ ada_add_local_symbols (struct obstack *obstackp,
>    while (block != NULL)
>      {
>        block_depth += 1;
> -      ada_add_block_symbols (obstackp, block, lookup_name, domain, NULL);
> +      ada_add_block_symbols (result, block, lookup_name, domain, NULL);
>  
>        /* If we found a non-function match, assume that's the one.  */
> -      if (is_nonfunction (defns_collected (obstackp, 0),
> -			  num_defns_collected (obstackp)))
> +      if (is_nonfunction (result))
>  	return;
>  
>        block = BLOCK_SUPERBLOCK (block);
> @@ -5365,8 +5314,8 @@ ada_add_local_symbols (struct obstack *obstackp,
>  
>    /* If no luck so far, try to find NAME as a local symbol in some lexically
>       enclosing subprogram.  */
> -  if (num_defns_collected (obstackp) == 0 && block_depth > 2)
> -    add_symbols_from_enclosing_procs (obstackp, lookup_name, domain);
> +  if (result.empty () && block_depth > 2)
> +    add_symbols_from_enclosing_procs (result, lookup_name, domain);
>  }
>  
>  /* An object of this type is used as the user_data argument when
> @@ -5375,7 +5324,7 @@ ada_add_local_symbols (struct obstack *obstackp,
>  struct match_data
>  {
>    struct objfile *objfile;
> -  struct obstack *obstackp;
> +  std::vector<struct block_symbol> *resultp;
>    struct symbol *arg_sym;
>    int found_sym;
>  };

The immediately following comment on aux_add_nonlocal_symbols needs to
be updated to reflect this change in member naming.

While you're changing match_data anyway, you could (maybe) give the
fields default values, i.e. 'struct objfile *objfile = nullptr', this
would allow you to remove the memset at the place where the match_data
is setup.

> @@ -5399,7 +5348,7 @@ aux_add_nonlocal_symbols (struct block_symbol *bsym,
>    if (sym == NULL)
>      {
>        if (!data->found_sym && data->arg_sym != NULL) 
> -	add_defn_to_vec (data->obstackp,
> +	add_defn_to_vec (*data->resultp,
>  			 fixup_symbol_section (data->arg_sym, data->objfile),
>  			 block);
>        data->found_sym = 0;
> @@ -5414,7 +5363,7 @@ aux_add_nonlocal_symbols (struct block_symbol *bsym,
>        else
>  	{
>  	  data->found_sym = 1;
> -	  add_defn_to_vec (data->obstackp,
> +	  add_defn_to_vec (*data->resultp,
>  			   fixup_symbol_section (sym, data->objfile),
>  			   block);
>  	}
> @@ -5427,13 +5376,13 @@ aux_add_nonlocal_symbols (struct block_symbol *bsym,
>     symbols to OBSTACKP.  Return whether we found such symbols.  */
>  
>  static int
> -ada_add_block_renamings (struct obstack *obstackp,
> +ada_add_block_renamings (std::vector<struct block_symbol> &result,

Comment needs updating.

>  			 const struct block *block,
>  			 const lookup_name_info &lookup_name,
>  			 domain_enum domain)
>  {
>    struct using_direct *renaming;
> -  int defns_mark = num_defns_collected (obstackp);
> +  int defns_mark = result.size ();
>  
>    symbol_name_matcher_ftype *name_match
>      = ada_get_symbol_name_matcher (lookup_name);
> @@ -5471,12 +5420,12 @@ ada_add_block_renamings (struct obstack *obstackp,
>  	{
>  	  lookup_name_info decl_lookup_name (renaming->declaration,
>  					     lookup_name.match_type ());
> -	  ada_add_all_symbols (obstackp, block, decl_lookup_name, domain,
> +	  ada_add_all_symbols (result, block, decl_lookup_name, domain,
>  			       1, NULL);
>  	}
>        renaming->searched = 0;
>      }
> -  return num_defns_collected (obstackp) != defns_mark;
> +  return result.size () != defns_mark;
>  }
>  
>  /* Implements compare_names, but only applying the comparision using
> @@ -5579,14 +5528,14 @@ ada_lookup_name (const lookup_name_info &lookup_name)
>     symbols otherwise.  */
>  
>  static void
> -add_nonlocal_symbols (struct obstack *obstackp,
> +add_nonlocal_symbols (std::vector<struct block_symbol> &result,

Comment again.

>  		      const lookup_name_info &lookup_name,
>  		      domain_enum domain, int global)
>  {
>    struct match_data data;
>  
>    memset (&data, 0, sizeof data);

This is the memset you could remove if you add default values to
match_data.  Maybe match_data should have a constructor that takes the
vector pointer?  I don't think we ever create a match_data without
setting up the pointer, right?

> -  data.obstackp = obstackp;
> +  data.resultp = &result;
>  
>    bool is_wild_match = lookup_name.ada ().wild_match_p ();
>  
> @@ -5609,13 +5558,13 @@ add_nonlocal_symbols (struct obstack *obstackp,
>  	  const struct block *global_block
>  	    = BLOCKVECTOR_BLOCK (COMPUNIT_BLOCKVECTOR (cu), GLOBAL_BLOCK);
>  
> -	  if (ada_add_block_renamings (obstackp, global_block, lookup_name,
> +	  if (ada_add_block_renamings (result, global_block, lookup_name,
>  				       domain))
>  	    data.found_sym = 1;
>  	}
>      }
>  
> -  if (num_defns_collected (obstackp) == 0 && global && !is_wild_match)
> +  if (result.empty () && global && !is_wild_match)
>      {
>        const char *name = ada_lookup_name (lookup_name);
>        std::string bracket_name = std::string ("<_ada_") + name + '>';
> @@ -5649,7 +5598,7 @@ add_nonlocal_symbols (struct obstack *obstackp,
>     to lookup global symbols.  */
>  
>  static void
> -ada_add_all_symbols (struct obstack *obstackp,
> +ada_add_all_symbols (std::vector<struct block_symbol> &result,

Comment again.

With the comments updated, this looks like a good change.

Thanks,
Andrew


>  		     const struct block *block,
>  		     const lookup_name_info &lookup_name,
>  		     domain_enum domain,
> @@ -5676,15 +5625,15 @@ ada_add_all_symbols (struct obstack *obstackp,
>    if (block != NULL)
>      {
>        if (full_search)
> -	ada_add_local_symbols (obstackp, lookup_name, block, domain);
> +	ada_add_local_symbols (result, lookup_name, block, domain);
>        else
>  	{
>  	  /* In the !full_search case we're are being called by
>  	     iterate_over_symbols, and we don't want to search
>  	     superblocks.  */
> -	  ada_add_block_symbols (obstackp, block, lookup_name, domain, NULL);
> +	  ada_add_block_symbols (result, block, lookup_name, domain, NULL);
>  	}
> -      if (num_defns_collected (obstackp) > 0 || !full_search)
> +      if (!result.empty () || !full_search)
>  	return;
>      }
>  
> @@ -5696,7 +5645,7 @@ ada_add_all_symbols (struct obstack *obstackp,
>  			    domain, &sym, &block))
>      {
>        if (sym != NULL)
> -	add_defn_to_vec (obstackp, sym, block);
> +	add_defn_to_vec (result, sym, block);
>        return;
>      }
>  
> @@ -5705,21 +5654,20 @@ ada_add_all_symbols (struct obstack *obstackp,
>  
>    /* Search symbols from all global blocks.  */
>   
> -  add_nonlocal_symbols (obstackp, lookup_name, domain, 1);
> +  add_nonlocal_symbols (result, lookup_name, domain, 1);
>  
>    /* Now add symbols from all per-file blocks if we've gotten no hits
>       (not strictly correct, but perhaps better than an error).  */
>  
> -  if (num_defns_collected (obstackp) == 0)
> -    add_nonlocal_symbols (obstackp, lookup_name, domain, 0);
> +  if (result.empty ())
> +    add_nonlocal_symbols (result, lookup_name, domain, 0);
>  }
>  
>  /* Find symbols in DOMAIN matching LOOKUP_NAME, in BLOCK and, if FULL_SEARCH
> -   is non-zero, enclosing scope and in global scopes, returning the number of
> -   matches.
> -   Fills *RESULTS with (SYM,BLOCK) tuples, indicating the symbols
> -   found and the blocks and symbol tables (if any) in which they were
> -   found.
> +   is non-zero, enclosing scope and in global scopes.
> +
> +   Returns (SYM,BLOCK) tuples, indicating the symbols found and the
> +   blocks and symbol tables (if any) in which they were found.
>  
>     When full_search is non-zero, any non-function/non-enumeral
>     symbol match within the nest of blocks whose innermost member is BLOCK,
> @@ -5730,55 +5678,44 @@ ada_add_all_symbols (struct obstack *obstackp,
>     Names prefixed with "standard__" are handled specially: "standard__"
>     is first stripped off, and only static and global symbols are searched.  */
>  
> -static int
> +static std::vector<struct block_symbol>
>  ada_lookup_symbol_list_worker (const lookup_name_info &lookup_name,
>  			       const struct block *block,
>  			       domain_enum domain,
> -			       std::vector<struct block_symbol> *results,
>  			       int full_search)
>  {
>    int syms_from_global_search;
> -  int ndefns;
> -  auto_obstack obstack;
> +  std::vector<struct block_symbol> results;
>  
> -  ada_add_all_symbols (&obstack, block, lookup_name,
> +  ada_add_all_symbols (results, block, lookup_name,
>  		       domain, full_search, &syms_from_global_search);
>  
> -  ndefns = num_defns_collected (&obstack);
> -
> -  struct block_symbol *base = defns_collected (&obstack, 1);
> -  for (int i = 0; i < ndefns; ++i)
> -    results->push_back (base[i]);
> +  remove_extra_symbols (&results);
>  
> -  ndefns = remove_extra_symbols (results);
> -
> -  if (ndefns == 0 && full_search && syms_from_global_search)
> +  if (results.empty () && full_search && syms_from_global_search)
>      cache_symbol (ada_lookup_name (lookup_name), domain, NULL, NULL);
>  
> -  if (ndefns == 1 && full_search && syms_from_global_search)
> +  if (results.size () == 1 && full_search && syms_from_global_search)
>      cache_symbol (ada_lookup_name (lookup_name), domain,
> -		  (*results)[0].symbol, (*results)[0].block);
> -
> -  ndefns = remove_irrelevant_renamings (results, block);
> +		  results[0].symbol, results[0].block);
>  
> -  return ndefns;
> +  remove_irrelevant_renamings (&results, block);
> +  return results;
>  }
>  
>  /* Find symbols in DOMAIN matching NAME, in BLOCK and enclosing scope and
> -   in global scopes, returning the number of matches, and filling *RESULTS
> -   with (SYM,BLOCK) tuples.
> +   in global scopes, returning (SYM,BLOCK) tuples.
>  
>     See ada_lookup_symbol_list_worker for further details.  */
>  
> -int
> +std::vector<struct block_symbol>
>  ada_lookup_symbol_list (const char *name, const struct block *block,
> -			domain_enum domain,
> -			std::vector<struct block_symbol> *results)
> +			domain_enum domain)
>  {
>    symbol_name_match_type name_match_type = name_match_type_from_name (name);
>    lookup_name_info lookup_name (name, name_match_type);
>  
> -  return ada_lookup_symbol_list_worker (lookup_name, block, domain, results, 1);
> +  return ada_lookup_symbol_list_worker (lookup_name, block, domain, 1);
>  }
>  
>  /* The result is as for ada_lookup_symbol_list with FULL_SEARCH set
> @@ -5814,12 +5751,10 @@ struct block_symbol
>  ada_lookup_symbol (const char *name, const struct block *block0,
>  		   domain_enum domain)
>  {
> -  std::vector<struct block_symbol> candidates;
> -  int n_candidates;
> +  std::vector<struct block_symbol> candidates
> +    = ada_lookup_symbol_list (name, block0, domain);
>  
> -  n_candidates = ada_lookup_symbol_list (name, block0, domain, &candidates);
> -
> -  if (n_candidates == 0)
> +  if (candidates.empty ())
>      return {};
>  
>    block_symbol info = candidates[0];
> @@ -6078,12 +6013,11 @@ wild_match (const char *name, const char *patn)
>      }
>  }
>  
> -/* Add symbols from BLOCK matching LOOKUP_NAME in DOMAIN to vector
> -   *defn_symbols, updating the list of symbols in OBSTACKP (if
> +/* Add symbols from BLOCK matching LOOKUP_NAME in DOMAIN to RESULT (if
>     necessary).  OBJFILE is the section containing BLOCK.  */
>  
>  static void
> -ada_add_block_symbols (struct obstack *obstackp,
> +ada_add_block_symbols (std::vector<struct block_symbol> &result,
>  		       const struct block *block,
>  		       const lookup_name_info &lookup_name,
>  		       domain_enum domain, struct objfile *objfile)
> @@ -6110,7 +6044,7 @@ ada_add_block_symbols (struct obstack *obstackp,
>  	      else
>  		{
>  		  found_sym = 1;
> -		  add_defn_to_vec (obstackp,
> +		  add_defn_to_vec (result,
>  				   fixup_symbol_section (sym, objfile),
>  				   block);
>  		}
> @@ -6120,12 +6054,12 @@ ada_add_block_symbols (struct obstack *obstackp,
>  
>    /* Handle renamings.  */
>  
> -  if (ada_add_block_renamings (obstackp, block, lookup_name, domain))
> +  if (ada_add_block_renamings (result, block, lookup_name, domain))
>      found_sym = 1;
>  
>    if (!found_sym && arg_sym != NULL)
>      {
> -      add_defn_to_vec (obstackp,
> +      add_defn_to_vec (result,
>  		       fixup_symbol_section (arg_sym, objfile),
>  		       block);
>      }
> @@ -6164,7 +6098,7 @@ ada_add_block_symbols (struct obstack *obstackp,
>  		    else
>  		      {
>  			found_sym = 1;
> -			add_defn_to_vec (obstackp,
> +			add_defn_to_vec (result,
>  					 fixup_symbol_section (sym, objfile),
>  					 block);
>  		      }
> @@ -6177,7 +6111,7 @@ ada_add_block_symbols (struct obstack *obstackp,
>  	 They aren't parameters, right?  */
>        if (!found_sym && arg_sym != NULL)
>  	{
> -	  add_defn_to_vec (obstackp,
> +	  add_defn_to_vec (result,
>  			   fixup_symbol_section (arg_sym, objfile),
>  			   block);
>  	}
> @@ -11317,12 +11251,12 @@ get_var_value (const char *name, const char *err_msg)
>  
>    lookup_name_info lookup_name (quoted_name, symbol_name_match_type::FULL);
>  
> -  std::vector<struct block_symbol> syms;
> -  int nsyms = ada_lookup_symbol_list_worker (lookup_name,
> -					     get_selected_block (0),
> -					     VAR_DOMAIN, &syms, 1);
> +  std::vector<struct block_symbol> syms
> +    = ada_lookup_symbol_list_worker (lookup_name,
> +				     get_selected_block (0),
> +				     VAR_DOMAIN, 1);
>  
> -  if (nsyms != 1)
> +  if (syms.size () != 1)
>      {
>        if (err_msg == NULL)
>  	return 0;
> @@ -13852,9 +13786,8 @@ class ada_language : public language_defn
>  	 domain_enum domain,
>  	 gdb::function_view<symbol_found_callback_ftype> callback) const override
>    {
> -    std::vector<struct block_symbol> results;
> -
> -    ada_lookup_symbol_list_worker (name, block, domain, &results, 0);
> +    std::vector<struct block_symbol> results
> +      = ada_lookup_symbol_list_worker (name, block, domain, 0);
>      for (block_symbol &sym : results)
>        {
>  	if (!callback (&sym))
> diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
> index dbf45a84928..8be4bf4ba69 100644
> --- a/gdb/ada-lang.h
> +++ b/gdb/ada-lang.h
> @@ -218,9 +218,8 @@ extern const char *ada_decode_symbol (const struct general_symbol_info *);
>  
>  extern std::string ada_decode (const char*);
>  
> -extern int ada_lookup_symbol_list (const char *, const struct block *,
> -				   domain_enum,
> -				   std::vector<struct block_symbol> *);
> +extern std::vector<struct block_symbol> ada_lookup_symbol_list
> +     (const char *, const struct block *, domain_enum);
>  
>  extern struct block_symbol ada_lookup_symbol (const char *,
>  					      const struct block *,
> -- 
> 2.26.2
> 

Re: [PATCH 3/4] Return a vector from ada_lookup_symbol_list

[Tom Tromey]

>> -	  else if (nsyms == 0) 
>> +	  else if (syms.empty ()) 

Andrew> Could you clear up the trailing white space here please?

[... many more comments ...]

Oops, I totally neglected this review.
Sorry about that.  I will fix it up tomorrow.

Tom

Re: [PATCH 3/4] Return a vector from ada_lookup_symbol_list

[Tom Tromey]

Andrew> Could you clear up the trailing white space here please?

Tom> [... many more comments ...]

Tom> Oops, I totally neglected this review.
Tom> Sorry about that.  I will fix it up tomorrow.

I'm about to send it, and check it in.  Sorry about that.

Tom

[PATCH 4/4] Use std::string rather than grow_vect

[Tom Tromey]

This removes the "GROW_VECT" macro and helper function in favor of
simply using std::string in a few spots.

gdb/ChangeLog
2021-02-18  Tom Tromey  <tromey@adacore.com>

	* ada-lang.c (ada_fold_name, ada_variant_discrim_name)
	(ada_enum_name, scan_discrim_bound, to_fixed_range_type): Use
	std::string.
	(GROW_VECT): Remove.
	(grow_vect): Remove.
---
 gdb/ChangeLog  |   8 ++++
 gdb/ada-lang.c | 102 +++++++++++++------------------------------------
 2 files changed, 34 insertions(+), 76 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 1bc7b912e91..bf44868c2f9 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -475,29 +475,6 @@ add_angle_brackets (const char *str)
   return string_printf ("<%s>", str);
 }
 
-/* Assuming V points to an array of S objects,  make sure that it contains at
-   least M objects, updating V and S as necessary.  */
-
-#define GROW_VECT(v, s, m)                                    \
-   if ((s) < (m)) (v) = (char *) grow_vect (v, &(s), m, sizeof *(v));
-
-/* Assuming VECT points to an array of *SIZE objects of size
-   ELEMENT_SIZE, grow it to contain at least MIN_SIZE objects,
-   updating *SIZE as necessary and returning the (new) array.  */
-
-static void *
-grow_vect (void *vect, size_t *size, size_t min_size, int element_size)
-{
-  if (*size < min_size)
-    {
-      *size *= 2;
-      if (*size < min_size)
-	*size = min_size;
-      vect = xrealloc (vect, *size * element_size);
-    }
-  return vect;
-}
-
 /* True (non-zero) iff TARGET matches FIELD_NAME up to any trailing
    suffix of FIELD_NAME beginning "___".  */
 
@@ -961,30 +938,21 @@ ada_encode (const char *decoded)
    quotes, unfolded, but with the quotes stripped away.  Result good
    to next call.  */
 
-static char *
+static const char *
 ada_fold_name (gdb::string_view name)
 {
-  static char *fold_buffer = NULL;
-  static size_t fold_buffer_size = 0;
-
-  int len = name.size ();
-  GROW_VECT (fold_buffer, fold_buffer_size, len + 1);
+  static std::string fold_storage;
 
   if (name[0] == '\'')
-    {
-      strncpy (fold_buffer, name.data () + 1, len - 2);
-      fold_buffer[len - 2] = '\000';
-    }
+    fold_storage = to_string (name.substr (1, name.size () - 2));
   else
     {
-      int i;
-
-      for (i = 0; i < len; i += 1)
-	fold_buffer[i] = tolower (name[i]);
-      fold_buffer[i] = '\0';
+      fold_storage = to_string (name);
+      for (int i = 0; i < name.size (); i += 1)
+	fold_storage[i] = tolower (fold_storage[i]);
     }
 
-  return fold_buffer;
+  return fold_storage.c_str ();
 }
 
 /* Return nonzero if C is either a digit or a lowercase alphabet character.  */
@@ -6693,8 +6661,7 @@ ada_is_others_clause (struct type *type, int field_num)
 const char *
 ada_variant_discrim_name (struct type *type0)
 {
-  static char *result = NULL;
-  static size_t result_len = 0;
+  static std::string result;
   struct type *type;
   const char *name;
   const char *discrim_end;
@@ -6730,10 +6697,8 @@ ada_variant_discrim_name (struct type *type0)
 	break;
     }
 
-  GROW_VECT (result, result_len, discrim_end - discrim_start + 1);
-  strncpy (result, discrim_start, discrim_end - discrim_start);
-  result[discrim_end - discrim_start] = '\0';
-  return result;
+  result = std::string (discrim_start, discrim_end - discrim_start);
+  return result.c_str ();
 }
 
 /* Scan STR for a subtype-encoded number, beginning at position K.
@@ -9046,8 +9011,7 @@ ada_aligned_value_addr (struct type *type, const gdb_byte *valaddr)
 const char *
 ada_enum_name (const char *name)
 {
-  static char *result;
-  static size_t result_len = 0;
+  static std::string storage;
   const char *tmp;
 
   /* First, unqualify the enumeration name:
@@ -9086,22 +9050,20 @@ ada_enum_name (const char *name)
 		|| (name[1] >= 'a' && name[1] <= 'z'))
 	       && name[2] == '\0')
 	{
-	  GROW_VECT (result, result_len, 4);
-	  xsnprintf (result, result_len, "'%c'", name[1]);
-	  return result;
+	  storage = string_printf ("'%c'", name[1]);
+	  return storage.c_str ();
 	}
       else
 	return name;
 
-      GROW_VECT (result, result_len, 16);
       if (isascii (v) && isprint (v))
-	xsnprintf (result, result_len, "'%c'", v);
+	storage = string_printf ("'%c'", v);
       else if (name[1] == 'U')
-	xsnprintf (result, result_len, "[\"%02x\"]", v);
+	storage = string_printf ("[\"%02x\"]", v);
       else
-	xsnprintf (result, result_len, "[\"%04x\"]", v);
+	storage = string_printf ("[\"%04x\"]", v);
 
-      return result;
+      return storage.c_str ();
     }
   else
     {
@@ -9110,10 +9072,8 @@ ada_enum_name (const char *name)
 	tmp = strstr (name, "$");
       if (tmp != NULL)
 	{
-	  GROW_VECT (result, result_len, tmp - name + 1);
-	  strncpy (result, name, tmp - name);
-	  result[tmp - name] = '\0';
-	  return result;
+	  storage = std::string (name, tmp - name);
+	  return storage.c_str ();
 	}
 
       return name;
@@ -11202,8 +11162,7 @@ static int
 scan_discrim_bound (const char *str, int k, struct value *dval, LONGEST * px,
 		    int *pnew_k)
 {
-  static char *bound_buffer = NULL;
-  static size_t bound_buffer_len = 0;
+  static std::string storage;
   const char *pstart, *pend, *bound;
   struct value *bound_val;
 
@@ -11222,11 +11181,8 @@ scan_discrim_bound (const char *str, int k, struct value *dval, LONGEST * px,
       int len = pend - pstart;
 
       /* Strip __ and beyond.  */
-      GROW_VECT (bound_buffer, bound_buffer_len, len + 1);
-      strncpy (bound_buffer, pstart, len);
-      bound_buffer[len] = '\0';
-
-      bound = bound_buffer;
+      storage = std::string (pstart, len);
+      bound = storage.c_str ();
       k = pend - str;
     }
 
@@ -11323,18 +11279,12 @@ to_fixed_range_type (struct type *raw_type, struct value *dval)
     }
   else
     {
-      static char *name_buf = NULL;
-      static size_t name_len = 0;
       int prefix_len = subtype_info - name;
       LONGEST L, U;
       struct type *type;
       const char *bounds_str;
       int n;
 
-      GROW_VECT (name_buf, name_len, prefix_len + 5);
-      strncpy (name_buf, name, prefix_len);
-      name_buf[prefix_len] = '\0';
-
       subtype_info += 5;
       bounds_str = strchr (subtype_info, '_');
       n = 1;
@@ -11352,8 +11302,8 @@ to_fixed_range_type (struct type *raw_type, struct value *dval)
 	}
       else
 	{
-	  strcpy (name_buf + prefix_len, "___L");
-	  if (!get_int_var_value (name_buf, L))
+	  std::string name_buf = std::string (name, prefix_len) + "___L";
+	  if (!get_int_var_value (name_buf.c_str (), L))
 	    {
 	      lim_warning (_("Unknown lower bound, using 1."));
 	      L = 1;
@@ -11368,8 +11318,8 @@ to_fixed_range_type (struct type *raw_type, struct value *dval)
 	}
       else
 	{
-	  strcpy (name_buf + prefix_len, "___U");
-	  if (!get_int_var_value (name_buf, U))
+	  std::string name_buf = std::string (name, prefix_len) + "___U";
+	  if (!get_int_var_value (name_buf.c_str (), U))
 	    {
 	      lim_warning (_("Unknown upper bound, using %ld."), (long) L);
 	      U = L;
-- 
2.26.2

Re: [PATCH 4/4] Use std::string rather than grow_vect

[Andrew Burgess]

* Tom Tromey <tromey@adacore.com> [2021-02-18 11:04:30 -0700]:

> This removes the "GROW_VECT" macro and helper function in favor of
> simply using std::string in a few spots.
> 
> gdb/ChangeLog
> 2021-02-18  Tom Tromey  <tromey@adacore.com>
> 
> 	* ada-lang.c (ada_fold_name, ada_variant_discrim_name)
> 	(ada_enum_name, scan_discrim_bound, to_fixed_range_type): Use
> 	std::string.
> 	(GROW_VECT): Remove.
> 	(grow_vect): Remove.

LGTM.

Thanks,
Andrew

> ---
>  gdb/ChangeLog  |   8 ++++
>  gdb/ada-lang.c | 102 +++++++++++++------------------------------------
>  2 files changed, 34 insertions(+), 76 deletions(-)
> 
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 1bc7b912e91..bf44868c2f9 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -475,29 +475,6 @@ add_angle_brackets (const char *str)
>    return string_printf ("<%s>", str);
>  }
>  
> -/* Assuming V points to an array of S objects,  make sure that it contains at
> -   least M objects, updating V and S as necessary.  */
> -
> -#define GROW_VECT(v, s, m)                                    \
> -   if ((s) < (m)) (v) = (char *) grow_vect (v, &(s), m, sizeof *(v));
> -
> -/* Assuming VECT points to an array of *SIZE objects of size
> -   ELEMENT_SIZE, grow it to contain at least MIN_SIZE objects,
> -   updating *SIZE as necessary and returning the (new) array.  */
> -
> -static void *
> -grow_vect (void *vect, size_t *size, size_t min_size, int element_size)
> -{
> -  if (*size < min_size)
> -    {
> -      *size *= 2;
> -      if (*size < min_size)
> -	*size = min_size;
> -      vect = xrealloc (vect, *size * element_size);
> -    }
> -  return vect;
> -}
> -
>  /* True (non-zero) iff TARGET matches FIELD_NAME up to any trailing
>     suffix of FIELD_NAME beginning "___".  */
>  
> @@ -961,30 +938,21 @@ ada_encode (const char *decoded)
>     quotes, unfolded, but with the quotes stripped away.  Result good
>     to next call.  */
>  
> -static char *
> +static const char *
>  ada_fold_name (gdb::string_view name)
>  {
> -  static char *fold_buffer = NULL;
> -  static size_t fold_buffer_size = 0;
> -
> -  int len = name.size ();
> -  GROW_VECT (fold_buffer, fold_buffer_size, len + 1);
> +  static std::string fold_storage;
>  
>    if (name[0] == '\'')
> -    {
> -      strncpy (fold_buffer, name.data () + 1, len - 2);
> -      fold_buffer[len - 2] = '\000';
> -    }
> +    fold_storage = to_string (name.substr (1, name.size () - 2));
>    else
>      {
> -      int i;
> -
> -      for (i = 0; i < len; i += 1)
> -	fold_buffer[i] = tolower (name[i]);
> -      fold_buffer[i] = '\0';
> +      fold_storage = to_string (name);
> +      for (int i = 0; i < name.size (); i += 1)
> +	fold_storage[i] = tolower (fold_storage[i]);
>      }
>  
> -  return fold_buffer;
> +  return fold_storage.c_str ();
>  }
>  
>  /* Return nonzero if C is either a digit or a lowercase alphabet character.  */
> @@ -6693,8 +6661,7 @@ ada_is_others_clause (struct type *type, int field_num)
>  const char *
>  ada_variant_discrim_name (struct type *type0)
>  {
> -  static char *result = NULL;
> -  static size_t result_len = 0;
> +  static std::string result;
>    struct type *type;
>    const char *name;
>    const char *discrim_end;
> @@ -6730,10 +6697,8 @@ ada_variant_discrim_name (struct type *type0)
>  	break;
>      }
>  
> -  GROW_VECT (result, result_len, discrim_end - discrim_start + 1);
> -  strncpy (result, discrim_start, discrim_end - discrim_start);
> -  result[discrim_end - discrim_start] = '\0';
> -  return result;
> +  result = std::string (discrim_start, discrim_end - discrim_start);
> +  return result.c_str ();
>  }
>  
>  /* Scan STR for a subtype-encoded number, beginning at position K.
> @@ -9046,8 +9011,7 @@ ada_aligned_value_addr (struct type *type, const gdb_byte *valaddr)
>  const char *
>  ada_enum_name (const char *name)
>  {
> -  static char *result;
> -  static size_t result_len = 0;
> +  static std::string storage;
>    const char *tmp;
>  
>    /* First, unqualify the enumeration name:
> @@ -9086,22 +9050,20 @@ ada_enum_name (const char *name)
>  		|| (name[1] >= 'a' && name[1] <= 'z'))
>  	       && name[2] == '\0')
>  	{
> -	  GROW_VECT (result, result_len, 4);
> -	  xsnprintf (result, result_len, "'%c'", name[1]);
> -	  return result;
> +	  storage = string_printf ("'%c'", name[1]);
> +	  return storage.c_str ();
>  	}
>        else
>  	return name;
>  
> -      GROW_VECT (result, result_len, 16);
>        if (isascii (v) && isprint (v))
> -	xsnprintf (result, result_len, "'%c'", v);
> +	storage = string_printf ("'%c'", v);
>        else if (name[1] == 'U')
> -	xsnprintf (result, result_len, "[\"%02x\"]", v);
> +	storage = string_printf ("[\"%02x\"]", v);
>        else
> -	xsnprintf (result, result_len, "[\"%04x\"]", v);
> +	storage = string_printf ("[\"%04x\"]", v);
>  
> -      return result;
> +      return storage.c_str ();
>      }
>    else
>      {
> @@ -9110,10 +9072,8 @@ ada_enum_name (const char *name)
>  	tmp = strstr (name, "$");
>        if (tmp != NULL)
>  	{
> -	  GROW_VECT (result, result_len, tmp - name + 1);
> -	  strncpy (result, name, tmp - name);
> -	  result[tmp - name] = '\0';
> -	  return result;
> +	  storage = std::string (name, tmp - name);
> +	  return storage.c_str ();
>  	}
>  
>        return name;
> @@ -11202,8 +11162,7 @@ static int
>  scan_discrim_bound (const char *str, int k, struct value *dval, LONGEST * px,
>  		    int *pnew_k)
>  {
> -  static char *bound_buffer = NULL;
> -  static size_t bound_buffer_len = 0;
> +  static std::string storage;
>    const char *pstart, *pend, *bound;
>    struct value *bound_val;
>  
> @@ -11222,11 +11181,8 @@ scan_discrim_bound (const char *str, int k, struct value *dval, LONGEST * px,
>        int len = pend - pstart;
>  
>        /* Strip __ and beyond.  */
> -      GROW_VECT (bound_buffer, bound_buffer_len, len + 1);
> -      strncpy (bound_buffer, pstart, len);
> -      bound_buffer[len] = '\0';
> -
> -      bound = bound_buffer;
> +      storage = std::string (pstart, len);
> +      bound = storage.c_str ();
>        k = pend - str;
>      }
>  
> @@ -11323,18 +11279,12 @@ to_fixed_range_type (struct type *raw_type, struct value *dval)
>      }
>    else
>      {
> -      static char *name_buf = NULL;
> -      static size_t name_len = 0;
>        int prefix_len = subtype_info - name;
>        LONGEST L, U;
>        struct type *type;
>        const char *bounds_str;
>        int n;
>  
> -      GROW_VECT (name_buf, name_len, prefix_len + 5);
> -      strncpy (name_buf, name, prefix_len);
> -      name_buf[prefix_len] = '\0';
> -
>        subtype_info += 5;
>        bounds_str = strchr (subtype_info, '_');
>        n = 1;
> @@ -11352,8 +11302,8 @@ to_fixed_range_type (struct type *raw_type, struct value *dval)
>  	}
>        else
>  	{
> -	  strcpy (name_buf + prefix_len, "___L");
> -	  if (!get_int_var_value (name_buf, L))
> +	  std::string name_buf = std::string (name, prefix_len) + "___L";
> +	  if (!get_int_var_value (name_buf.c_str (), L))
>  	    {
>  	      lim_warning (_("Unknown lower bound, using 1."));
>  	      L = 1;
> @@ -11368,8 +11318,8 @@ to_fixed_range_type (struct type *raw_type, struct value *dval)
>  	}
>        else
>  	{
> -	  strcpy (name_buf + prefix_len, "___U");
> -	  if (!get_int_var_value (name_buf, U))
> +	  std::string name_buf = std::string (name, prefix_len) + "___U";
> +	  if (!get_int_var_value (name_buf.c_str (), U))
>  	    {
>  	      lim_warning (_("Unknown upper bound, using %ld."), (long) L);
>  	      U = L;
> -- 
> 2.26.2
> 

Re: [PATCH 0/4] Minor C++-ifications for Ada

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

Tom> This short series C++-ifies the Ada code a bit.  The main motivation
Tom> for this is simplification; overall this series reduces the number of
Tom> lines of code in gdb.  It also replaces some manual management with
Tom> automatic management.

Tom> Tested on x86-64 Fedora 32.

I'm checking these in now.

Tom