[libcxx-commits] [PATCH] D106916: [libc++] Handle arrays in std::destroy_at

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

Quuxplusone accepted this revision.
Quuxplusone added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/test/std/utilities/memory/specialized.algorithms/specialized.destroy/destroy.pass.cpp:36
+#if TEST_STD_VER > 17
+TEST_CONSTEXPR_CXX20 bool test_arrays() {
+    {
----------------
Could use just `constexpr` here if you want.


================
Comment at: libcxx/test/std/utilities/memory/specialized.algorithms/specialized.destroy/destroy.pass.cpp:53
+        std::destroy(pool, pool + 5);
+        static_assert(std::is_same_v<decltype(std::destroy(pool, pool + 5)), void>);
+        assert(counter == 0);
----------------
FWIW, in my most recent patch I've rekindled my love affair with
```
ASSERT_SAME_TYPE(decltype(std::destroy(pool, pool + 5)), void);
```


================
Comment at: libcxx/test/std/utilities/memory/specialized.algorithms/specialized.destroy/destroy_n.pass.cpp:110-128
+    // Make sure it works with a bare forward_iterator
+    {
+        using Alloc = std::allocator<Counted>;
+        int counter = 0;
+        Alloc alloc;
+        Counted* pool = std::allocator_traits<Alloc>::allocate(alloc, 5);
 
----------------
Not blocking, but it would be //cleaner// to factor these repeated blocks into `template<class It> void test1()` and then have the body of the top-level `test()` be just `test1<Counted*>(); test1<forward_iterator<Counted*>>();`
(Applies equally to all the tests touched in this PR, if you choose to do it at all.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106916