Skip to content
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

[libcxx] patch for implementing ranges::find_last #67270

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions libcxx/docs/FeatureTestMacroTable.rst
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,8 @@ Status
--------------------------------------------------- -----------------
``__cpp_lib_expected`` ``202211L``
--------------------------------------------------- -----------------
``__cpp_lib_find_last`` ``202207L``
--------------------------------------------------- -----------------
``__cpp_lib_format_ranges`` ``202207L``
--------------------------------------------------- -----------------
``__cpp_lib_formatters`` *unimplemented*
Expand Down
2 changes: 1 addition & 1 deletion libcxx/docs/ReleaseNotes/18.rst
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ Implemented Papers
- P2697R1 - Interfacing ``bitset`` with ``string_view``
- P2443R1 - ``views::chunk_by``
- P2538R1 - ADL-proof ``std::projected``

- P1223R5 - ``find_last``

Improvements and New Features
-----------------------------
Expand Down
2 changes: 1 addition & 1 deletion libcxx/docs/Status/Cxx23Papers.csv
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
"`P0429R9 <https://wg21.link/P0429R9>`__","LWG","A Standard ``flat_map``","July 2022","",""
"`P1169R4 <https://wg21.link/P1169R4>`__","LWG","``static operator()``","July 2022","|Complete|","16.0"
"`P1222R4 <https://wg21.link/P1222R4>`__","LWG","A Standard ``flat_set``","July 2022","",""
"`P1223R5 <https://wg21.link/P1223R5>`__","LWG","``ranges::find_last()``, ``ranges::find_last_if()``, and ``ranges::find_last_if_not()``","July 2022","","","|ranges|"
"`P1223R5 <https://wg21.link/P1223R5>`__","LWG","``ranges::find_last()``, ``ranges::find_last_if()``, and ``ranges::find_last_if_not()``","July 2022","|Complete|","18.0","|ranges|"
"`P1467R9 <https://wg21.link/P1467R9>`__","LWG","Extended ``floating-point`` types and standard names","July 2022","",""
"`P1642R11 <https://wg21.link/P1642R11>`__","LWG","Freestanding ``[utilities]``, ``[ranges]``, and ``[iterators]``","July 2022","",""
"`P1899R3 <https://wg21.link/P1899R3>`__","LWG","``stride_view``","July 2022","","","|ranges|"
Expand Down
3 changes: 3 additions & 0 deletions libcxx/include/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ set(files
__algorithm/ranges_find_first_of.h
__algorithm/ranges_find_if.h
__algorithm/ranges_find_if_not.h
__algorithm/ranges_find_last.h
__algorithm/ranges_find_last_if.h
__algorithm/ranges_find_last_if_not.h
__algorithm/ranges_for_each.h
__algorithm/ranges_for_each_n.h
__algorithm/ranges_generate.h
Expand Down
62 changes: 62 additions & 0 deletions libcxx/include/__algorithm/ranges_find_last.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef _LIBCPP___ALGORITHM_RANGES_FIND_LAST_H
#define _LIBCPP___ALGORITHM_RANGES_FIND_LAST_H

#include <__algorithm/ranges_find_last_if.h>
#include <__config>
#include <__functional/identity.h>
#include <__functional/ranges_operations.h>
#include <__iterator/concepts.h>
#include <__iterator/projected.h>
#include <__ranges/access.h>
#include <__ranges/concepts.h>
#include <__ranges/dangling.h>
#include <__utility/forward.h>
#include <__utility/move.h>

#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
#endif

#if _LIBCPP_STD_VER >= 23

_LIBCPP_BEGIN_NAMESPACE_STD

namespace ranges {
namespace __find_last {
struct __fn {
template <forward_iterator _Ip, sentinel_for<_Ip> _Sp, class _Tp, class _Proj = identity>
requires indirect_binary_predicate < ranges::equal_to, projected<_Ip, _Proj>,
const _Tp* > _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr subrange<_Ip>
operator()(_Ip __first, _Sp __last, const _Tp& __value, _Proj __proj = {}) const {
auto __pred = [&](auto&& __e) { return std::forward<decltype(__e)>(__e) == __value; };
return ranges::__find_last_if_impl(std::move(__first), std::move(__last), __pred, __proj);
}

template <forward_range _Rp, class _Tp, class _Proj = identity>
requires indirect_binary_predicate < ranges::equal_to, projected<iterator_t<_Rp>, _Proj>,
const _Tp* > _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr borrowed_subrange_t<_Rp>
operator()(_Rp&& __r, const _Tp& __value, _Proj __proj = {}) const {
auto __pred = [&](auto&& __e) { return std::forward<decltype(__e)>(__e) == __value; };
return ranges::__find_last_if_impl(ranges::begin(__r), ranges::end(__r), __pred, __proj);
}
};
} // namespace __find_last

inline namespace __cpo {
inline constexpr auto find_last = __find_last::__fn{};
} // namespace __cpo
} // namespace ranges

_LIBCPP_END_NAMESPACE_STD

#endif // _LIBCPP_STD_VER >= 23

#endif // _LIBCPP___ALGORITHM_RANGES_FIND_LAST_H
104 changes: 104 additions & 0 deletions libcxx/include/__algorithm/ranges_find_last_if.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef _LIBCPP___ALGORITHM_RANGES_FIND_LAST_IF_H
#define _LIBCPP___ALGORITHM_RANGES_FIND_LAST_IF_H

#include <__concepts/assignable.h>
#include <__config>
#include <__functional/identity.h>
#include <__functional/invoke.h>
#include <__iterator/concepts.h>
#include <__iterator/next.h>
#include <__iterator/projected.h>
#include <__ranges/access.h>
#include <__ranges/concepts.h>
#include <__ranges/dangling.h>
#include <__ranges/range_adaptor.h>
#include <__ranges/subrange.h>
#include <__utility/move.h>

#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
#endif

#if _LIBCPP_STD_VER >= 23

_LIBCPP_BEGIN_NAMESPACE_STD

namespace ranges {

template <class _Ip, class _Sp, class _Pred, class _Proj>
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI static constexpr subrange<_Ip>
__find_last_if_impl(_Ip __first, _Sp __last, _Pred&& __pred, _Proj&& __proj) {
if constexpr (common_range<_Ip> && bidirectional_range<_Ip>) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is incorrect. since here you passed in iterator and sentinel, not the range itself, so they will never be common_range or bidirectional_range. I think your second branch almost covers it except that instead of directly copying the last, it copies the first then assigns it to the last. If we want to keep this branch, I think the correct check is to check iterator and sentinel are the same type and the iterator models bidi iterator

// Optimized for common_range and bidirectional_range
_Ip __original_last = __last; // Save the original value of __last
while (__first != __last) {
--__last;
if (std::invoke(__pred, std::invoke(__proj, *__last))) {
return {__last, __original_last}; // Directly return the result
}
}
return {__last, __original_last};
} else if constexpr (bidirectional_iterator<_Ip> && assignable_from<_Ip&, _Sp&>) {
// For non-common but bidirectional ranges
_Ip __result = ranges::next(__first, __last);
Comment on lines +49 to +51
Copy link
Contributor

Choose a reason for hiding this comment

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

To make sure ranges::next is constant time, I think you can do

(random_access_iterator<_Ip> && sized_sentinel_for<_Sp, _Ip>) || (bidirectional_iterator<_Ip> && assignable_from<_Ip&, _Sp&>)

_Ip __original_last = __result;
while (__first != __result) {
--__result;
if (std::invoke(__pred, std::invoke(__proj, *__result))) {
return {__result, __original_last};
}
}
return {__result, __original_last};
} else {
// Code for the non-bidirectional case
_Ip __original_first = __first;
_Ip __result = __first;
while (__first != __last) {
if (std::invoke(__pred, std::invoke(__proj, *__first))) {
__result = __first;
}
++__first;
}
return {__result, __original_first};
Copy link
Contributor

Choose a reason for hiding this comment

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

The return value is not correct. you want to return the incremented iterator which equals to last

Returns: Let i be the last iterator in the range [first, last) for which E is true. Returns {i, last}, or {last, last} if no such iterator is found.

Please add tests after the fix

}
}

namespace __find_last_if {
struct __fn {
template <forward_range _Rp,
class _Proj = identity,
indirect_unary_predicate<projected<iterator_t<_Rp>, _Proj>> _Pred>
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr borrowed_subrange_t<_Rp>
operator()(_Rp&& __r, _Pred __pred, _Proj __proj = {}) const {
return ranges::__find_last_if_impl(ranges::begin(__r), ranges::end(__r), __pred, __proj);
}

template <forward_iterator _Ip,
sentinel_for<_Ip> _Sp,
class _Proj = identity,
indirect_unary_predicate<projected<_Ip, _Proj>> _Pred>
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr subrange<_Ip>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add tests for the [[nodiscard]] extensions.

operator()(_Ip __first, _Sp __last, _Pred __pred, _Proj __proj = {}) const {
return ranges::__find_last_if_impl(std::move(__first), std::move(__last), __pred, __proj);
}
};
} // namespace __find_last_if

inline namespace __cpo {
inline constexpr auto find_last_if = __find_last_if::__fn{};
} // namespace __cpo
} // namespace ranges

_LIBCPP_END_NAMESPACE_STD

#endif // _LIBCPP_STD_VER >= 23

#endif // _LIBCPP___ALGORITHM_RANGES_FIND_LAST_IF_H
69 changes: 69 additions & 0 deletions libcxx/include/__algorithm/ranges_find_last_if_not.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef _LIBCPP___ALGORITHM_RANGES_FIND_LAST_IF_NOT_H
#define _LIBCPP___ALGORITHM_RANGES_FIND_LAST_IF_NOT_H

#include <__algorithm/ranges_find_last_if.h>
#include <__config>
#include <__functional/identity.h>
#include <__functional/invoke.h>
#include <__iterator/concepts.h>
#include <__iterator/projected.h>
#include <__ranges/access.h>
#include <__ranges/concepts.h>
#include <__ranges/dangling.h>
#include <__utility/forward.h>
#include <__utility/move.h>

#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
#endif

#if _LIBCPP_STD_VER >= 23

_LIBCPP_BEGIN_NAMESPACE_STD

namespace ranges {
namespace __find_last_if_not {
struct __fn {
static constexpr auto __make_negate_pred = [](auto&& __pred) {
return [&__pred](auto&& __e) { return !std::invoke(__pred, std::forward<decltype(__e)>(__e)); };
};

template <forward_iterator _Ip,
sentinel_for<_Ip> _Sp,
class _Proj = identity,
indirect_unary_predicate<projected<_Ip, _Proj>> _Pred>
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr subrange<_Ip>
operator()(_Ip __first, _Sp __last, _Pred __pred, _Proj __proj = {}) const {
auto __negate_pred = __make_negate_pred(__pred);
return ranges::__find_last_if_impl(std::move(__first), std::move(__last), __negate_pred, __proj);
}

template <forward_range _Rp,
class _Proj = identity,
indirect_unary_predicate<projected<iterator_t<_Rp>, _Proj>> _Pred>
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr borrowed_subrange_t<_Rp>
operator()(_Rp&& __r, _Pred __pred, _Proj __proj = {}) const {
auto __negate_pred = __make_negate_pred(__pred);
return ranges::__find_last_if_impl(ranges::begin(__r), ranges::end(__r), __negate_pred, __proj);
}
};
} // namespace __find_last_if_not

inline namespace __cpo {
inline constexpr auto find_last_if_not = __find_last_if_not::__fn{};
} // namespace __cpo
} // namespace ranges

_LIBCPP_END_NAMESPACE_STD

#endif // _LIBCPP_STD_VER >= 23

#endif // _LIBCPP___ALGORITHM_RANGES_FIND_LAST_IF_NOT_H
33 changes: 33 additions & 0 deletions libcxx/include/algorithm
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,36 @@ namespace ranges {
constexpr borrowed_iterator_t<R>
find_if_not(R&& r, Pred pred, Proj proj = {}); // since C++20

template<forward_iterator I, sentinel_for<I> S, class T, class Proj = identity>
requires indirect_binary_predicate<ranges::equal_to, projected<I, Proj>, const T*>
constexpr subrange<I>
ranges::find_last(I first, S last, const T& value, Proj proj = {}); // since C++23

template<forward_range R, class T, class Proj = identity>
requires indirect_binary_predicate<ranges::equal_to, projected<iterator_t<R>, Proj>, const T*>
constexpr borrowed_subrange_t<R>
ranges::find_last(R&& r, const T& value, Proj proj = {}); //since C++23

template<forward_iterator I, sentinel_for<I> S, class Proj = identity,
indirect_unary_predicate<projected<I, Proj>> Pred>
constexpr subrange<I>
ranges::find_last_if(I first, S last, Pred pred, Proj proj = {}); // since C++23

template<forward_range R, class Proj = identity,
indirect_unary_predicate<projected<iterator_t<R>, Proj>> Pred>
constexpr borrowed_subrange_t<R>
ranges::find_last_if(R&& r, Pred pred, Proj proj = {}); // since C++23

template<forward_iterator I, sentinel_for<I> S, class Proj = identity,
indirect_unary_predicate<projected<I, Proj>> Pred>
constexpr subrange<I>
ranges::find_last_if_not(I first, S last, Pred pred, Proj proj = {}); // since C++23

template<forward_range R, class Proj = identity,
indirect_unary_predicate<projected<iterator_t<R>, Proj>> Pred>
constexpr borrowed_subrange_t<R>
ranges::find_last_if_not(R&& r, Pred pred, Proj proj = {}); // since C++23

template<class T, class Proj = identity,
indirect_strict_weak_order<projected<const T*, Proj>> Comp = ranges::less>
constexpr const T& min(const T& a, const T& b, Comp comp = {}, Proj proj = {}); // since C++20
Expand Down Expand Up @@ -1859,6 +1889,9 @@ template <class BidirectionalIterator, class Compare>
#include <__algorithm/ranges_find_first_of.h>
#include <__algorithm/ranges_find_if.h>
#include <__algorithm/ranges_find_if_not.h>
#include <__algorithm/ranges_find_last.h>
#include <__algorithm/ranges_find_last_if.h>
#include <__algorithm/ranges_find_last_if_not.h>
#include <__algorithm/ranges_for_each.h>
#include <__algorithm/ranges_for_each_n.h>
#include <__algorithm/ranges_generate.h>
Expand Down
12 changes: 12 additions & 0 deletions libcxx/include/module.modulemap.in
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,18 @@ module std_private_algorithm_ranges_find_end [system
module std_private_algorithm_ranges_find_first_of [system] { header "__algorithm/ranges_find_first_of.h" }
module std_private_algorithm_ranges_find_if [system] { header "__algorithm/ranges_find_if.h" }
module std_private_algorithm_ranges_find_if_not [system] { header "__algorithm/ranges_find_if_not.h" }
module std_private_algorithm_ranges_find_last [system] {
header "__algorithm/ranges_find_last.h"
export std_private_ranges_subrange
}
module std_private_algorithm_ranges_find_last_if [system] {
header "__algorithm/ranges_find_last_if.h"
export std_private_ranges_subrange
}
module std_private_algorithm_ranges_find_last_if_not [system] {
header "__algorithm/ranges_find_last_if_not.h"
export std_private_ranges_subrange
}
module std_private_algorithm_ranges_for_each [system] {
header "__algorithm/ranges_for_each.h"
export std_private_algorithm_in_fun_result
Expand Down
2 changes: 2 additions & 0 deletions libcxx/include/version
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ __cpp_lib_execution 201902L <execution>
201603L // C++17
__cpp_lib_expected 202211L <expected>
__cpp_lib_filesystem 201703L <filesystem>
__cpp_lib_find_last 202207L <algorithm>
__cpp_lib_format 202106L <format>
__cpp_lib_format_ranges 202207L <format>
__cpp_lib_formatters 202302L <stacktrace> <thread>
Expand Down Expand Up @@ -422,6 +423,7 @@ __cpp_lib_within_lifetime 202306L <type_traits>
# define __cpp_lib_constexpr_memory 202202L
# define __cpp_lib_constexpr_typeinfo 202106L
# define __cpp_lib_expected 202211L
# define __cpp_lib_find_last 202207L
# define __cpp_lib_format_ranges 202207L
// # define __cpp_lib_formatters 202302L
# define __cpp_lib_forward_like 202207L
Expand Down
Loading
Loading