[llvm][ADT] Structured bindings for move-only types in StringMap
#114676
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-adt Author: Jan Svoboda (jansvoboda11) ChangesThis PR implements destructuring of Full diff: https://github.com/llvm/llvm-project/pull/114676.diff 2 Files Affected:
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 {
|
kuhar
requested changes
Nov 2, 2024
kuhar
approved these changes
Nov 3, 2024
| static_assert(Index == 0 || Index == 1); | ||
| if constexpr (Index == 0) | ||
| return E.getKey(); | ||
| if constexpr (Index == 1) |
There was a problem hiding this comment.
since this is guarded by the static assert above, we can use else
| else | ||
| return E.second; | ||
| return E.getKey(); | ||
| if constexpr (Index == 1) |
|
Thank you for the review! |
jansvoboda11
deleted the
string-map-entry-structured-bindings-move-only
branch
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/35/builds/3300 Here is the relevant piece of the build log for the reference |
smallp-o-p
pushed a commit
to smallp-o-p/llvm-project
that referenced
this pull request
Nov 3, 2024
…lvm#114676) This PR implements destructuring of `StringMapEntry<T>` where `T` is a move-only type. Also adds the non-const version.
NoumanAmir657
pushed a commit
to NoumanAmir657/llvm-project
that referenced
this pull request
Nov 4, 2024
…lvm#114676) This PR implements destructuring of `StringMapEntry<T>` where `T` is a move-only type. Also adds the non-const version.
PhilippRados
pushed a commit
to PhilippRados/llvm-project
that referenced
this pull request
Nov 6, 2024
…lvm#114676) This PR implements destructuring of `StringMapEntry<T>` where `T` is a move-only type. Also adds the non-const version.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements destructuring of
StringMapEntry<T>whereTis a move-only type. Also adds the non-const version.