This is an archive of the discontinued LLVM Phabricator instance.

[libcxx][ranges] Add `default_sentinel` and `default_sentinel_t`.

Authored by zoecarver on Jun 1 2021, 12:56 PM.

Diff Detail

Event Timeline

zoecarver created this revision.Jun 1 2021, 12:56 PM
zoecarver requested review of this revision.Jun 1 2021, 12:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 1 2021, 12:56 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

LGTM % that one additional test.

#include <concepts>
#include <type_traits>

Actually, if you replaced same_as with is_same_v and semiregular with is_copy_constructible_v, could you remove the UNSUPPORTED: libcpp-no-concepts line so that this would be testable in more situations?
If de-concept-ifying this test turns into a rabbit hole, then never mind; but if it's trivial to simplify, then it'd be nice.


I'd also like to see here that

std::default_sentinel_t s1;
auto s2 = std::default_sentinel_t{};

both compile. (C++20 seems to require that these compile. Unlike some types, e.g. std::nullopt_t, which are not supposed to default-construct.)

zoecarver added inline comments.Jun 1 2021, 1:40 PM

Doesn't semiregular check that? Or is this a default constructible vs default initializable thing?

Quuxplusone added inline comments.Jun 1 2021, 1:58 PM

If we don't know, that's a great reason to add the test. :)
The snippet I suggested will test both "default initialization" (s1) and "construction from nothing" (s2).

This revision was not accepted when it landed; it landed in state Needs Review.Jun 1 2021, 2:07 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.

LGTM post commit.


FWIW, libcpp-no-concepts is something that will die eventually as we move to newer compilers. IMO there's little benefit in doing any work to improve the situation on compilers that do not support concepts.