[libcxx-commits] [PATCH] D91004: [libc++] Implements concept destructible

[Michael Schellenberger Costa via Phabricator via libcxx-commits]

miscco accepted this revision.
miscco added a subscriber: cjdb.
miscco added a comment.

LGTM

If there is another earnest push for concepts / ranges we should get together with @cjdb, @ldionne to see whether we find a way to not step on our toes and duplicate work.

Also I note that the bottleneck has always been getting everything reviewed.

NOTE: I am not a maintainer so wait for one before you commit



================
Comment at: libcxx/test/std/concepts/concept.destructible/destructible.pass.cpp:13
+// template<class T>
+// concept destructible;
+
----------------
I believe it would improve readability of the test a lot if you would add ` =  std::is_nothrow_constructible_v<T>`


================
Comment at: libcxx/test/std/concepts/concept.destructible/destructible.pass.cpp:41
+constexpr bool test() {
+    assert( std::destructible<Empty>);
+
----------------
I believe these should all be static_asserts and we should not need to have a "runtime" check of the trait


================
Comment at: libcxx/test/std/concepts/concept.destructible/destructible.pass.cpp:48
+    assert(!std::destructible<NoexceptFalse>);
+    assert( std::destructible<Noexcept>);
+
----------------
Nitpick: Could you order this in front similar to the structs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91004