[libcxx-commits] [PATCH] D98154: [libcxx] adds concept std::regular

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/concepts/object/regular.compile.pass.cpp:12
+
+// template<class From>
+// concept regular = see below;
----------------
s/From/T/


================
Comment at: libcxx/test/std/concepts/object/regular.compile.pass.cpp:37
+static_assert(std::regular<int volatile const*>);
+static_assert(std::regular<int (*)()>);
+
----------------
Here I think it would be more interesting to check some other built-in types — notably `float`/`double`, `void*`, and maybe even member-pointer-into-a-union-type (which supports `operator==` but with underspecified semantics).

You could pay for these new test cases by collapsing the 4 `int*`/`int const*`/`int volatile*`/`int volatile const*` lines into 1 line, and the following 25 lines into 1 line.



================
Comment at: libcxx/test/std/concepts/object/regular.compile.pass.cpp:75
+static_assert(std::regular<std::vector<std::unique_ptr<int> > >);
+static_assert(std::semiregular<std::in_place_t>);
+static_assert(!std::regular<std::in_place_t>);
----------------
This line is out of place.
It did strike me tonight that we could have organized these tests completely differently: we could have presented one //type// at a time, and static-asserted exactly where it falls on the concept hierarchy. For example:

```
struct Simple {};
static_assert(std::semiregular<Simple> && !std::regular<Simple>);

struct MoveCtor { MoveCtor(MoveCtor&&); };
static_assert(std::move_constructible<MoveCtor> && !std::movable<MoveCtor>);
```
and so on.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98154/new/

https://reviews.llvm.org/D98154