[PATCH] D146061: [ADT] Make llvm::is_contained call member `contains` when available

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar created this revision.
kuhar added reviewers: kazu, dblaikie, zero9178, MaskRay.
Herald added a subscriber: hanchung.
Herald added a project: All.
kuhar requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This makes it so that calling `llvm::is_contained` no longer degrades
performance over member contains, even though both have almost identical
names. This would be the case in most set/map classes that can check for
an element being present in O(1) or O(log n) time vs. linear scan with
`std::find`.

I also considered detecting member contains and triggering a
`static_assert` instead, but decied against it because it's just as easy
to do the right thing and call `.contains`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146061

Files:
  llvm/include/llvm/ADT/STLExtras.h
  llvm/unittests/ADT/STLExtrasTest.cpp


Index: llvm/unittests/ADT/STLExtrasTest.cpp
===================================================================
--- llvm/unittests/ADT/STLExtrasTest.cpp
+++ llvm/unittests/ADT/STLExtrasTest.cpp
@@ -1029,6 +1029,26 @@
   static_assert(!is_contained({1, 2, 3, 4}, 5), "It's not there :(");
 }
 
+TEST(STLExtrasTest, IsContainedMemberContains) {
+  // Check that `llvm::is_contained` uses the `.contains()` when available.
+  struct Foo {
+    bool contains(int) const {
+      ++NumContainsCalls;
+      return ContainsResult;
+    }
+    bool ContainsResult = false;
+    mutable unsigned NumContainsCalls = 0;
+  } Container;
+
+  EXPECT_EQ(Container.NumContainsCalls, 0u);
+  EXPECT_FALSE(is_contained(Container, 1));
+  EXPECT_EQ(Container.NumContainsCalls, 1u);
+
+  Container.ContainsResult = true;
+  EXPECT_TRUE(is_contained(Container, 1));
+  EXPECT_EQ(Container.NumContainsCalls, 2u);
+}
+
 TEST(STLExtrasTest, addEnumValues) {
   enum A { Zero = 0, One = 1 };
   enum B { IntMax = INT_MAX, ULongLongMax = ULLONG_MAX };
Index: llvm/include/llvm/ADT/STLExtras.h
===================================================================
--- llvm/include/llvm/ADT/STLExtras.h
+++ llvm/include/llvm/ADT/STLExtras.h
@@ -1906,11 +1906,24 @@
   return std::move(adl_begin(Range), adl_end(Range), Out);
 }
 
-/// Wrapper function around std::find to detect if an element exists
-/// in a container.
+namespace detail {
+template <typename Range, typename Element>
+using check_has_member_contains_t =
+    decltype(std::declval<Range &>().contains(std::declval<const Element &>()));
+
+template <typename Range, typename Element>
+static constexpr bool HasMemberContains =
+    is_detected<check_has_member_contains_t, Range, Element>::value;
+} // namespace detail
+
+/// Returns true if \p Element is found in \p Range. This either calls
+/// `.contains()` for range types `R` that define it, or `llvm::find`.
 template <typename R, typename E>
 bool is_contained(R &&Range, const E &Element) {
-  return std::find(adl_begin(Range), adl_end(Range), Element) != adl_end(Range);
+  if constexpr (detail::HasMemberContains<R, E>)
+    return Range.contains(Element);
+  else
+    return llvm::find(Range, Element) != adl_end(Range);
 }
 
 /// Returns true iff \p Element exists in \p Set. This overload takes \p Set as


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D146061.505137.patch
Type: text/x-patch
Size: 2312 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230314/519967ca/attachment.bin>