[PATCH] gdb: turn gdb::bcache's function pointers into virtual methods

[PATCH] gdb: turn gdb::bcache's function pointers into virtual methods

[Simon Marchi]

The two function pointers optionally passed to gdb::bcache are very good
candidates to be turned into virtual methods, this patch does that in
the most straightforward / unsurprising way.

gdb/ChangeLog:

	* bcache.h (struct bcache) <bcache>: Remove constructor.
	<m_hash_function, m_compare_function>: Remove.
	<~bcache>: Make virtual.
	<compare>: Remove static method, introduce virtual method.
	<default_hash>: Remove.
	<hash>: New virtual method.
	* bcache.c (bcache::expand_hash_table): Update.
	(bcache::insert): Update.
	(bcache::hash): New.
	(bcache::compare): Update comment and parameter names.
	* gdbtypes.c (types_deeply_equal): Update.
	* psymtab.h (struct psymbol_bcache): New struct.
	(class psymtab_storage) <psymtab_storage>: Make default.
	<psymbol_cache>: Change type to psymbol_bcache.
	* psymtab.c (psymtab_storage::psymtab_storage): Remove.
	(psymbol_hash): Change to...
	(psymbol_bcache::hash): ... this.
	(psymbol_compare): Change to...
	(psymbol_bcache::compare): ... this.

Change-Id: I41d578e61de8ac1163461a28fbd220d1f855e372
---
 gdb/bcache.c   | 21 ++++++++++++++-------
 gdb/bcache.h   | 41 +++++++++++------------------------------
 gdb/gdbtypes.c |  2 +-
 gdb/psymtab.c  | 27 ++++++---------------------
 gdb/psymtab.h  | 22 ++++++++++++++++++----
 5 files changed, 50 insertions(+), 63 deletions(-)

diff --git a/gdb/bcache.c b/gdb/bcache.c
index 49d9d3f486ba..ba4cbdddb549 100644
--- a/gdb/bcache.c
+++ b/gdb/bcache.c
@@ -113,7 +113,7 @@ bcache::expand_hash_table ()
 	  struct bstring **new_bucket;
 	  next = s->next;
 
-	  new_bucket = &new_buckets[(m_hash_function (&s->d.data, s->length)
+	  new_bucket = &new_buckets[(this->hash (&s->d.data, s->length)
 				     % new_num_buckets)];
 	  s->next = *new_bucket;
 	  *new_bucket = s;
@@ -167,7 +167,7 @@ bcache::insert (const void *addr, int length, int *added)
   m_total_count++;
   m_total_size += length;
 
-  full_hash = m_hash_function (addr, length);
+  full_hash = this->hash (addr, length);
 
   half_hash = (full_hash >> 16);
   hash_index = full_hash % m_num_buckets;
@@ -180,7 +180,7 @@ bcache::insert (const void *addr, int length, int *added)
       if (s->half_hash == half_hash)
 	{
 	  if (s->length == length
-	      && m_compare_function (&s->d.data, addr, length))
+	      && this->compare (&s->d.data, addr, length))
 	    return &s->d.data;
 	  else
 	    m_half_hash_miss_count++;
@@ -211,13 +211,20 @@ bcache::insert (const void *addr, int length, int *added)
 }
 \f
 
-/* Compare the byte string at ADDR1 of lenght LENGHT to the
-   string at ADDR2.  Return 1 if they are equal.  */
+/* See bcache.h.  */
+
+unsigned long
+bcache::hash (const void *addr, int length)
+{
+  return fast_hash (addr, length, 0);
+}
+
+/* See bcache.h.  */
 
 int
-bcache::compare (const void *addr1, const void *addr2, int length)
+bcache::compare (const void *left, const void *right, int length)
 {
-  return memcmp (addr1, addr2, length) == 0;
+  return memcmp (left, right, length) == 0;
 }
 
 /* Free all the storage associated with BCACHE.  */
diff --git a/gdb/bcache.h b/gdb/bcache.h
index 929375642046..02c7067a70b0 100644
--- a/gdb/bcache.h
+++ b/gdb/bcache.h
@@ -142,21 +142,7 @@ struct bstring;
 
 struct bcache
 {
-  /* Allocate a bcache.  HASH_FN and COMPARE_FN can be used to pass in
-     custom hash, and compare functions to be used by this bcache.  If
-     HASH_FUNCTION is NULL fast_hash() is used and if COMPARE_FUNCTION is
-     NULL memcmp() is used.  */
-
-  explicit bcache (unsigned long (*hash_fn)(const void *,
-					    int length) = nullptr,
-		   int (*compare_fn)(const void *, const void *,
-				     int length) = nullptr)
-    : m_hash_function (hash_fn == nullptr ? default_hash : hash_fn),
-      m_compare_function (compare_fn == nullptr ? compare : compare_fn)
-  {
-  }
-
-  ~bcache ();
+  virtual ~bcache ();
 
   /* Find a copy of the LENGTH bytes at ADDR in BCACHE.  If BCACHE has
      never seen those bytes before, add a copy of them to BCACHE.  In
@@ -175,6 +161,16 @@ struct bcache
   void print_statistics (const char *type);
   int memory_used ();
 
+protected:
+
+  /* Hash function to be used for this bcache object.  Defaults to
+     fast_hash.  */
+  virtual unsigned long hash (const void *addr, int length);
+
+  /* Compare function to be used for this bcache object.  Defaults to
+     memcmp.  */
+  virtual int compare (const void *left, const void *right, int length);
+
 private:
 
   /* All the bstrings are allocated here.  */
@@ -205,21 +201,6 @@ struct bcache
      length/data compare missed.  */
   unsigned long m_half_hash_miss_count = 0;
 
-  /* Hash function to be used for this bcache object.  */
-  unsigned long (*m_hash_function)(const void *addr, int length);
-
-  /* Compare function to be used for this bcache object.  */
-  int (*m_compare_function)(const void *, const void *, int length);
-
-  /* Default compare function.  */
-  static int compare (const void *addr1, const void *addr2, int length);
-
-  /* Default hash function.  */
-  static unsigned long default_hash (const void *ptr, int length)
-  {
-    return fast_hash (ptr, length, 0);
-  }
-
   /* Expand the hash table.  */
   void expand_hash_table ();
 };
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index c1eb03d89844..22cc00d33f41 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -4120,7 +4120,7 @@ types_deeply_equal (struct type *type1, struct type *type2)
   if (type1 == type2)
     return true;
 
-  gdb::bcache cache (nullptr, nullptr);
+  gdb::bcache cache;
   worklist.emplace_back (type1, type2);
   return check_types_worklist (&worklist, &cache);
 }
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 47e31aab61e0..7b5e08ebf1ba 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -53,16 +53,6 @@ static struct partial_symbol *find_pc_sect_psymbol (struct objfile *,
 static struct compunit_symtab *psymtab_to_symtab (struct objfile *objfile,
 						  struct partial_symtab *pst);
 
-\f
-
-static unsigned long psymbol_hash (const void *addr, int length);
-static int psymbol_compare (const void *addr1, const void *addr2, int length);
-
-psymtab_storage::psymtab_storage ()
-  : psymbol_cache (psymbol_hash, psymbol_compare)
-{
-}
-
 psymtab_storage::~psymtab_storage ()
 {
   partial_symtab *iter = psymtabs;
@@ -1537,13 +1527,10 @@ end_psymtab_common (struct objfile *objfile, struct partial_symtab *pst)
   sort_pst_symbols (objfile, pst);
 }
 
-/* Calculate a hash code for the given partial symbol.  The hash is
-   calculated using the symbol's value, language, domain, class
-   and name.  These are the values which are set by
-   add_psymbol_to_bcache.  */
+/* See psymtab.h.  */
 
-static unsigned long
-psymbol_hash (const void *addr, int length)
+unsigned long
+psymbol_bcache::hash (const void *addr, int length)
 {
   unsigned long h = 0;
   struct partial_symbol *psymbol = (struct partial_symbol *) addr;
@@ -1562,12 +1549,10 @@ psymbol_hash (const void *addr, int length)
   return h;
 }
 
-/* Returns true if the symbol at addr1 equals the symbol at addr2.
-   For the comparison this function uses a symbols value,
-   language, domain, class and name.  */
+/* See psymtab.h.  */
 
-static int
-psymbol_compare (const void *addr1, const void *addr2, int length)
+int
+psymbol_bcache::compare (const void *addr1, const void *addr2, int length)
 {
   struct partial_symbol *sym1 = (struct partial_symbol *) addr1;
   struct partial_symbol *sym2 = (struct partial_symbol *) addr2;
diff --git a/gdb/psymtab.h b/gdb/psymtab.h
index e8bafbe4338f..3c2d65f40ee5 100644
--- a/gdb/psymtab.h
+++ b/gdb/psymtab.h
@@ -27,6 +27,22 @@
 
 struct partial_symbol;
 
+/* Specialization of bcache to store partial symbols.  */
+
+struct psymbol_bcache : public gdb::bcache
+{
+  /* Calculate a hash code for the given partial symbol.  The hash is
+     calculated using the symbol's value, language, domain, class
+     and name.  These are the values which are set by
+     add_psymbol_to_bcache.  */
+  unsigned long hash (const void *addr, int length) override;
+
+  /* Returns true if the symbol LEFT equals the symbol RIGHT.
+     For the comparison this function uses a symbols value,
+     language, domain, class and name.  */
+  int compare (const void *left, const void *right, int length) override;
+};
+
 /* An instance of this class manages the partial symbol tables and
    partial symbols for a given objfile.
 
@@ -48,9 +64,7 @@ struct partial_symbol;
 class psymtab_storage
 {
 public:
-
-  psymtab_storage ();
-
+  psymtab_storage () = default;
   ~psymtab_storage ();
 
   DISABLE_COPY_AND_ASSIGN (psymtab_storage);
@@ -121,7 +135,7 @@ class psymtab_storage
   /* A byte cache where we can stash arbitrary "chunks" of bytes that
      will not change.  */
 
-  gdb::bcache psymbol_cache;
+  psymbol_bcache psymbol_cache;
 
   /* Vectors of all partial symbols read in from file.  The actual data
      is stored in the objfile_obstack.  */
-- 
2.28.0

Re: [PATCH] gdb: turn gdb::bcache's function pointers into virtual methods

[Simon Marchi]

On 2020-09-01 8:48 a.m., Simon Marchi wrote:
> The two function pointers optionally passed to gdb::bcache are very good
> candidates to be turned into virtual methods, this patch does that in
> the most straightforward / unsurprising way.

I pushed this patch.

Simon