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

[Michael Schellenberger Costa via Phabricator via libcxx-commits]

miscco added inline comments.


================
Comment at: libcxx/test/std/concepts/concept.destructible/destructible.pass.cpp:56
+    static_assert( std::destructible<NoexceptDependant<int>>);
+    static_assert(!std::destructible<NoexceptDependant<double>>);
+}
----------------
Mordante wrote:
> cjdb wrote:
> > @CaseyCarter had this to say back in D74291.
> > 
> > > There's tons of coverage here for class types and reference types. What about `void`, function types, pointers, arrays, pointers-to-members? (I don't know about clang/libc++, but the MSVC STL tests for type traits tend to _be_ the tests for compiler intrinsics used to implement those traits, so we consequently cover everything under the sun.)
> > 
> > I'd suggest we apply this here as well.
> Normally I would agree with this comment. However the concept is just a small wrapper for `is_nothrow_destructible_v<T>` so I thought some sanity checks would suffice. The tests of `is_nothrow_destructible` already tests the type trait.
> 
> I've also created a not-yet-posted patch for `constructible_from` which takes the same approach.
> 
> For my WIP patch for `default_initializable` I'm busy adding a lot more tests since this concept is not a simple wrapper for a type trait. For that patch I feel the additional tests are required.
I believe we should be as comprehensive as possible. There is no guarantee that in the future we would change the implementation or whatever. So each feature should get all the love it needs


================
Comment at: libcxx/test/std/concepts/concept.destructible/destructible.pass.cpp:41
+constexpr bool test() {
+    assert( std::destructible<Empty>);
+
----------------
Mordante wrote:
> miscco wrote:
> > I believe these should all be static_asserts and we should not need to have a "runtime" check of the trait
> I just verified with the type_traits and they only use `static_asserts`, so I agree we should do the same with concepts.
Sorry that I forgot this the first time. We now have compile only tests that do not require any linking at all. This (and all other concepts tests) are prime examples of compile only tests and we should convert it to one. As far as I know it is only necessary to change the file ending to .compile.pass @ldionne?


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

https://reviews.llvm.org/D91004