-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[ADT] Refactor MapVector::insert, try_emplace, and operator[] (NFC) #155205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ADT] Refactor MapVector::insert, try_emplace, and operator[] (NFC) #155205
Conversation
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/pr-subscribers-llvm-adt Author: Kazu Hirata (kazutakahirata) ChangesThe l-value and r-value reference variants of try_emplace contain This patch introduces a templated helper function, try_emplace_impl, This patch moves "using" declarations to the top of the class because Full diff: https://github.com/llvm/llvm-project/pull/155205.diff 1 Files Affected:
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>
|
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 the "private:" section to the end of the class so
that the new helper function can use iterator.