This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Implement ranges::starts_with
ClosedPublic

Authored by ZijunZhao on May 8 2023, 3:22 PM.

Details

Reviewers
ldionne
var-const
philnik
Group Reviewers
Restricted Project
Summary

[libc++] Implement ranges::starts_with

Diff Detail

Event Timeline

ZijunZhao created this revision.May 8 2023, 3:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 8 2023, 3:22 PM
ZijunZhao requested review of this revision.May 8 2023, 3:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 8 2023, 3:22 PM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald Transcript
ZijunZhao updated this revision to Diff 520831.May 9 2023, 3:05 PM
philnik requested changes to this revision.May 9 2023, 6:12 PM
philnik added a subscriber: philnik.

Looks pretty good already. Please make sure the CI is green.

libcxx/include/__algorithm/ranges_starts_with.h
12

You have to #include <__config>.

50

Please update the nodiscard_extension tests!

58

I think we should just forward this for ranges::equal (for random access ranges) or ranges::mismatch (otherwise) to grab any optimizations that are done there.

libcxx/test/std/algorithms/alg.nonmodifying/alg.starts_with/ranges.starts_with.pass.cpp
13

This isn't a relevant flag anymore.

33

Looks like clang-format does weird things again. Please update the formatting.

194–202

This can be simplified now. See ranges.copy.pass.cpp for an example.

204–224

We have a few robust_against_footests for this and other things. Please make sure you update them.

256

Newline!

This revision now requires changes to proceed.May 9 2023, 6:12 PM
ZijunZhao marked 3 inline comments as done.May 10 2023, 4:43 PM
ZijunZhao added inline comments.
libcxx/include/__algorithm/ranges_starts_with.h
58

yes, I used ranges::mismatch directly before but the comparators and projections will be copied, which fail ranges_robust_against_copying_comparators.pass.cpp and ranges_robust_against_copying_projections.pass.cpp

philnik added inline comments.May 10 2023, 4:46 PM
libcxx/include/__algorithm/ranges_starts_with.h
58

You can call the actual implementation instead of the public interface to avoid that.

ZijunZhao updated this revision to Diff 521195.May 10 2023, 7:11 PM
ZijunZhao marked 5 inline comments as done.
ZijunZhao added inline comments.May 10 2023, 7:14 PM
libcxx/include/__algorithm/ranges_starts_with.h
58

I see! Thank you!

libcxx/test/std/algorithms/alg.nonmodifying/alg.starts_with/ranges.starts_with.pass.cpp
33

Are there any command instructions to disable clang-format? I fixed part of the code manually 😂

ZijunZhao added inline comments.May 11 2023, 11:17 AM
libcxx/include/__algorithm/ranges_starts_with.h
12

I see. Yes, according to the build error log, I need to include more header files. WIP

ZijunZhao marked an inline comment as done.
ZijunZhao updated this revision to Diff 521448.May 11 2023, 2:24 PM
philnik accepted this revision.May 11 2023, 3:44 PM

LGTM % comments with green CI.

libcxx/include/__algorithm/ranges_starts_with.h
12
52

Would you be interested in optimizing random access iterators? If not, I can also do that in a follow-up patch.

libcxx/test/std/algorithms/alg.nonmodifying/alg.starts_with/ranges.starts_with.pass.cpp
33

You can enable and disable clang-format with // clang-format off/on.

65

Please use two spaces per indentation level.

217–224
253–254
This revision is now accepted and ready to land.May 11 2023, 3:44 PM
ldionne accepted this revision.May 15 2023, 10:36 AM

LGTM, thanks!

ZijunZhao marked 5 inline comments as done.May 16 2023, 3:47 PM