-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[ADT] Use adl_being
/adl_end
in make_early_inc_range
#130518
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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. Also fix potential issue with double-move on the input range.
@llvm/pr-subscribers-llvm-adt Author: Jakub Kuderski (kuhar) ChangesThis 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. Also fix potential issue with double-move on the input range. Full diff: https://github.com/llvm/llvm-project/pull/130518.diff 2 Files Affected:
diff --git a/llvm/include/llvm/ADT/STLExtras.h b/llvm/include/llvm/ADT/STLExtras.h
index ace5f60b572d7..0e4de4f9e1c88 100644
--- a/llvm/include/llvm/ADT/STLExtras.h
+++ b/llvm/include/llvm/ADT/STLExtras.h
@@ -657,8 +657,8 @@ iterator_range<early_inc_iterator_impl<detail::IterOfRange<RangeT>>>
make_early_inc_range(RangeT &&Range) {
using EarlyIncIteratorT =
early_inc_iterator_impl<detail::IterOfRange<RangeT>>;
- return make_range(EarlyIncIteratorT(std::begin(std::forward<RangeT>(Range))),
- EarlyIncIteratorT(std::end(std::forward<RangeT>(Range))));
+ return make_range(EarlyIncIteratorT(adl_begin(Range)),
+ EarlyIncIteratorT(adl_end(Range)));
}
// Forward declarations required by zip_shortest/zip_equal/zip_first/zip_longest
diff --git a/llvm/unittests/ADT/STLExtrasTest.cpp b/llvm/unittests/ADT/STLExtrasTest.cpp
index 406ff2bc16073..b700f7d7f4404 100644
--- a/llvm/unittests/ADT/STLExtrasTest.cpp
+++ b/llvm/unittests/ADT/STLExtrasTest.cpp
@@ -767,7 +767,7 @@ TEST(STLExtrasTest, EarlyIncrementTest) {
#endif
// Inserting shouldn't break anything. We should be able to keep dereferencing
- // the currrent iterator and increment. The increment to go to the "next"
+ // the current iterator and increment. The increment to go to the "next"
// iterator from before we inserted.
L.insert(std::next(L.begin(), 2), -1);
++I;
@@ -781,6 +781,14 @@ TEST(STLExtrasTest, EarlyIncrementTest) {
EXPECT_EQ(EIR.end(), I);
}
+TEST(STLExtrasTest, EarlyIncADLTest) {
+ // Make sure that we use the `begin`/`end` functions from `some_namespace`,
+ // using ADL.
+ some_namespace::some_struct S;
+ S.data = {1, 2, 3};
+ EXPECT_THAT(make_early_inc_range(S), ElementsAre(1, 2, 3));
+}
+
// A custom iterator that returns a pointer when dereferenced. This is used to
// test make_early_inc_range with iterators that do not return a reference on
// dereferencing.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/12359 Here is the relevant piece of the build log for the reference
|
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.
Also fix potential issue with double-move on the input range.