[libcxx] patch for implementing ranges::find_last

[phyBrackets]

Summary

patch for implementing https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p1223r5.pdf

[huixie90]

libcxx/include/__algorithm/ranges_find_last.h

2

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

30

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

#if _LIBCPP_STD_VER >= 23

same applies to other files in the patch

40

question. can we use static operator() now?

libcxx/include/__algorithm/ranges_find_last_if.h

37

There are several issues.

1. In general __last is a sentinel (as supposed to be an iterator). The only thing you can do about it is to compare with an iterator. so you cannot use operator-- for a sentinel.
2. The algorithm's requirement is forward_iterator (instead of bidirectional_iterator). operator-- does not exist for forward_iteartor. You will probably need to dispatch to different implementations depending on whether it is common_range, bidirectional_range, sized_sentinel_for/sized_range

[fsb4000]

libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find_last_if_not.pass.cpp

217

Should be static_assert?

[fsb4000]

libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find_last.pass.cpp

246

static_assert

libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find_last_if.pass.cpp

231

static_assert

[fsb4000]

libcxx/docs/Status/Cxx2bPapers.csv

58

instead of

"","|ranges|"

should be

"17.0","|ranges|"

[Mordante]

Thanks for your patch. I only want over it lightly. I will do an full review after the current open issues are addressed.

libcxx/include/__algorithm/ranges_find_last.h

30

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

40

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

56

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

libcxx/include/__algorithm/ranges_find_last_if.h

39

Please don't use auto everywhere, this just makes things harder to read.
Please make sure all names are __uglified, for example result too,

libcxx/include/__algorithm/ranges_find_last_if_not.h

43

naming is hard, but let's try to use something better than just a number.
How about __not_pred since this is basically the inverse of the predicate given.

Another option is just not giving this predicate a name and write it in the return statement on the next line.

libcxx/include/algorithm

102

This indention is off.

104

Please align all these since entries.

libcxx/include/module.modulemap.in

365

Please fix the alignment of the {.

libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find_last.pass.cpp

82

Here you should use std::same_as<return type> auto method. For example have a look at
libcxx/test/std/ranges/range.adaptors/range.common.view/begin.pass.cpp

libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find_last_if.pass.cpp

106

Please use clang-format on these tests, this looks wrong.

libcxx/utils/data/ignore_format.txt

119

The ignore list is there since we don't want to reformat our entire code base and have merge conflicts for existing patches. New files should always be formatted.

llvm/utils/gn/secondary/libcxx/include/BUILD.gn

170 ↗

(On Diff #506629)

Typically we don't update the bazel files, there is a post-commit bot that does that.