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

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-adt

Author: Kazu Hirata (kazutakahirata)

<details>
<summary>Changes</summary>

The l-value and r-value reference variants of try_emplace contain
nearly identical code.  Also, operator[] makes its own call to
Map.try_emplace.

This patch introduces a templated helper function, try_emplace_impl,
and uses it in all of MapVector::insert, try_emplace, and operator[].
The helper function uses perfect forwarding to preserve the exact key
type.

This patch moves "using" declarations to the top of the class because
the new helper function needs iterator.


---
Full diff: https://github.com/llvm/llvm-project/pull/155205.diff


1 Files Affected:

- (modified) llvm/include/llvm/ADT/MapVector.h (+27-33) 


``````````diff
diff --git a/llvm/include/llvm/ADT/MapVector.h b/llvm/include/llvm/ADT/MapVector.h
index 24976535716d1..5f9f2b6794bc6 100644
--- a/llvm/include/llvm/ADT/MapVector.h
+++ b/llvm/include/llvm/ADT/MapVector.h
@@ -34,13 +34,6 @@ template <typename KeyT, typename ValueT,
           typename MapType = DenseMap<KeyT, unsigned>,
           typename VectorType = SmallVector<std::pair<KeyT, ValueT>, 0>>
 class MapVector {
-  MapType Map;
-  VectorType Vector;
-
-  static_assert(
-      std::is_integral_v<typename MapType::mapped_type>,
-      "The mapped_type of the specified Map must be an integral type");
-
 public:
   using key_type = KeyT;
   using value_type = typename VectorType::value_type;
@@ -51,6 +44,28 @@ class MapVector {
   using reverse_iterator = typename VectorType::reverse_iterator;
   using const_reverse_iterator = typename VectorType::const_reverse_iterator;
 
+private:
+  MapType Map;
+  VectorType Vector;
+
+  static_assert(
+      std::is_integral_v<typename MapType::mapped_type>,
+      "The mapped_type of the specified Map must be an integral type");
+
+  template <typename KeyArgT, typename... Ts>
+  std::pair<iterator, bool> try_emplace_impl(KeyArgT &&Key, Ts &&...Args) {
+    auto Result = Map.try_emplace(Key);
+    if (Result.second) {
+      Result.first->second = Vector.size();
+      Vector.emplace_back(std::piecewise_construct,
+                          std::forward_as_tuple(std::forward<KeyArgT>(Key)),
+                          std::forward_as_tuple(std::forward<Ts>(Args)...));
+      return {std::prev(end()), true};
+    }
+    return {begin() + Result.first->second, false};
+  }
+
+public:
   /// Clear the MapVector and return the underlying vector.
   VectorType takeVector() {
     Map.clear();
@@ -99,13 +114,7 @@ class MapVector {
   }
 
   ValueT &operator[](const KeyT &Key) {
-    std::pair<typename MapType::iterator, bool> Result = Map.try_emplace(Key);
-    auto &I = Result.first->second;
-    if (Result.second) {
-      Vector.push_back(std::make_pair(Key, ValueT()));
-      I = Vector.size() - 1;
-    }
-    return Vector[I].second;
+    return try_emplace_impl(Key).first->second;
   }
 
   // Returns a copy of the value.  Only allowed if ValueT is copyable.
@@ -118,33 +127,18 @@ class MapVector {
 
   template <typename... Ts>
   std::pair<iterator, bool> try_emplace(const KeyT &Key, Ts &&...Args) {
-    auto [It, Inserted] = Map.try_emplace(Key);
-    if (Inserted) {
-      It->second = Vector.size();
-      Vector.emplace_back(std::piecewise_construct, std::forward_as_tuple(Key),
-                          std::forward_as_tuple(std::forward<Ts>(Args)...));
-      return {std::prev(end()), true};
-    }
-    return {begin() + It->second, false};
+    return try_emplace_impl(Key, std::forward<Ts>(Args)...);
   }
   template <typename... Ts>
   std::pair<iterator, bool> try_emplace(KeyT &&Key, Ts &&...Args) {
-    auto [It, Inserted] = Map.try_emplace(Key);
-    if (Inserted) {
-      It->second = Vector.size();
-      Vector.emplace_back(std::piecewise_construct,
-                          std::forward_as_tuple(std::move(Key)),
-                          std::forward_as_tuple(std::forward<Ts>(Args)...));
-      return {std::prev(end()), true};
-    }
-    return {begin() + It->second, false};
+    return try_emplace_impl(std::move(Key), std::forward<Ts>(Args)...);
   }
 
   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);
   }
   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));
   }
 
   template <typename V>

``````````

</details>


https://github.com/llvm/llvm-project/pull/155205