Page MenuHomePhabricator

[libc++] Implement ranges::adjacent_find
ClosedPublic

Authored by philnik on May 29 2022, 2:59 AM.

Details

Reviewers
ldionne
Mordante
var-const
Group Reviewers
Restricted Project
Commits
rG916e9052ba95: [libc++] Implement ranges::adjacent_find

Diff Detail

Event Timeline

philnik created this revision.May 29 2022, 2:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 29 2022, 2:59 AM
Herald added a subscriber: mgorny. · View Herald Transcript
philnik requested review of this revision.May 29 2022, 2:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 29 2022, 2:59 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
var-const requested changes to this revision.Jun 3 2022, 9:59 PM

Almost LGTM, just a couple of test cases that can be added.

libcxx/test/std/algorithms/alg.nonmodifying/alg.adjacent.find/ranges.adjacent_find.pass.cpp
34

Nit: s/ ,/, /.

101

Can you also add two test cases to check that a non-default projection and a non-default predicate work?

102

Nit: s/checkt/check/.

103

Nit: decltype(auto).

This revision now requires changes to proceed.Jun 3 2022, 9:59 PM

Please update the ranges status page.

For the most part LGTM except for some minor issues.

libcxx/include/__type_traits/enable_if.h
13 ↗(On Diff #432775)

This seems unrelated, please commit it separately.

libcxx/test/std/algorithms/alg.nonmodifying/alg.adjacent.find/ranges.adjacent_find.pass.cpp
78

Can't N be deduced?

106

Please also test the complexity when a match is found.

philnik updated this revision to Diff 434439.Jun 6 2022, 5:15 AM
philnik marked 7 inline comments as done.
  • Address comments
libcxx/include/__type_traits/enable_if.h
13 ↗(On Diff #432775)

This was part of D126469. I don't know why it was part of this diff.

libcxx/test/std/algorithms/alg.nonmodifying/alg.adjacent.find/ranges.adjacent_find.pass.cpp
78

I don't think so. @var-const and I also thought it could be deduced, but I couldn't get it to work.

var-const accepted this revision as: var-const.Jun 6 2022, 1:27 PM

LGTM, but please wait for @Mordante to approve as well before merging.

Mordante accepted this revision.Tue, Jun 7, 11:58 AM

LGTM, but one suggested change.

libcxx/test/std/algorithms/alg.nonmodifying/alg.adjacent.find/ranges.adjacent_find.pass.cpp
55
61
78

I gave it a shot, but I too failed to get it to work. I would suggest then to change the type to size_t to match the type used in std::array.

This revision is now accepted and ready to land.Tue, Jun 7, 11:58 AM
philnik marked 3 inline comments as done.Wed, Jun 8, 3:15 AM
This revision was automatically updated to reflect the committed changes.