This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Implement ranges::lexicographical_compare
ClosedPublic

Authored by philnik on Jun 6 2022, 10:37 AM.

Diff Detail

Event Timeline

philnik created this revision.Jun 6 2022, 10:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2022, 10:37 AM
Herald added a subscriber: mgorny. · View Herald Transcript
philnik requested review of this revision.Jun 6 2022, 10:37 AM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
var-const requested changes to this revision.Jun 9 2022, 7:19 PM
var-const added inline comments.
libcxx/include/__algorithm/ranges_lexicographical_compare.h
64

Nit: add the include.

libcxx/test/std/algorithms/alg.sorting/alg.lex.comparison/ranges.lexicographical_compare.pass.cpp
133

Please also add tests with a non-default projection or a non-default comparator.

190

Question: why is std::begin necessary here?

This revision now requires changes to proceed.Jun 9 2022, 7:19 PM
philnik updated this revision to Diff 436372.Jun 13 2022, 6:38 AM
philnik marked 3 inline comments as done.
  • Address comments
philnik updated this revision to Diff 436373.Jun 13 2022, 6:41 AM
  • Fix modulemap
philnik added inline comments.Jun 13 2022, 10:34 AM
libcxx/test/std/algorithms/alg.sorting/alg.lex.comparison/ranges.lexicographical_compare.pass.cpp
190

It's not necessary, but I think std::begin and std::end should always go together. It is just another thing you have to think about otherwise.

var-const accepted this revision.Jun 13 2022, 2:42 PM
var-const added inline comments.
libcxx/test/std/algorithms/alg.sorting/alg.lex.comparison/ranges.lexicographical_compare.pass.cpp
190

Sorry, what I meant was, why not just use (a, a + 5, a, a + 5, red, proj1, proj2)?

This revision is now accepted and ready to land.Jun 13 2022, 2:42 PM
philnik updated this revision to Diff 436644.Jun 13 2022, 9:54 PM
  • Fix Modules build
philnik marked an inline comment as done.Jun 13 2022, 9:54 PM
philnik added inline comments.
libcxx/test/std/algorithms/alg.sorting/alg.lex.comparison/ranges.lexicographical_compare.pass.cpp
190

@Mordante requested this change on another review and I agree with him that std::begin(a), std::end(a) is much easier to digest than a, a + 5. It's just correct on sight.

Mordante accepted this revision.Jun 14 2022, 12:35 PM

LGTM when the CI is happy.

Should this be marked as completed?

philnik updated this revision to Diff 437156.Jun 15 2022, 7:25 AM
philnik marked an inline comment as done.
  • Mark algorihtm as done; fix CI
This revision was automatically updated to reflect the committed changes.