Skip to content

Commit 0eaacc2

Browse files
committed
[ADT] Make llvm::is_contained call member contains or find when available
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`. For C++17 maps/sets without `.contains`, use `.find` when available, falling back to a linear scan with `std::find`. I also considered detecting member contains and triggering a `static_assert` instead, but decided against it because it's just as easy to do the right thing and call `.contains`. This would also make some code fail only when compiled in the C++20 mode when more container types come with `.contains` member functions. This was actually already the case with `CommandLine.h` calling `is_contained` on `SmallPtrSet` and in a recent BOLT patch. Reviewed By: kazu, dblaikie, MaskRay Differential Revision: https://reviews.llvm.org/D146061
1 parent d8df871 commit 0eaacc2

File tree

3 files changed

+83
-4
lines changed

3 files changed

+83
-4
lines changed

llvm/include/llvm/ADT/STLExtras.h

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1906,11 +1906,40 @@ OutputIt move(R &&Range, OutputIt Out) {
19061906
return std::move(adl_begin(Range), adl_end(Range), Out);
19071907
}
19081908

1909-
/// Wrapper function around std::find to detect if an element exists
1910-
/// in a container.
1909+
namespace detail {
1910+
template <typename Range, typename Element>
1911+
using check_has_member_contains_t =
1912+
decltype(std::declval<Range &>().contains(std::declval<const Element &>()));
1913+
1914+
template <typename Range, typename Element>
1915+
static constexpr bool HasMemberContains =
1916+
is_detected<check_has_member_contains_t, Range, Element>::value;
1917+
1918+
template <typename Range, typename Element>
1919+
using check_has_member_find_t =
1920+
decltype(std::declval<Range &>().find(std::declval<const Element &>()) !=
1921+
std::declval<Range &>().end());
1922+
1923+
template <typename Range, typename Element>
1924+
static constexpr bool HasMemberFind =
1925+
is_detected<check_has_member_find_t, Range, Element>::value;
1926+
1927+
} // namespace detail
1928+
1929+
/// Returns true if \p Element is found in \p Range. Delegates the check to
1930+
/// either `.contains(Element)`, `.find(Element)`, or `std::find`, in this
1931+
/// order of preference. This is intended as the canonical way to check if an
1932+
/// element exists in a range in generic code or range type that does not
1933+
/// expose a `.contains(Element)` member.
19111934
template <typename R, typename E>
19121935
bool is_contained(R &&Range, const E &Element) {
1913-
return std::find(adl_begin(Range), adl_end(Range), Element) != adl_end(Range);
1936+
if constexpr (detail::HasMemberContains<R, E>)
1937+
return Range.contains(Element);
1938+
else if constexpr (detail::HasMemberFind<R, E>)
1939+
return Range.find(Element) != Range.end();
1940+
else
1941+
return std::find(adl_begin(Range), adl_end(Range), Element) !=
1942+
adl_end(Range);
19141943
}
19151944

19161945
/// Returns true iff \p Element exists in \p Set. This overload takes \p Set as

llvm/include/llvm/Analysis/LoopInfoImpl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ void LoopBase<BlockT, LoopT>::verifyLoop() const {
371371

372372
// Check the parent loop pointer.
373373
if (ParentLoop) {
374-
assert(is_contained(*ParentLoop, this) &&
374+
assert(is_contained(ParentLoop->getSubLoops(), this) &&
375375
"Loop is not a subloop of its parent!");
376376
}
377377
#endif

llvm/unittests/ADT/STLExtrasTest.cpp

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,6 +1029,56 @@ TEST(STLExtrasTest, IsContainedInitializerList) {
10291029
static_assert(!is_contained({1, 2, 3, 4}, 5), "It's not there :(");
10301030
}
10311031

1032+
TEST(STLExtrasTest, IsContainedMemberContains) {
1033+
// Check that `llvm::is_contained` uses the member `.contains()` when
1034+
// available. Check that `.contains()` is preferred over `.find()`.
1035+
struct Foo {
1036+
bool contains(int) const {
1037+
++NumContainsCalls;
1038+
return ContainsResult;
1039+
}
1040+
int *begin() { return nullptr; }
1041+
int *end() { return nullptr; }
1042+
int *find(int) { return nullptr; }
1043+
1044+
bool ContainsResult = false;
1045+
mutable unsigned NumContainsCalls = 0;
1046+
} Container;
1047+
1048+
EXPECT_EQ(Container.NumContainsCalls, 0u);
1049+
EXPECT_FALSE(is_contained(Container, 1));
1050+
EXPECT_EQ(Container.NumContainsCalls, 1u);
1051+
1052+
Container.ContainsResult = true;
1053+
EXPECT_TRUE(is_contained(Container, 1));
1054+
EXPECT_EQ(Container.NumContainsCalls, 2u);
1055+
}
1056+
1057+
TEST(STLExtrasTest, IsContainedMemberFind) {
1058+
// Check that `llvm::is_contained` uses the member `.find(x)` when available.
1059+
struct Foo {
1060+
auto begin() { return Data.begin(); }
1061+
auto end() { return Data.end(); }
1062+
auto find(int X) {
1063+
++NumFindCalls;
1064+
return std::find(begin(), end(), X);
1065+
}
1066+
1067+
std::vector<int> Data;
1068+
mutable unsigned NumFindCalls = 0;
1069+
} Container;
1070+
1071+
Container.Data = {1, 2, 3};
1072+
1073+
EXPECT_EQ(Container.NumFindCalls, 0u);
1074+
EXPECT_TRUE(is_contained(Container, 1));
1075+
EXPECT_TRUE(is_contained(Container, 3));
1076+
EXPECT_EQ(Container.NumFindCalls, 2u);
1077+
1078+
EXPECT_FALSE(is_contained(Container, 4));
1079+
EXPECT_EQ(Container.NumFindCalls, 3u);
1080+
}
1081+
10321082
TEST(STLExtrasTest, addEnumValues) {
10331083
enum A { Zero = 0, One = 1 };
10341084
enum B { IntMax = INT_MAX, ULongLongMax = ULLONG_MAX };

0 commit comments

Comments
 (0)