[libc++] Refactor ranges::{min, max, min_element, max_element} to use std::__min_element #132418
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
llvmbot
added
the
libc++
Member
|
@llvm/pr-subscribers-libcxx Author: Peng Liu (winner245) ChangesPreviously, Upon removal of This refactoring unifies the implementation across these algorithms, ensuring that future optimizations or maintenance work only need to be applied in one place. Full diff: https://github.com/llvm/llvm-project/pull/132418.diff 5 Files Affected:
diff --git a/libcxx/include/__algorithm/min_element.h b/libcxx/include/__algorithm/min_element.h
index db996365bf1de..fdab63aefec7e 100644
--- a/libcxx/include/__algorithm/min_element.h
+++ b/libcxx/include/__algorithm/min_element.h
@@ -29,7 +29,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD
template <class _Comp, class _Iter, class _Sent, class _Proj>
inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 _Iter
-__min_element(_Iter __first, _Sent __last, _Comp __comp, _Proj& __proj) {
+__min_element(_Iter __first, _Sent __last, _Comp& __comp, _Proj& __proj) {
if (__first == __last)
return __first;
diff --git a/libcxx/include/__algorithm/ranges_max.h b/libcxx/include/__algorithm/ranges_max.h
index f631344422ed5..a8fe13a734f5b 100644
--- a/libcxx/include/__algorithm/ranges_max.h
+++ b/libcxx/include/__algorithm/ranges_max.h
@@ -9,7 +9,7 @@
#ifndef _LIBCPP___ALGORITHM_RANGES_MAX_H
#define _LIBCPP___ALGORITHM_RANGES_MAX_H
-#include <__algorithm/ranges_min_element.h>
+#include <__algorithm/min_element.h>
#include <__assert>
#include <__concepts/copyable.h>
#include <__config>
@@ -57,7 +57,7 @@ struct __max {
__il.begin() != __il.end(), "initializer_list must contain at least one element");
auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) -> bool { return std::invoke(__comp, __rhs, __lhs); };
- return *ranges::__min_element_impl(__il.begin(), __il.end(), __comp_lhs_rhs_swapped, __proj);
+ return *std::__min_element(__il.begin(), __il.end(), __comp_lhs_rhs_swapped, __proj);
}
template <input_range _Rp,
@@ -75,7 +75,7 @@ struct __max {
auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) -> bool {
return std::invoke(__comp, __rhs, __lhs);
};
- return *ranges::__min_element_impl(std::move(__first), std::move(__last), __comp_lhs_rhs_swapped, __proj);
+ return *std::__min_element(std::move(__first), std::move(__last), __comp_lhs_rhs_swapped, __proj);
} else {
range_value_t<_Rp> __result = *__first;
while (++__first != __last) {
diff --git a/libcxx/include/__algorithm/ranges_max_element.h b/libcxx/include/__algorithm/ranges_max_element.h
index 869f71ecc8d20..db6d5f6b9c276 100644
--- a/libcxx/include/__algorithm/ranges_max_element.h
+++ b/libcxx/include/__algorithm/ranges_max_element.h
@@ -9,7 +9,7 @@
#ifndef _LIBCPP___ALGORITHM_RANGES_MAX_ELEMENT_H
#define _LIBCPP___ALGORITHM_RANGES_MAX_ELEMENT_H
-#include <__algorithm/ranges_min_element.h>
+#include <__algorithm/min_element.h>
#include <__config>
#include <__functional/identity.h>
#include <__functional/invoke.h>
@@ -40,7 +40,7 @@ struct __max_element {
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Ip
operator()(_Ip __first, _Sp __last, _Comp __comp = {}, _Proj __proj = {}) const {
auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) -> bool { return std::invoke(__comp, __rhs, __lhs); };
- return ranges::__min_element_impl(__first, __last, __comp_lhs_rhs_swapped, __proj);
+ return std::__min_element(__first, __last, __comp_lhs_rhs_swapped, __proj);
}
template <forward_range _Rp,
@@ -49,7 +49,7 @@ struct __max_element {
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr borrowed_iterator_t<_Rp>
operator()(_Rp&& __r, _Comp __comp = {}, _Proj __proj = {}) const {
auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) -> bool { return std::invoke(__comp, __rhs, __lhs); };
- return ranges::__min_element_impl(ranges::begin(__r), ranges::end(__r), __comp_lhs_rhs_swapped, __proj);
+ return std::__min_element(ranges::begin(__r), ranges::end(__r), __comp_lhs_rhs_swapped, __proj);
}
};
diff --git a/libcxx/include/__algorithm/ranges_min.h b/libcxx/include/__algorithm/ranges_min.h
index 302b5d7975b00..9f1c78eaa9e25 100644
--- a/libcxx/include/__algorithm/ranges_min.h
+++ b/libcxx/include/__algorithm/ranges_min.h
@@ -9,7 +9,7 @@
#ifndef _LIBCPP___ALGORITHM_RANGES_MIN_H
#define _LIBCPP___ALGORITHM_RANGES_MIN_H
-#include <__algorithm/ranges_min_element.h>
+#include <__algorithm/min_element.h>
#include <__assert>
#include <__concepts/copyable.h>
#include <__config>
@@ -54,7 +54,7 @@ struct __min {
operator()(initializer_list<_Tp> __il, _Comp __comp = {}, _Proj __proj = {}) const {
_LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
__il.begin() != __il.end(), "initializer_list must contain at least one element");
- return *ranges::__min_element_impl(__il.begin(), __il.end(), __comp, __proj);
+ return *std::__min_element(__il.begin(), __il.end(), __comp, __proj);
}
template <input_range _Rp,
@@ -67,7 +67,7 @@ struct __min {
auto __last = ranges::end(__r);
_LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__first != __last, "range must contain at least one element");
if constexpr (forward_range<_Rp> && !__is_cheap_to_copy<range_value_t<_Rp>>) {
- return *ranges::__min_element_impl(__first, __last, __comp, __proj);
+ return *std::__min_element(__first, __last, __comp, __proj);
} else {
range_value_t<_Rp> __result = *__first;
while (++__first != __last) {
diff --git a/libcxx/include/__algorithm/ranges_min_element.h b/libcxx/include/__algorithm/ranges_min_element.h
index fb92ae56bcd62..5deb409ccd85e 100644
--- a/libcxx/include/__algorithm/ranges_min_element.h
+++ b/libcxx/include/__algorithm/ranges_min_element.h
@@ -9,6 +9,7 @@
#ifndef _LIBCPP___ALGORITHM_RANGES_MIN_ELEMENT_H
#define _LIBCPP___ALGORITHM_RANGES_MIN_ELEMENT_H
+#include <__algorithm/min_element.h>
#include <__config>
#include <__functional/identity.h>
#include <__functional/invoke.h>
@@ -32,20 +33,6 @@ _LIBCPP_PUSH_MACROS
_LIBCPP_BEGIN_NAMESPACE_STD
namespace ranges {
-
-// TODO(ranges): `ranges::min_element` can now simply delegate to `std::__min_element`.
-template <class _Ip, class _Sp, class _Proj, class _Comp>
-_LIBCPP_HIDE_FROM_ABI constexpr _Ip __min_element_impl(_Ip __first, _Sp __last, _Comp& __comp, _Proj& __proj) {
- if (__first == __last)
- return __first;
-
- _Ip __i = __first;
- while (++__i != __last)
- if (std::invoke(__comp, std::invoke(__proj, *__i), std::invoke(__proj, *__first)))
- __first = __i;
- return __first;
-}
-
struct __min_element {
template <forward_iterator _Ip,
sentinel_for<_Ip> _Sp,
@@ -53,7 +40,7 @@ struct __min_element {
indirect_strict_weak_order<projected<_Ip, _Proj>> _Comp = ranges::less>
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Ip
operator()(_Ip __first, _Sp __last, _Comp __comp = {}, _Proj __proj = {}) const {
- return ranges::__min_element_impl(__first, __last, __comp, __proj);
+ return std::__min_element(__first, __last, __comp, __proj);
}
template <forward_range _Rp,
@@ -61,7 +48,7 @@ struct __min_element {
indirect_strict_weak_order<projected<iterator_t<_Rp>, _Proj>> _Comp = ranges::less>
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr borrowed_iterator_t<_Rp>
operator()(_Rp&& __r, _Comp __comp = {}, _Proj __proj = {}) const {
- return ranges::__min_element_impl(ranges::begin(__r), ranges::end(__r), __comp, __proj);
+ return std::__min_element(ranges::begin(__r), ranges::end(__r), __comp, __proj);
}
};
|
ldionne
approved these changes
Mar 27, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Previously,
ranges::min_elementdelegated toranges::__min_element_impl, which duplicated the definition ofstd::__min_element. This patch updatesranges::min_elementto directly callstd::__min_element, which allows us to remove the redundant code inranges::__min_element_impl.Upon removal of
ranges::__min_element_impl, the other ranges algorithmsranges::{min, max, max_element}, which previously delegated toranges::__min_element_impl, have been updated to callstd::__min_elementinstead.This refactoring unifies the implementation across these algorithms, ensuring that future optimizations or maintenance work only need to be applied in one place.