Page MenuHomePhabricator

[libc++] Implement ranges::reverse
ClosedPublic

Authored by philnik on May 17 2022, 1:52 AM.

Details

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

Diff Detail

Event Timeline

philnik created this revision.May 17 2022, 1:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2022, 1:52 AM
Herald added a subscriber: mgorny. · View Herald Transcript
philnik requested review of this revision.May 17 2022, 1:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2022, 1:52 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
var-const requested changes to this revision.May 17 2022, 10:20 AM

In general looks really good, thanks! (just a few comments)

libcxx/include/__algorithm/ranges_reverse.h
41

Question: how is this loop more efficient than the non-random access iterator version?

libcxx/test/std/algorithms/alg.modifying.operations/alg.reverse/ranges.reverse.pass.cpp
13

Can you also check the complexity requirement?

35

Can you also check the permutable constraint?

48

Nit: decltype(auto).

63

Please also check an odd number of elements.

libcxx/test/support/almost_satisfies_types.h
142

Nit: make it a struct, so that it won't be necessary to specify public?

This revision now requires changes to proceed.May 17 2022, 10:20 AM
philnik updated this revision to Diff 430321.May 18 2022, 4:20 AM
philnik marked 6 inline comments as done.
  • Address comments
libcxx/include/__algorithm/ranges_reverse.h
41

In the forward variant we have to check that __first != __end and __first != --__end. In the random-access version we can just check that __first < --__end, so we do half the comparisons.

libcxx/test/support/almost_satisfies_types.h
142

I made everything else in this file a class and then made it public. I'd rather keep it that way for consistency.

var-const accepted this revision.May 23 2022, 8:30 PM

Thanks! Sorry about slow reviews.

libcxx/test/support/almost_satisfies_types.h
199

Nit: there's an extra blank line here.

This revision is now accepted and ready to land.May 23 2022, 8:30 PM
This revision was automatically updated to reflect the committed changes.
philnik marked an inline comment as done.