[llvm] [ADT] Consolidate copyFrom in DenseMap.h (NFC) (PR #165101)

Kazu Hirata via llvm-commits llvm-commits at lists.llvm.org
Sat Oct 25 07:53:41 PDT 2025


https://github.com/kazutakahirata created https://github.com/llvm/llvm-project/pull/165101

DenseMap.h has:

- DenseMapBase::copyFrom
- DenseMap::copyFrom
- SmallDenseMap::copyFrom

The latter two clear and set up the storage again before delegating
DenseMapBase::copyFrom to do the actual work of copying buckets.

This patch consolidates all these into DenseMapBase::copyFrom while
eliminating name shadowing concerns.  Note that DenseMap::copyFrom and
SmallDenseMap::copyFrom are nearly identical, and they can be made
identical with small adjustments:

- Set NumEntries and NumTombstones to 0 unconditionally.
- Teach SmallDenseMap::allocateBuckets to always return true.

This patch essentially applies these adjustments and then "inlines"
the identical function body to the beginning of
DenseMapBase::copyFrom.

This patch de-templatizes DenseMapBase::copyFrom because nobody calls
it with any type other than DerivedT.


>From 158e29f3962e08826b8521db7adf591e164b178f Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Fri, 24 Oct 2025 16:56:01 -0700
Subject: [PATCH] [ADT] Consolidate copyFrom in DenseMap.h (NFC)

DenseMap.h has:

- DenseMapBase::copyFrom
- DenseMap::copyFrom
- SmallDenseMap::copyFrom

The latter two clear and set up the storage again before delegating
DenseMapBase::copyFrom to do the actual work of copying buckets.

This patch consolidates all these into DenseMapBase::copyFrom while
eliminating name shadowing concerns.  Note that DenseMap::copyFrom and
SmallDenseMap::copyFrom are nearly identical, and they can be made
identical with small adjustments:

- Set NumEntries and NumTombstones to 0 unconditionally.
- Teach SmallDenseMap::allocateBuckets to always return true.

This patch essentially applies these adjustments and then "inlines"
the identical function body to the beginning of
DenseMapBase::copyFrom.

This patch de-templatizes DenseMapBase::copyFrom because nobody calls
it with any type other than DerivedT.
---
 llvm/include/llvm/ADT/DenseMap.h | 42 ++++++++++++--------------------
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index baa91f3a5f533..c784859081221 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -418,9 +418,16 @@ class DenseMapBase : public DebugEpochBase {
     }
   }
 
-  template <typename OtherBaseT>
-  void copyFrom(
-      const DenseMapBase<OtherBaseT, KeyT, ValueT, KeyInfoT, BucketT> &other) {
+  void copyFrom(const DerivedT &other) {
+    this->destroyAll();
+    derived().deallocateBuckets();
+    setNumEntries(0);
+    setNumTombstones(0);
+    if (!derived().allocateBuckets(other.getNumBuckets())) {
+      // The bucket list is empty.  No work to do.
+      return;
+    }
+
     assert(&other != this);
     assert(getNumBuckets() == other.getNumBuckets());
 
@@ -725,7 +732,7 @@ class DenseMap : public DenseMapBase<DenseMap<KeyT, ValueT, KeyInfoT, BucketT>,
 
   DenseMap(const DenseMap &other) : BaseT() {
     init(0);
-    copyFrom(other);
+    this->copyFrom(other);
   }
 
   DenseMap(DenseMap &&other) : BaseT() {
@@ -761,7 +768,7 @@ class DenseMap : public DenseMapBase<DenseMap<KeyT, ValueT, KeyInfoT, BucketT>,
 
   DenseMap &operator=(const DenseMap &other) {
     if (&other != this)
-      copyFrom(other);
+      this->copyFrom(other);
     return *this;
   }
 
@@ -831,17 +838,6 @@ class DenseMap : public DenseMapBase<DenseMap<KeyT, ValueT, KeyInfoT, BucketT>,
     }
   }
 
-  void copyFrom(const DenseMap &other) {
-    this->destroyAll();
-    deallocateBuckets();
-    if (allocateBuckets(other.NumBuckets)) {
-      this->BaseT::copyFrom(other);
-    } else {
-      NumEntries = 0;
-      NumTombstones = 0;
-    }
-  }
-
   void grow(unsigned AtLeast) {
     unsigned OldNumBuckets = NumBuckets;
     BucketT *OldBuckets = Buckets;
@@ -902,7 +898,7 @@ class SmallDenseMap
 
   SmallDenseMap(const SmallDenseMap &other) : BaseT() {
     init(0);
-    copyFrom(other);
+    this->copyFrom(other);
   }
 
   SmallDenseMap(SmallDenseMap &&other) : BaseT() {
@@ -1001,7 +997,7 @@ class SmallDenseMap
 
   SmallDenseMap &operator=(const SmallDenseMap &other) {
     if (&other != this)
-      copyFrom(other);
+      this->copyFrom(other);
     return *this;
   }
 
@@ -1099,7 +1095,7 @@ class SmallDenseMap
     getLargeRep()->~LargeRep();
   }
 
-  void allocateBuckets(unsigned Num) {
+  bool allocateBuckets(unsigned Num) {
     if (Num <= InlineBuckets) {
       Small = true;
     } else {
@@ -1108,6 +1104,7 @@ class SmallDenseMap
           allocate_buffer(sizeof(BucketT) * Num, alignof(BucketT)));
       new (getLargeRep()) LargeRep{NewBuckets, Num};
     }
+    return true;
   }
 
   void init(unsigned InitNumEntries) {
@@ -1116,13 +1113,6 @@ class SmallDenseMap
     this->BaseT::initEmpty();
   }
 
-  void copyFrom(const SmallDenseMap &other) {
-    this->destroyAll();
-    deallocateBuckets();
-    allocateBuckets(other.getNumBuckets());
-    this->BaseT::copyFrom(other);
-  }
-
   void grow(unsigned AtLeast) {
     if (AtLeast > InlineBuckets)
       AtLeast = std::max<unsigned>(64, NextPowerOf2(AtLeast - 1));



More information about the llvm-commits mailing list