This is an archive of the discontinued LLVM Phabricator instance.

[libc++] [ranges] SFINAE-friendly "write it three times" in views::counted.
ClosedPublic

Authored by Quuxplusone on Dec 6 2021, 12:47 PM.

Details

Summary

Before this patch, the new test's CountedInvocable<int*, int*> would hard-error instead of SFINAEing and cleanly returning false.

Notice that views::counted specifically does NOT work with pipes; counted(42) is ill-formed. This is because counted's first argument is supposed to be an iterator, not a range.

Also in this PR, drive-by replace some stray std:: with _VSTD::-or-nothing, as appropriate. These will be committed in a separate commit, but I want to poke buildkite about them just to be on the safe side.

The new free function base(cpp20_input_iterator) is needed for the new counted test; it's just filling out a bit of the test_iterators.h API that was missing originally. It's on my TODO list to move the other test iterators' base(i) functions into hidden friends too; I've just been procrastinating and/or rabbit-holing. (See D111372, which got most of the way down the rabbit hole and then I went off to do other things and haven't come back yet.)

Diff Detail

Event Timeline

Quuxplusone requested review of this revision.Dec 6 2021, 12:47 PM
Quuxplusone created this revision.
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald TranscriptDec 6 2021, 12:47 PM
jloser added a subscriber: jloser.Dec 6 2021, 12:52 PM
jloser added inline comments.
libcxx/include/__ranges/counted.h
43

Should to_address be SFINAE friendly instead? I vaguely remember running into this in a PR a few months ago (https://reviews.llvm.org/D108855).

ldionne requested changes to this revision.Dec 6 2021, 1:19 PM
ldionne added inline comments.
libcxx/include/__compare/weak_order.h
45–51 ↗(On Diff #392152)

Please make those changes in a separate NFC patch. They really don't belong here.

libcxx/include/__ranges/counted.h
36–37

I think this can be removed now.

62

You're adding [[nodiscard]]. Please call it out in the commit message and add a test for it.

libcxx/test/std/ranges/range.adaptors/range.counted/counted.pass.cpp
20–21

#include <memory> for addressof, <cstddef> to size_t.

This revision now requires changes to proceed.Dec 6 2021, 1:19 PM
Quuxplusone marked 4 inline comments as done.

Rationalize the nodiscard tests.
Fix the GCC failure, and add another test inspired by it.
Remove the dead concept (thanks!).

ldionne accepted this revision.Dec 7 2021, 9:08 AM

LGTM. You can ignore the failing CI, it should have been fixed on main.

This revision is now accepted and ready to land.Dec 7 2021, 9:08 AM