[v4,02/22] Convert compile-c-symbols.c to new hash table

Message ID 20240823184910.883268-3-simon.marchi@efficios.com
State New
Headers
Series Add a C++ hash table to gdbsupport |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_gdb_build--master-arm fail Patch failed to apply

Commit Message

Simon Marchi Aug. 23, 2024, 6:48 p.m. UTC
  This converts compile-c-symbols.c to use the new hash table.

I made it use a set of string_view instead of a set of `symbol *`, to
avoid calling `symbol::natural_name` over and over.  This appears safe
to do, since I don't expect the storage behing the natural names to
change during the lifetime of the map.

Change-Id: Ie9f9334d4f03b9a8ae6886287f82cd435eee217c
Co-Authored-By: Tom Tromey <tom@tromey.com>
---
 gdb/compile/compile-c-symbols.c | 49 ++++-----------------------------
 1 file changed, 5 insertions(+), 44 deletions(-)
  

Comments

Tom Tromey Oct. 4, 2024, 8:20 p.m. UTC | #1
>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> This converts compile-c-symbols.c to use the new hash table.
Simon> I made it use a set of string_view instead of a set of `symbol *`, to
Simon> avoid calling `symbol::natural_name` over and over.  This appears safe
Simon> to do, since I don't expect the storage behing the natural names to
Simon> change during the lifetime of the map.

I don't recall if I did this or you did, but anyway the change here
seems fine, and I agree about the lifetime.

Tom
  

Patch

diff --git a/gdb/compile/compile-c-symbols.c b/gdb/compile/compile-c-symbols.c
index 7a38d3a6d593..f7331453ccf9 100644
--- a/gdb/compile/compile-c-symbols.c
+++ b/gdb/compile/compile-c-symbols.c
@@ -30,7 +30,7 @@ 
 #include "gdbtypes.h"
 #include "dwarf2/loc.h"
 #include "inferior.h"
-
+#include "gdbsupport/unordered_set.h"
 
 /* Compute the name of the pointer representing a local symbol's
    address.  */
@@ -441,46 +441,6 @@  gcc_symbol_address (void *datum, struct gcc_c_context *gcc_context,
   return result;
 }
 
-
-
-/* A hash function for symbol names.  */
-
-static hashval_t
-hash_symname (const void *a)
-{
-  const struct symbol *sym = (const struct symbol *) a;
-
-  return htab_hash_string (sym->natural_name ());
-}
-
-/* A comparison function for hash tables that just looks at symbol
-   names.  */
-
-static int
-eq_symname (const void *a, const void *b)
-{
-  const struct symbol *syma = (const struct symbol *) a;
-  const struct symbol *symb = (const struct symbol *) b;
-
-  return strcmp (syma->natural_name (), symb->natural_name ()) == 0;
-}
-
-/* If a symbol with the same name as SYM is already in HASHTAB, return
-   1.  Otherwise, add SYM to HASHTAB and return 0.  */
-
-static int
-symbol_seen (htab_t hashtab, struct symbol *sym)
-{
-  void **slot;
-
-  slot = htab_find_slot (hashtab, sym, INSERT);
-  if (*slot != NULL)
-    return 1;
-
-  *slot = sym;
-  return 0;
-}
-
 /* Generate C code to compute the length of a VLA.  */
 
 static void
@@ -626,8 +586,7 @@  generate_c_for_variable_locations (compile_instance *compiler,
 
   /* Ensure that a given name is only entered once.  This reflects the
      reality of shadowing.  */
-  htab_up symhash (htab_create_alloc (1, hash_symname, eq_symname, NULL,
-				      xcalloc, xfree));
+  gdb::unordered_set<std::string_view> symset;
 
   while (1)
     {
@@ -635,7 +594,9 @@  generate_c_for_variable_locations (compile_instance *compiler,
 	 compute the location of each local variable.  */
       for (struct symbol *sym : block_iterator_range (block))
 	{
-	  if (!symbol_seen (symhash.get (), sym))
+	  bool inserted = symset.insert (sym->natural_name ()).second;
+
+	  if (inserted)
 	    generate_c_for_for_one_variable (compiler, stream, gdbarch,
 					     registers_used, pc, sym);
 	}