[llvm] [ADT] Refactor DenseMap::insert, try_emplace, and operator[] (NFC) (PR #155204)

[Kazu Hirata via llvm-commits]

https://github.com/kazutakahirata updated https://github.com/llvm/llvm-project/pull/155204

>From 81cbab4d6239ab9b0a8651dda40d4623f9bb59ec Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Sun, 24 Aug 2025 16:02:03 -0700
Subject: [PATCH 1/2] [ADT] Refactor DenseMap::insert, try_emplace, and
 operator[] (NFC)

try_emplace and operator[] contain nearly identical code, and the code
is duplicated for l-value and r-value reference variants.

This patch introduces a templated helper function, try_emplace_impl,
and uses it in all of DenseMap::insert, try_emplace, and operator[].
The helper function uses perfect forwarding to preserve the exact key
type.
---
 llvm/include/llvm/ADT/DenseMap.h | 45 +++++++++++++-------------------
 1 file changed, 18 insertions(+), 27 deletions(-)

diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index ab1bc6356dcb9..4960c85138436 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -240,14 +240,14 @@ class DenseMapBase : public DebugEpochBase {
   // If the key is already in the map, it returns false and doesn't update the
   // value.
   std::pair<iterator, bool> insert(const std::pair<KeyT, ValueT> &KV) {
-    return try_emplace(KV.first, KV.second);
+    return try_emplace_impl(KV.first, KV.second);
   }
 
   // Inserts key,value pair into the map if the key isn't already in the map.
   // If the key is already in the map, it returns false and doesn't update the
   // value.
   std::pair<iterator, bool> insert(std::pair<KeyT, ValueT> &&KV) {
-    return try_emplace(std::move(KV.first), std::move(KV.second));
+    return try_emplace_impl(std::move(KV.first), std::move(KV.second));
   }
 
   // Inserts key,value pair into the map if the key isn't already in the map.
@@ -255,14 +255,7 @@ class DenseMapBase : public DebugEpochBase {
   // it is not moved.
   template <typename... Ts>
   std::pair<iterator, bool> try_emplace(KeyT &&Key, Ts &&...Args) {
-    BucketT *TheBucket;
-    if (LookupBucketFor(Key, TheBucket))
-      return {makeInsertIterator(TheBucket), false}; // Already in map.
-
-    // Otherwise, insert the new element.
-    TheBucket =
-        InsertIntoBucket(TheBucket, std::move(Key), std::forward<Ts>(Args)...);
-    return {makeInsertIterator(TheBucket), true};
+    return try_emplace_impl(std::move(Key), std::forward<Ts>(Args)...);
   }
 
   // Inserts key,value pair into the map if the key isn't already in the map.
@@ -270,13 +263,7 @@ class DenseMapBase : public DebugEpochBase {
   // it is not moved.
   template <typename... Ts>
   std::pair<iterator, bool> try_emplace(const KeyT &Key, Ts &&...Args) {
-    BucketT *TheBucket;
-    if (LookupBucketFor(Key, TheBucket))
-      return {makeInsertIterator(TheBucket), false}; // Already in map.
-
-    // Otherwise, insert the new element.
-    TheBucket = InsertIntoBucket(TheBucket, Key, std::forward<Ts>(Args)...);
-    return {makeInsertIterator(TheBucket), true};
+    return try_emplace_impl(Key, std::forward<Ts>(Args)...);
   }
 
   /// Alternate version of insert() which allows a different, and possibly
@@ -360,19 +347,11 @@ class DenseMapBase : public DebugEpochBase {
   }
 
   ValueT &operator[](const KeyT &Key) {
-    BucketT *TheBucket;
-    if (LookupBucketFor(Key, TheBucket))
-      return TheBucket->second;
-
-    return InsertIntoBucket(TheBucket, Key)->second;
+    return try_emplace_impl(Key).first->second;
   }
 
   ValueT &operator[](KeyT &&Key) {
-    BucketT *TheBucket;
-    if (LookupBucketFor(Key, TheBucket))
-      return TheBucket->second;
-
-    return InsertIntoBucket(TheBucket, std::move(Key))->second;
+    return try_emplace_impl(std::move(Key)).first->second;
   }
 
   /// isPointerIntoBucketsArray - Return true if the specified pointer points
@@ -496,6 +475,18 @@ class DenseMapBase : public DebugEpochBase {
   static const KeyT getTombstoneKey() { return KeyInfoT::getTombstoneKey(); }
 
 private:
+  template <typename KeyArgT, typename... Ts>
+  std::pair<iterator, bool> try_emplace_impl(KeyArgT &&Key, Ts &&...Args) {
+    BucketT *TheBucket;
+    if (LookupBucketFor(Key, TheBucket))
+      return {makeInsertIterator(TheBucket), false}; // Already in map.
+
+    // Otherwise, insert the new element.
+    TheBucket = InsertIntoBucket(TheBucket, std::forward<KeyArgT>(Key),
+                                 std::forward<Ts>(Args)...);
+    return {makeInsertIterator(TheBucket), true};
+  }
+
   iterator makeIterator(BucketT *P, BucketT *E, DebugEpochBase &Epoch,
                         bool NoAdvance = false) {
     if (shouldReverseIterate<KeyT>()) {

>From 697ef54d52450d0c90de08e1ed013b2b19cd66be Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Mon, 25 Aug 2025 07:17:52 -0700
Subject: [PATCH 2/2] Address a comment.

---
 llvm/include/llvm/ADT/DenseMap.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index 4960c85138436..60846af5bb283 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -477,9 +477,9 @@ class DenseMapBase : public DebugEpochBase {
 private:
   template <typename KeyArgT, typename... Ts>
   std::pair<iterator, bool> try_emplace_impl(KeyArgT &&Key, Ts &&...Args) {
-    BucketT *TheBucket;
+    BucketT *TheBucket = nullptr;
     if (LookupBucketFor(Key, TheBucket))
-      return {makeInsertIterator(TheBucket), false}; // Already in map.
+      return {makeInsertIterator(TheBucket), false}; // Already in the map.
 
     // Otherwise, insert the new element.
     TheBucket = InsertIntoBucket(TheBucket, std::forward<KeyArgT>(Key),