Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

Differential D136522 Diff 516117 libcxx/include/__algorithm/ranges_find_last.h

Changeset View

Standalone View

View Options

libcxx/include/__algorithm/ranges_find_last.h

  • This file was added.
//===----------------------------------------------------------------------===//
//
huixie90Unsubmitted
Done
ReplyInline Actions

Please add tests for all the algorithms' behaviour and constraints.
You can take a look at this to get an idea about how we test algorithms
https://reviews.llvm.org/D130404

huixie90: Please add tests for all the algorithms' behaviour and constraints. You can take a look at this…
// 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/invoke.h>
var-constUnsubmitted
Not Done
ReplyInline Actions

Is this used?

var-const: Is this used?
#include <__functional/ranges_operations.h>
var-constUnsubmitted
Not Done
ReplyInline Actions

Is this used?

var-const: Is this used?
#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
huixie90Unsubmitted
Done
ReplyInline Actions

is the paper C++23? If so, should it be

#if _LIBCPP_STD_VER >= 23

same applies to other files in the patch

huixie90: is the paper C++23? If so, should it be ``` #if _LIBCPP_STD_VER >= 23 ``` same applies to other…
MordanteUnsubmitted
Done
ReplyInline Actions

@phyBrackets can you mark items as done when they are done? That makes reviewing these changes a lot easier.

Mordante: @phyBrackets can you mark items as done when they are done? That makes reviewing these changes…
_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_HIDE_FROM_ABI constexpr subrange<_Ip>
var-constUnsubmitted
Not Done
ReplyInline Actions

This formatting looks really weird, is it produced by clang-format? (If so, feel free to disable it for these lines by adding a // clang-format off comment, then reenable with // clang-format on)

var-const: This formatting looks really weird, is it produced by `clang-format`? (If so, feel free to…
var-constUnsubmitted
Not Done
ReplyInline Actions

Can you also add _LIBCPP_NODISCARD_EXT to all the new algorithms? We want to mark these as [[nodiscard]] since they have no side effects and are only used to produce a value, but since they aren't marked as such in the Standard, we have to do it as an extension. Please also update the test/libcxx/diagnostics/ranges.nodiscard_extensions.verify.cpp test file.

var-const: Can you also add `_LIBCPP_NODISCARD_EXT` to all the new algorithms? We want to mark these as `…
operator()(_Ip __first, _Sp __last, const _Tp& __value, _Proj __proj = {}) const {
auto __pred = [&](auto&& __e) { return std::forward<decltype(__e)>(__e) == __value; };
huixie90Unsubmitted
Not Done
ReplyInline Actions

question. can we use static operator() now?

huixie90: question. can we use `static operator()` now?
MordanteUnsubmitted
Not Done
ReplyInline Actions

Why would that not be possible, I've seen this used in other ranges patches too.

Mordante: Why would that not be possible, I've seen this used in other ranges patches too.
var-constUnsubmitted
Not Done
ReplyInline Actions

Question: what's the case where forward would make a difference? Do we have a test for that?

var-const: Question: what's the case where `forward` would make a difference? Do we have a test for that?
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_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
MordanteUnsubmitted
Done
ReplyInline Actions

Please run clang-format on new files, I'm quite sure this indention is off.

Mordante: Please run clang-format on new files, I'm quite sure this indention is off.
} // namespace ranges
_LIBCPP_END_NAMESPACE_STD
#endif // _LIBCPP_STD_VER > 23
var-constUnsubmitted
Not Done
ReplyInline Actions

>= 23.

var-const: `>= 23`.
#endif // _LIBCPP___ALGORITHM_RANGES_FIND_LAST_H
No newline at end of file