This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Add range_size_t
ClosedPublic

Authored by ldionne on Jul 23 2021, 1:46 PM.

Details

Reviewers
Quuxplusone
Mordante
Group Reviewers
Restricted Project
Commits
rGfbaf7f0bc768: [libc++] Add range_size_t

Diff Detail

Event Timeline

ldionne requested review of this revision.Jul 23 2021, 1:46 PM
ldionne created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJul 23 2021, 1:46 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne updated this revision to Diff 361336.Jul 23 2021, 1:47 PM

Update with review link.

Quuxplusone added a subscriber: Quuxplusone.

LGTM mod comments.

libcxx/include/__ranges/concepts.h
73

This is actually from section [range.range], not [range.sized]. I doubt we care.

libcxx/include/ranges
38–40

No, T was correct. https://eel.is/c++draft/ranges#syn
It's noteworthy that iterator_t is not constrained — class T is correct, not range T. I don't know why.

libcxx/test/std/ranges/range.req/range.range/range_size_t.compile.pass.cpp
29

Please add SFINAE-friendliness tests. I recommend replacing this whole file with:

template<class T>
concept has_size_t = requires { typename std::ranges::range_size_t<T>; };

struct A { int *begin(); int *end(); short size(); };
static_assert(std::same_as<std::ranges::range_size_t<A>, short>);
static_assert(std::same_as<std::ranges::range_size_t<A&>, short>);
static_assert(std::same_as<std::ranges::range_size_t<A&&>, short>);
static_assert(!has_size_t<const A>);
static_assert(!has_size_t<const A&>);
static_assert(!has_size_t<const A&&>);

struct B { int *begin(); int *end(); };
static_assert(std::same_as<std::ranges::range_size_t<B>, std::size_t>);
static_assert(std::same_as<std::ranges::range_size_t<B&>, std::size_t>);
static_assert(std::same_as<std::ranges::range_size_t<B&&>, std::size_t>);
static_assert(!has_size_t<const B>);
static_assert(!has_size_t<const B&>);
static_assert(!has_size_t<const B&&>);

struct C { bidirectional_iterator<int*> begin(); bidirectional_iterator<int*> end(); };
static_assert(!has_size_t<C>);
Mordante accepted this revision.Jul 24 2021, 5:15 AM
Mordante added a subscriber: Mordante.

LGTM, modulo comment.

libcxx/test/std/ranges/range.req/range.range/range_size_t.compile.pass.cpp
29

+1

This revision is now accepted and ready to land.Jul 24 2021, 5:15 AM
ldionne marked 4 inline comments as done.Jul 26 2021, 7:54 AM
ldionne added inline comments.
libcxx/include/__ranges/concepts.h
73

Yeah, I couldn't put it in the section above cause it needs sized_range to be defined. I won't add another comment, that would just be confusing IMO.

ldionne updated this revision to Diff 361655.Jul 26 2021, 7:55 AM
ldionne marked an inline comment as done.

Address review comments (thanks!)

This revision was automatically updated to reflect the committed changes.