[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