diff mbox

[3/6] Eliminate make_symbol_overload_list-related globals & cleanup

Message ID f4ea23e1-bfb5-1258-84e9-21d9da63e475@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves Nov. 21, 2018, 12:47 p.m. UTC 
  On 10/17/2018 06:25 PM, Simon Marchi wrote:
> On 2018-10-15 11:11 a.m., Pedro Alves wrote:
>> This gets rid of a few globals and a cleanup.
>>
>> make_symbol_overload_list & friends currently maintain a global
>> open-coded vector.  Reimplement that with a std::vector, trickled down
>> through the functions.  Rename a few functions from "make_" to "add_"
>> for clarity.
> 
> LGTM, my only comment would be to avoid non-const passing by reference, as in
> the previous patch.
Done.  Here's what I merged.

From 0891c3cc132495ad7b323896efae4f91eca87c6c Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 21 Nov 2018 11:55:13 +0000
Subject: [PATCH 3/6] Eliminate make_symbol_overload_list-related globals &
 cleanup

This gets rid of a few globals and a cleanup.

make_symbol_overload_list & friends currently maintain a global
open-coded vector.  Reimplement that with a std::vector, trickled down
through the functions.  Rename a few functions from "make_" to "add_"
for clarity.

gdb/ChangeLog:
2018-11-21  Pedro Alves  <palves@redhat.com>

	* cp-support.c (sym_return_val_size, sym_return_val_index)
	(sym_return_val): Delete.
	(overload_list_add_symbol): Add std::vector parameter.  Adjust to
	add to the vector.
	(make_symbol_overload_list): Adjust to return a std::vector
	instead of maintaining a global open coded vector.
	(make_symbol_overload_list_block): Add std::vector parameter.
	(make_symbol_overload_list_block): Rename to ...
	(add_symbol_overload_list_block): ... this and add std::vector
	parameter.
	(make_symbol_overload_list_namespace): Rename to ...
	(add_symbol_overload_list_namespace): ... this and add std::vector
	parameter.
	(make_symbol_overload_list_adl_namespace): Rename to ...
	(add_symbol_overload_list_adl_namespace): ... this and add
	std::vector parameter.
	(make_symbol_overload_list_adl): Delete.
	(add_symbol_overload_list_adl): New.
	(make_symbol_overload_list_using): Rename to ...
	(add_symbol_overload_list_using): ... this and add std::vector
	parameter.
	(make_symbol_overload_list_qualified): Rename to ...
	(add_symbol_overload_list_qualified): ... this and add std::vector
	parameter.
	* cp-support.h: Include "common/array-view.h" and <vector>.
	(make_symbol_overload_list): Change return type to std::vector.
	(make_symbol_overload_list_adl): Delete declaration.
	(add_symbol_overload_list_adl): New declaration.
	* valops.c (find_overload_match): Local 'oload_syms' now a
	std::vector.
	(find_oload_champ_namespace): 'oload_syms' parameter now a
	std::vector pointer.
	(find_oload_champ_namespace_loop): 'oload_syms' parameter now a
	std::vector pointer.  Adjust to new make_symbol_overload_list
	interface.
---
 gdb/ChangeLog    |  38 +++++++++++++++
 gdb/cp-support.c | 142 ++++++++++++++++++++++++-------------------------------
 gdb/cp-support.h |  13 +++--
 gdb/valops.c     |  37 ++++++---------
 4 files changed, 124 insertions(+), 106 deletions(-)
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 27da6431b3..e808dd5dab 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,41 @@ 
+2018-11-21  Pedro Alves  <palves@redhat.com>
+
+	* cp-support.c (sym_return_val_size, sym_return_val_index)
+	(sym_return_val): Delete.
+	(overload_list_add_symbol): Add std::vector parameter.  Adjust to
+	add to the vector.
+	(make_symbol_overload_list): Adjust to return a std::vector
+	instead of maintaining a global open coded vector.
+	(make_symbol_overload_list_block): Add std::vector parameter.
+	(make_symbol_overload_list_block): Rename to ...
+	(add_symbol_overload_list_block): ... this and add std::vector
+	parameter.
+	(make_symbol_overload_list_namespace): Rename to ...
+	(add_symbol_overload_list_namespace): ... this and add std::vector
+	parameter.
+	(make_symbol_overload_list_adl_namespace): Rename to ...
+	(add_symbol_overload_list_adl_namespace): ... this and add
+	std::vector parameter.
+	(make_symbol_overload_list_adl): Delete.
+	(add_symbol_overload_list_adl): New.
+	(make_symbol_overload_list_using): Rename to ...
+	(add_symbol_overload_list_using): ... this and add std::vector
+	parameter.
+	(make_symbol_overload_list_qualified): Rename to ...
+	(add_symbol_overload_list_qualified): ... this and add std::vector
+	parameter.
+	* cp-support.h: Include "common/array-view.h" and <vector>.
+	(make_symbol_overload_list): Change return type to std::vector.
+	(make_symbol_overload_list_adl): Delete declaration.
+	(add_symbol_overload_list_adl): New declaration.
+	* valops.c (find_overload_match): Local 'oload_syms' now a
+	std::vector.
+	(find_oload_champ_namespace): 'oload_syms' parameter now a
+	std::vector pointer.
+	(find_oload_champ_namespace_loop): 'oload_syms' parameter now a
+	std::vector pointer.  Adjust to new make_symbol_overload_list
+	interface.
+
 2018-11-21  Pedro Alves  <palves@redhat.com>
 
 	* common/array-view.h (array_view::splice(size_type, size_t)): New.
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 25c887035c..1f95253cb0 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -48,19 +48,19 @@  static unsigned int cp_find_first_component_aux (const char *name,
 
 static void demangled_name_complaint (const char *name);
 
-/* Functions/variables related to overload resolution.  */
-
-static int sym_return_val_size = -1;
-static int sym_return_val_index;
-static struct symbol **sym_return_val;
+/* Functions related to overload resolution.  */
 
 static void overload_list_add_symbol (struct symbol *sym,
-				      const char *oload_name);
+				      const char *oload_name,
+				      std::vector<symbol *> *overload_list);
 
-static void make_symbol_overload_list_using (const char *func_name,
-					     const char *the_namespace);
+static void add_symbol_overload_list_using
+  (const char *func_name, const char *the_namespace,
+   std::vector<symbol *> *overload_list);
 
-static void make_symbol_overload_list_qualified (const char *func_name);
+static void add_symbol_overload_list_qualified
+  (const char *func_name,
+   std::vector<symbol *> *overload_list);
 
 /* The list of "maint cplus" commands.  */
 
@@ -1137,30 +1137,28 @@  cp_entire_prefix_len (const char *name)
 /* Overload resolution functions.  */
 
 /* Test to see if SYM is a symbol that we haven't seen corresponding
-   to a function named OLOAD_NAME.  If so, add it to the current
-   completion list.  */
+   to a function named OLOAD_NAME.  If so, add it to
+   OVERLOAD_LIST.  */
 
 static void
 overload_list_add_symbol (struct symbol *sym,
-			  const char *oload_name)
+			  const char *oload_name,
+			  std::vector<symbol *> *overload_list)
 {
-  int newsize;
-  int i;
-  gdb::unique_xmalloc_ptr<char> sym_name;
-
   /* If there is no type information, we can't do anything, so
      skip.  */
   if (SYMBOL_TYPE (sym) == NULL)
     return;
 
   /* skip any symbols that we've already considered.  */
-  for (i = 0; i < sym_return_val_index; ++i)
+  for (symbol *listed_sym : *overload_list)
     if (strcmp (SYMBOL_LINKAGE_NAME (sym),
-		SYMBOL_LINKAGE_NAME (sym_return_val[i])) == 0)
+		SYMBOL_LINKAGE_NAME (listed_sym)) == 0)
       return;
 
   /* Get the demangled name without parameters */
-  sym_name = cp_remove_params (SYMBOL_NATURAL_NAME (sym));
+  gdb::unique_xmalloc_ptr<char> sym_name
+    = cp_remove_params (SYMBOL_NATURAL_NAME (sym));
   if (!sym_name)
     return;
 
@@ -1168,36 +1166,22 @@  overload_list_add_symbol (struct symbol *sym,
   if (strcmp (sym_name.get (), oload_name) != 0)
     return;
 
-  /* We have a match for an overload instance, so add SYM to the
-     current list of overload instances */
-  if (sym_return_val_index + 3 > sym_return_val_size)
-    {
-      newsize = (sym_return_val_size *= 2) * sizeof (struct symbol *);
-      sym_return_val = (struct symbol **)
-	xrealloc ((char *) sym_return_val, newsize);
-    }
-  sym_return_val[sym_return_val_index++] = sym;
-  sym_return_val[sym_return_val_index] = NULL;
+  overload_list->push_back (sym);
 }
 
 /* Return a null-terminated list of pointers to function symbols that
    are named FUNC_NAME and are visible within NAMESPACE.  */
 
-struct symbol **
+struct std::vector<symbol *>
 make_symbol_overload_list (const char *func_name,
 			   const char *the_namespace)
 {
-  struct cleanup *old_cleanups;
   const char *name;
+  std::vector<symbol *> overload_list;
 
-  sym_return_val_size = 100;
-  sym_return_val_index = 0;
-  sym_return_val = XNEWVEC (struct symbol *, sym_return_val_size + 1);
-  sym_return_val[0] = NULL;
-
-  old_cleanups = make_cleanup (xfree, sym_return_val);
+  overload_list.reserve (100);
 
-  make_symbol_overload_list_using (func_name, the_namespace);
+  add_symbol_overload_list_using (func_name, the_namespace, &overload_list);
 
   if (the_namespace[0] == '\0')
     name = func_name;
@@ -1211,19 +1195,17 @@  make_symbol_overload_list (const char *func_name,
       name = concatenated_name;
     }
 
-  make_symbol_overload_list_qualified (name);
-
-  discard_cleanups (old_cleanups);
-
-  return sym_return_val;
+  add_symbol_overload_list_qualified (name, &overload_list);
+  return overload_list;
 }
 
 /* Add all symbols with a name matching NAME in BLOCK to the overload
    list.  */
 
 static void
-make_symbol_overload_list_block (const char *name,
-                                 const struct block *block)
+add_symbol_overload_list_block (const char *name,
+				const struct block *block,
+				std::vector<symbol *> *overload_list)
 {
   struct block_iterator iter;
   struct symbol *sym;
@@ -1231,14 +1213,15 @@  make_symbol_overload_list_block (const char *name,
   lookup_name_info lookup_name (name, symbol_name_match_type::FULL);
 
   ALL_BLOCK_SYMBOLS_WITH_NAME (block, lookup_name, iter, sym)
-    overload_list_add_symbol (sym, name);
+    overload_list_add_symbol (sym, name, overload_list);
 }
 
 /* Adds the function FUNC_NAME from NAMESPACE to the overload set.  */
 
 static void
-make_symbol_overload_list_namespace (const char *func_name,
-                                     const char *the_namespace)
+add_symbol_overload_list_namespace (const char *func_name,
+				    const char *the_namespace,
+				    std::vector<symbol *> *overload_list)
 {
   const char *name;
   const struct block *block = NULL;
@@ -1259,12 +1242,12 @@  make_symbol_overload_list_namespace (const char *func_name,
   /* Look in the static block.  */
   block = block_static_block (get_selected_block (0));
   if (block)
-    make_symbol_overload_list_block (name, block);
+    add_symbol_overload_list_block (name, block, overload_list);
 
   /* Look in the global block.  */
   block = block_global_block (block);
   if (block)
-    make_symbol_overload_list_block (name, block);
+    add_symbol_overload_list_block (name, block, overload_list);
 
 }
 
@@ -1272,8 +1255,9 @@  make_symbol_overload_list_namespace (const char *func_name,
    base types.  */
 
 static void
-make_symbol_overload_list_adl_namespace (struct type *type,
-                                         const char *func_name)
+add_symbol_overload_list_adl_namespace (struct type *type,
+					const char *func_name,
+					std::vector<symbol *> *overload_list)
 {
   char *the_namespace;
   const char *type_name;
@@ -1303,7 +1287,8 @@  make_symbol_overload_list_adl_namespace (struct type *type,
       strncpy (the_namespace, type_name, prefix_len);
       the_namespace[prefix_len] = '\0';
 
-      make_symbol_overload_list_namespace (func_name, the_namespace);
+      add_symbol_overload_list_namespace (func_name, the_namespace,
+					  overload_list);
     }
 
   /* Check public base type */
@@ -1311,28 +1296,23 @@  make_symbol_overload_list_adl_namespace (struct type *type,
     for (i = 0; i < TYPE_N_BASECLASSES (type); i++)
       {
 	if (BASETYPE_VIA_PUBLIC (type, i))
-	  make_symbol_overload_list_adl_namespace (TYPE_BASECLASS (type,
-								   i),
-						   func_name);
+	  add_symbol_overload_list_adl_namespace (TYPE_BASECLASS (type, i),
+						  func_name,
+						  overload_list);
       }
 }
 
-/* Adds the overload list overload candidates for FUNC_NAME found
-   through argument dependent lookup.  */
+/* Adds to OVERLOAD_LIST the overload list overload candidates for
+   FUNC_NAME found through argument dependent lookup.  */
 
-struct symbol **
-make_symbol_overload_list_adl (struct type **arg_types, int nargs,
-                               const char *func_name)
+void
+add_symbol_overload_list_adl (gdb::array_view<type *> arg_types,
+			      const char *func_name,
+			      std::vector<symbol *> *overload_list)
 {
-  int i;
-
-  gdb_assert (sym_return_val_size != -1);
-
-  for (i = 1; i <= nargs; i++)
-    make_symbol_overload_list_adl_namespace (arg_types[i - 1],
-					     func_name);
-
-  return sym_return_val;
+  for (type *arg_type : arg_types)
+    add_symbol_overload_list_adl_namespace (arg_type, func_name,
+					    overload_list);
 }
 
 /* This applies the using directives to add namespaces to search in,
@@ -1341,8 +1321,9 @@  make_symbol_overload_list_adl (struct type **arg_types, int nargs,
    make_symbol_overload_list.  */
 
 static void
-make_symbol_overload_list_using (const char *func_name,
-				 const char *the_namespace)
+add_symbol_overload_list_using (const char *func_name,
+				const char *the_namespace,
+				std::vector<symbol *> *overload_list)
 {
   struct using_direct *current;
   const struct block *block;
@@ -1374,13 +1355,15 @@  make_symbol_overload_list_using (const char *func_name,
 	    scoped_restore reset_directive_searched
 	      = make_scoped_restore (&current->searched, 1);
 
-	    make_symbol_overload_list_using (func_name,
-					     current->import_src);
+	    add_symbol_overload_list_using (func_name,
+					    current->import_src,
+					    overload_list);
 	  }
       }
 
   /* Now, add names for this namespace.  */
-  make_symbol_overload_list_namespace (func_name, the_namespace);
+  add_symbol_overload_list_namespace (func_name, the_namespace,
+				      overload_list);
 }
 
 /* This does the bulk of the work of finding overloaded symbols.
@@ -1388,7 +1371,8 @@  make_symbol_overload_list_using (const char *func_name,
    (possibly including namespace info).  */
 
 static void
-make_symbol_overload_list_qualified (const char *func_name)
+add_symbol_overload_list_qualified (const char *func_name,
+				    std::vector<symbol *> *overload_list)
 {
   struct compunit_symtab *cust;
   struct objfile *objfile;
@@ -1407,7 +1391,7 @@  make_symbol_overload_list_qualified (const char *func_name)
      complete on local vars.  */
 
   for (b = get_selected_block (0); b != NULL; b = BLOCK_SUPERBLOCK (b))
-    make_symbol_overload_list_block (func_name, b);
+    add_symbol_overload_list_block (func_name, b, overload_list);
 
   surrounding_static_block = block_static_block (get_selected_block (0));
 
@@ -1418,7 +1402,7 @@  make_symbol_overload_list_qualified (const char *func_name)
   {
     QUIT;
     b = BLOCKVECTOR_BLOCK (COMPUNIT_BLOCKVECTOR (cust), GLOBAL_BLOCK);
-    make_symbol_overload_list_block (func_name, b);
+    add_symbol_overload_list_block (func_name, b, overload_list);
   }
 
   ALL_COMPUNITS (objfile, cust)
@@ -1428,7 +1412,7 @@  make_symbol_overload_list_qualified (const char *func_name)
     /* Don't do this block twice.  */
     if (b == surrounding_static_block)
       continue;
-    make_symbol_overload_list_block (func_name, b);
+    add_symbol_overload_list_block (func_name, b, overload_list);
   }
 }
 
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index 4e26921595..0402df02d3 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -28,6 +28,8 @@ 
 #include "vec.h"
 #include "gdb_vecs.h"
 #include "gdb_obstack.h"
+#include "common/array-view.h"
+#include <vector>
 
 /* Opaque declarations.  */
 
@@ -107,12 +109,13 @@  extern gdb::unique_xmalloc_ptr<char> cp_remove_params
 extern gdb::unique_xmalloc_ptr<char> cp_remove_params_if_any
   (const char *demangled_name, bool completion_mode);
 
-extern struct symbol **make_symbol_overload_list (const char *,
-						  const char *);
+extern std::vector<symbol *> make_symbol_overload_list (const char *,
+							const char *);
 
-extern struct symbol **make_symbol_overload_list_adl (struct type **arg_types,
-                                                      int nargs,
-                                                      const char *func_name);
+extern void add_symbol_overload_list_adl
+  (gdb::array_view<type *> arg_types,
+   const char *func_name,
+   std::vector<symbol *> *overload_list);
 
 extern struct type *cp_lookup_rtti_type (const char *name,
 					 struct block *block);
diff --git a/gdb/valops.c b/gdb/valops.c
index f0e53a7ce9..7fbedd77bb 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -56,13 +56,13 @@  static struct value *search_struct_method (const char *, struct value **,
 
 static int find_oload_champ_namespace (gdb::array_view<value *> args,
 				       const char *, const char *,
-				       struct symbol ***,
+				       std::vector<symbol *> *oload_syms,
 				       struct badness_vector **,
 				       const int no_adl);
 
 static int find_oload_champ_namespace_loop (gdb::array_view<value *> args,
 					    const char *, const char *,
-					    int, struct symbol ***,
+					    int, std::vector<symbol *> *oload_syms,
 					    struct badness_vector **, int *,
 					    const int no_adl);
 
@@ -2517,7 +2517,7 @@  find_overload_match (gdb::array_view<value *> args,
   /* For methods, the list of overloaded methods.  */
   struct fn_field *fns_ptr = NULL;
   /* For non-methods, the list of overloaded function symbols.  */
-  struct symbol **oload_syms = NULL;
+  std::vector<symbol *> oload_syms;
   /* For xmethods, the vector of xmethod workers.  */
   std::vector<xmethod_worker_up> xm_worker_vec;
   /* Number of overloaded instances being considered.  */
@@ -2717,7 +2717,6 @@  find_overload_match (gdb::array_view<value *> args,
 	func_match_quality = classify_oload_match (func_badness,
 						   args.size (), 0);
 
-      make_cleanup (xfree, oload_syms);
       make_cleanup (xfree, func_badness);
     }
 
@@ -2857,7 +2856,7 @@  static int
 find_oload_champ_namespace (gdb::array_view<value *> args,
 			    const char *func_name,
 			    const char *qualified_name,
-			    struct symbol ***oload_syms,
+			    std::vector<symbol *> *oload_syms,
 			    struct badness_vector **oload_champ_bv,
 			    const int no_adl)
 {
@@ -2887,17 +2886,15 @@  find_oload_champ_namespace_loop (gdb::array_view<value *> args,
 				 const char *func_name,
 				 const char *qualified_name,
 				 int namespace_len,
-				 struct symbol ***oload_syms,
+				 std::vector<symbol *> *oload_syms,
 				 struct badness_vector **oload_champ_bv,
 				 int *oload_champ,
 				 const int no_adl)
 {
   int next_namespace_len = namespace_len;
   int searched_deeper = 0;
-  int num_fns = 0;
   struct cleanup *old_cleanups;
   int new_oload_champ;
-  struct symbol **new_oload_syms;
   struct badness_vector *new_oload_champ_bv;
   char *new_namespace;
 
@@ -2910,7 +2907,6 @@  find_oload_champ_namespace_loop (gdb::array_view<value *> args,
     cp_find_first_component (qualified_name + next_namespace_len);
 
   /* Initialize these to values that can safely be xfree'd.  */
-  *oload_syms = NULL;
   *oload_champ_bv = NULL;
 
   /* First, see if we have a deeper namespace we can search in.
@@ -2938,13 +2934,13 @@  find_oload_champ_namespace_loop (gdb::array_view<value *> args,
      because this overload mechanism only gets called if there's a
      function symbol to start off with.)  */
 
-  old_cleanups = make_cleanup (xfree, *oload_syms);
-  make_cleanup (xfree, *oload_champ_bv);
+  old_cleanups = make_cleanup (xfree, *oload_champ_bv);
   new_namespace = (char *) alloca (namespace_len + 1);
   strncpy (new_namespace, qualified_name, namespace_len);
   new_namespace[namespace_len] = '\0';
-  new_oload_syms = make_symbol_overload_list (func_name,
-					      new_namespace);
+
+  std::vector<symbol *> new_oload_syms
+    = make_symbol_overload_list (func_name, new_namespace);
 
   /* If we have reached the deepest level perform argument
      determined lookup.  */
@@ -2958,14 +2954,12 @@  find_oload_champ_namespace_loop (gdb::array_view<value *> args,
 	alloca (args.size () * (sizeof (struct type *)));
       for (ix = 0; ix < args.size (); ix++)
 	arg_types[ix] = value_type (args[ix]);
-      make_symbol_overload_list_adl (arg_types, args.size (), func_name);
+      add_symbol_overload_list_adl ({arg_types, args.size ()}, func_name,
+				    &new_oload_syms);
     }
 
-  while (new_oload_syms[num_fns])
-    ++num_fns;
-
-  new_oload_champ = find_oload_champ (args, num_fns,
-				      NULL, NULL, new_oload_syms,
+  new_oload_champ = find_oload_champ (args, new_oload_syms.size (),
+				      NULL, NULL, new_oload_syms.data (),
 				      &new_oload_champ_bv);
 
   /* Case 1: We found a good match.  Free earlier matches (if any),
@@ -2978,7 +2972,7 @@  find_oload_champ_namespace_loop (gdb::array_view<value *> args,
   if (new_oload_champ != -1
       && classify_oload_match (new_oload_champ_bv, args.size (), 0) == STANDARD)
     {
-      *oload_syms = new_oload_syms;
+      *oload_syms = std::move (new_oload_syms);
       *oload_champ = new_oload_champ;
       *oload_champ_bv = new_oload_champ_bv;
       do_cleanups (old_cleanups);
@@ -2986,14 +2980,13 @@  find_oload_champ_namespace_loop (gdb::array_view<value *> args,
     }
   else if (searched_deeper)
     {
-      xfree (new_oload_syms);
       xfree (new_oload_champ_bv);
       discard_cleanups (old_cleanups);
       return 0;
     }
   else
     {
-      *oload_syms = new_oload_syms;
+      *oload_syms = std::move (new_oload_syms);
       *oload_champ = new_oload_champ;
       *oload_champ_bv = new_oload_champ_bv;
       do_cleanups (old_cleanups);