[llvm] [llvm][ADT] Structured bindings for move-only types in `StringMap` (PR #114676)
Jan Svoboda via llvm-commits
llvm-commits at lists.llvm.org
Sat Nov 2 10:52:34 PDT 2024
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/114676
>From 2bd84494c6b52447b82065bd441c94b3fe405b61 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan at svoboda.ai>
Date: Fri, 1 Nov 2024 21:53:00 -0700
Subject: [PATCH 1/3] [llvm][ADT] Structured bindings for move-only types in
`StringMap`
---
llvm/include/llvm/ADT/StringMapEntry.h | 56 ++++++++++++++++++++------
llvm/unittests/ADT/StringMapTest.cpp | 23 +++++++++++
2 files changed, 67 insertions(+), 12 deletions(-)
diff --git a/llvm/include/llvm/ADT/StringMapEntry.h b/llvm/include/llvm/ADT/StringMapEntry.h
index d93af5aedc39d7..03e3ae0fe51d6b 100644
--- a/llvm/include/llvm/ADT/StringMapEntry.h
+++ b/llvm/include/llvm/ADT/StringMapEntry.h
@@ -17,7 +17,7 @@
#define LLVM_ADT_STRINGMAPENTRY_H
#include "llvm/ADT/StringRef.h"
-#include <optional>
+#include <utility>
namespace llvm {
@@ -147,25 +147,57 @@ class StringMapEntry final : public StringMapEntryStorage<ValueTy> {
};
// Allow structured bindings on StringMapEntry.
+
+namespace detail {
+template <std::size_t Index> struct StringMapEntryGet;
+
+template <> struct StringMapEntryGet<0> {
+ template <typename ValueTy> static StringRef get(StringMapEntry<ValueTy> &E) {
+ return E.getKey();
+ }
+
+ template <typename ValueTy>
+ static StringRef get(const StringMapEntry<ValueTy> &E) {
+ return E.getKey();
+ }
+};
+
+template <> struct StringMapEntryGet<1> {
+ template <typename ValueTy> static ValueTy &get(StringMapEntry<ValueTy> &E) {
+ return E.getValue();
+ }
+
+ template <typename ValueTy>
+ static const ValueTy &get(const StringMapEntry<ValueTy> &E) {
+ return E.getValue();
+ }
+};
+} // namespace detail
+
+template <std::size_t Index, typename ValueTy>
+decltype(auto) get(StringMapEntry<ValueTy> &E) {
+ return detail::StringMapEntryGet<Index>::get(E);
+}
+
template <std::size_t Index, typename ValueTy>
decltype(auto) get(const StringMapEntry<ValueTy> &E) {
- static_assert(Index < 2);
- if constexpr (Index == 0)
- return E.first();
- else
- return E.second;
+ return detail::StringMapEntryGet<Index>::get(E);
}
} // end namespace llvm
-namespace std {
template <typename ValueTy>
-struct tuple_size<llvm::StringMapEntry<ValueTy>>
+struct std::tuple_size<llvm::StringMapEntry<ValueTy>>
: std::integral_constant<std::size_t, 2> {};
-template <std::size_t I, typename ValueTy>
-struct tuple_element<I, llvm::StringMapEntry<ValueTy>>
- : std::conditional<I == 0, llvm::StringRef, ValueTy> {};
-} // namespace std
+template <typename ValueTy>
+struct std::tuple_element<0, llvm::StringMapEntry<ValueTy>> {
+ using type = llvm::StringRef;
+};
+
+template <typename ValueTy>
+struct std::tuple_element<1, llvm::StringMapEntry<ValueTy>> {
+ using type = ValueTy;
+};
#endif // LLVM_ADT_STRINGMAPENTRY_H
diff --git a/llvm/unittests/ADT/StringMapTest.cpp b/llvm/unittests/ADT/StringMapTest.cpp
index c9ef3f8a096ee9..35135cdb297e79 100644
--- a/llvm/unittests/ADT/StringMapTest.cpp
+++ b/llvm/unittests/ADT/StringMapTest.cpp
@@ -381,6 +381,9 @@ struct MoveOnly {
return *this;
}
+ bool operator==(const MoveOnly &RHS) const { return i == RHS.i; }
+ bool operator!=(const MoveOnly &RHS) const { return i != RHS.i; }
+
private:
MoveOnly(const MoveOnly &) = delete;
MoveOnly &operator=(const MoveOnly &) = delete;
@@ -550,6 +553,26 @@ TEST_F(StringMapTest, StructuredBindings) {
EXPECT_EQ("a", Key);
EXPECT_EQ(42, Value);
}
+
+ for (const auto &[Key, Value] : A) {
+ EXPECT_EQ("a", Key);
+ EXPECT_EQ(42, Value);
+ }
+}
+
+TEST_F(StringMapTest, StructuredBindingsMoveOnly) {
+ StringMap<MoveOnly> A;
+ A.insert(std::make_pair("a", MoveOnly(42)));
+
+ for (auto &[Key, Value] : A) {
+ EXPECT_EQ("a", Key);
+ EXPECT_EQ(MoveOnly(42), Value);
+ }
+
+ for (const auto &[Key, Value] : A) {
+ EXPECT_EQ("a", Key);
+ EXPECT_EQ(MoveOnly(42), Value);
+ }
}
namespace {
>From b34da91d7f112b6d387c69bea6271efd6df69347 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan at svoboda.ai>
Date: Sat, 2 Nov 2024 10:33:37 -0700
Subject: [PATCH 2/3] Use `if constexpr` instead of struct template
---
llvm/include/llvm/ADT/StringMapEntry.h | 38 +++++++-------------------
1 file changed, 10 insertions(+), 28 deletions(-)
diff --git a/llvm/include/llvm/ADT/StringMapEntry.h b/llvm/include/llvm/ADT/StringMapEntry.h
index 03e3ae0fe51d6b..b167c991749b32 100644
--- a/llvm/include/llvm/ADT/StringMapEntry.h
+++ b/llvm/include/llvm/ADT/StringMapEntry.h
@@ -148,40 +148,22 @@ class StringMapEntry final : public StringMapEntryStorage<ValueTy> {
// Allow structured bindings on StringMapEntry.
-namespace detail {
-template <std::size_t Index> struct StringMapEntryGet;
-
-template <> struct StringMapEntryGet<0> {
- template <typename ValueTy> static StringRef get(StringMapEntry<ValueTy> &E) {
- return E.getKey();
- }
-
- template <typename ValueTy>
- static StringRef get(const StringMapEntry<ValueTy> &E) {
- return E.getKey();
- }
-};
-
-template <> struct StringMapEntryGet<1> {
- template <typename ValueTy> static ValueTy &get(StringMapEntry<ValueTy> &E) {
- return E.getValue();
- }
-
- template <typename ValueTy>
- static const ValueTy &get(const StringMapEntry<ValueTy> &E) {
- return E.getValue();
- }
-};
-} // namespace detail
-
template <std::size_t Index, typename ValueTy>
decltype(auto) get(StringMapEntry<ValueTy> &E) {
- return detail::StringMapEntryGet<Index>::get(E);
+ static_assert(Index == 0 || Index == 1);
+ if constexpr (Index == 0)
+ return E.getKey();
+ if constexpr (Index == 1)
+ return E.getValue();
}
template <std::size_t Index, typename ValueTy>
decltype(auto) get(const StringMapEntry<ValueTy> &E) {
- return detail::StringMapEntryGet<Index>::get(E);
+ static_assert(Index == 0 || Index == 1);
+ if constexpr (Index == 0)
+ return E.getKey();
+ if constexpr (Index == 1)
+ return E.getValue();
}
} // end namespace llvm
>From cf073f191b52a36f401d5d32c48fd086309c73f3 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan at svoboda.ai>
Date: Sat, 2 Nov 2024 10:52:06 -0700
Subject: [PATCH 3/3] Revert `tuple_element` change, add assert
---
llvm/include/llvm/ADT/StringMapEntry.h | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/llvm/include/llvm/ADT/StringMapEntry.h b/llvm/include/llvm/ADT/StringMapEntry.h
index b167c991749b32..ceb3b74b932970 100644
--- a/llvm/include/llvm/ADT/StringMapEntry.h
+++ b/llvm/include/llvm/ADT/StringMapEntry.h
@@ -172,14 +172,10 @@ template <typename ValueTy>
struct std::tuple_size<llvm::StringMapEntry<ValueTy>>
: std::integral_constant<std::size_t, 2> {};
-template <typename ValueTy>
-struct std::tuple_element<0, llvm::StringMapEntry<ValueTy>> {
- using type = llvm::StringRef;
-};
-
-template <typename ValueTy>
-struct std::tuple_element<1, llvm::StringMapEntry<ValueTy>> {
- using type = ValueTy;
+template <std::size_t Index, typename ValueTy>
+struct std::tuple_element<Index, llvm::StringMapEntry<ValueTy>>
+ : std::conditional<Index == 0, llvm::StringRef, ValueTy> {
+ static_assert(Index == 0 || Index == 1);
};
#endif // LLVM_ADT_STRINGMAPENTRY_H
More information about the llvm-commits
mailing list