[llvm] [ADT] Refactor MapVector::insert, try_emplace, and operator[] (NFC) (PR #155205)
Kazu Hirata via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 25 08:08:25 PDT 2025
https://github.com/kazutakahirata updated https://github.com/llvm/llvm-project/pull/155205
>From 99afb55c68c1c5da250f6ffdba2bc3460965d736 Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Sun, 24 Aug 2025 17:29:21 -0700
Subject: [PATCH 1/3] [ADT] Refactor MapVector::insert, try_emplace, and
operator[] (NFC)
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.
---
llvm/include/llvm/ADT/MapVector.h | 60 ++++++++++++++-----------------
1 file changed, 27 insertions(+), 33 deletions(-)
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>
>From 3e088c4026e5fb6a4c90cd4f2029661561593aa4 Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Mon, 25 Aug 2025 07:28:13 -0700
Subject: [PATCH 2/3] Address a comment.
---
llvm/include/llvm/ADT/MapVector.h | 43 +++++++++++++++----------------
1 file changed, 21 insertions(+), 22 deletions(-)
diff --git a/llvm/include/llvm/ADT/MapVector.h b/llvm/include/llvm/ADT/MapVector.h
index 5f9f2b6794bc6..7b6735c94b53d 100644
--- a/llvm/include/llvm/ADT/MapVector.h
+++ b/llvm/include/llvm/ADT/MapVector.h
@@ -44,28 +44,6 @@ 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();
@@ -218,6 +196,27 @@ class MapVector {
/// Erase all elements that match \c Pred in a single pass. Takes linear
/// time.
template <class Predicate> void remove_if(Predicate Pred);
+
+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};
+ }
};
template <typename KeyT, typename ValueT, typename MapType, typename VectorType>
>From c20b30ca6ef6f3977c53da75111aa21c4107b165 Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Mon, 25 Aug 2025 08:08:04 -0700
Subject: [PATCH 3/3] Use structured binding.
---
llvm/include/llvm/ADT/MapVector.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/llvm/include/llvm/ADT/MapVector.h b/llvm/include/llvm/ADT/MapVector.h
index 7b6735c94b53d..4a50126ff5aad 100644
--- a/llvm/include/llvm/ADT/MapVector.h
+++ b/llvm/include/llvm/ADT/MapVector.h
@@ -207,15 +207,15 @@ class MapVector {
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();
+ auto [It, Inserted] = Map.try_emplace(Key);
+ if (Inserted) {
+ It->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};
+ return {begin() + It->second, false};
}
};
More information about the llvm-commits
mailing list