Skip to content

Conversation

@kuhar
Copy link
Member

@kuhar kuhar commented Mar 9, 2025

This is to make sure that ADT helpers consistently use argument
dependent lookup when dealing with input ranges.

This was a part of #87936 but reverted due to buildbot failures.

This PR is stacked on top of #130508.

@llvmbot
Copy link
Member

llvmbot commented Mar 9, 2025

@llvm/pr-subscribers-llvm-adt

Author: Jakub Kuderski (kuhar)

Changes

This is to make sure that ADT helpers consistently use argument
dependent lookup when dealing with input ranges.

This was a part of #87936 but reverted due to buildbot failures.

This PR is stacked on top of #130508.


Full diff: https://github.com/llvm/llvm-project/pull/130521.diff

2 Files Affected:

  • (modified) llvm/include/llvm/ADT/STLExtras.h (+3-4)
  • (modified) llvm/unittests/ADT/STLExtrasTest.cpp (+30)
diff --git a/llvm/include/llvm/ADT/STLExtras.h b/llvm/include/llvm/ADT/STLExtras.h
index fe1d010574e31..8600c59d9306d 100644
--- a/llvm/include/llvm/ADT/STLExtras.h
+++ b/llvm/include/llvm/ADT/STLExtras.h
@@ -376,8 +376,7 @@ inline mapped_iterator<ItTy, FuncTy> map_iterator(ItTy I, FuncTy F) {
 
 template <class ContainerTy, class FuncTy>
 auto map_range(ContainerTy &&C, FuncTy F) {
-  return make_range(map_iterator(std::begin(C), F),
-                    map_iterator(std::end(C), F));
+  return make_range(map_iterator(adl_begin(C), F), map_iterator(adl_end(C), F));
 }
 
 /// A base type of mapped iterator, that is useful for building derived
@@ -1438,7 +1437,7 @@ template <typename EltTy, typename FirstTy> class first_or_second_type {
 
 /// Given a container of pairs, return a range over the first elements.
 template <typename ContainerTy> auto make_first_range(ContainerTy &&c) {
-  using EltTy = decltype((*std::begin(c)));
+  using EltTy = decltype(*adl_begin(c));
   return llvm::map_range(std::forward<ContainerTy>(c),
                          [](EltTy elt) -> typename detail::first_or_second_type<
                                            EltTy, decltype((elt.first))>::type {
@@ -1448,7 +1447,7 @@ template <typename ContainerTy> auto make_first_range(ContainerTy &&c) {
 
 /// Given a container of pairs, return a range over the second elements.
 template <typename ContainerTy> auto make_second_range(ContainerTy &&c) {
-  using EltTy = decltype((*std::begin(c)));
+  using EltTy = decltype(*adl_begin(c));
   return llvm::map_range(
       std::forward<ContainerTy>(c),
       [](EltTy elt) ->
diff --git a/llvm/unittests/ADT/STLExtrasTest.cpp b/llvm/unittests/ADT/STLExtrasTest.cpp
index e107cb570641b..6d1478a9102a6 100644
--- a/llvm/unittests/ADT/STLExtrasTest.cpp
+++ b/llvm/unittests/ADT/STLExtrasTest.cpp
@@ -421,6 +421,15 @@ void swap(some_struct &lhs, some_struct &rhs) {
   rhs.swap_val = "rhs";
 }
 
+struct Pairs {
+  std::vector<std::pair<std::string, int>> data;
+  using const_iterator =
+      std::vector<std::pair<std::string, int>>::const_iterator;
+};
+
+Pairs::const_iterator begin(const Pairs &p) { return p.data.begin(); }
+Pairs::const_iterator end(const Pairs &p) { return p.data.end(); }
+
 struct requires_move {};
 int *begin(requires_move &&) { return nullptr; }
 int *end(requires_move &&) { return nullptr; }
@@ -504,6 +513,15 @@ TEST(STLExtrasTest, ConcatRange) {
   EXPECT_EQ(Expected, Test);
 }
 
+TEST(STLExtrasTest, MakeFirstSecondRangeADL) {
+  // Make sure that we use the `begin`/`end` functions from `some_namespace`,
+  // using ADL.
+  some_namespace::Pairs Pairs;
+  Pairs.data = {{"foo", 1}, {"bar", 2}};
+  EXPECT_THAT(make_first_range(Pairs), ElementsAre("foo", "bar"));
+  EXPECT_THAT(make_second_range(Pairs), ElementsAre(1, 2));
+}
+
 template <typename T> struct Iterator {
   int i = 0;
   T operator*() const { return i; }
@@ -730,6 +748,18 @@ TEST(STLExtrasTest, DropEndDefaultTest) {
   EXPECT_EQ(i, 4);
 }
 
+TEST(STLExtrasTest, MapRangeTest) {
+  SmallVector<int, 5> Vec{0, 1, 2};
+  EXPECT_THAT(map_range(Vec, [](int V) { return V + 1; }),
+              ElementsAre(1, 2, 3));
+
+  // Make sure that we use the `begin`/`end` functions
+  // from `some_namespace`, using ADL.
+  some_namespace::some_struct S;
+  S.data = {3, 4, 5};
+  EXPECT_THAT(map_range(S, [](int V) { return V * 2; }), ElementsAre(6, 8, 10));
+}
+
 TEST(STLExtrasTest, EarlyIncrementTest) {
   std::list<int> L = {1, 2, 3, 4};
 

Copy link
Contributor

@kazutakahirata kazutakahirata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

This is to make sure that ADT helpers consistently use argument
dependent lookup when dealing with input ranges.

This was a part of llvm#87936 but reverted due to buildbot failures.

This PR is stacked on top of llvm#130508.
@kuhar kuhar force-pushed the adl-first-second-range branch from 0139378 to 1904e2e Compare March 12, 2025 02:42
@kuhar kuhar merged commit e427f06 into llvm:main Mar 12, 2025
6 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants