[libcxx-commits] [PATCH] D57403: Extending make_shared to Support Arrays (Partially) - P0674R1

[Marshall Clow via Phabricator via libcxx-commits]

mclow.lists added inline comments.


================
Comment at: include/memory:3705
 
+template <typename _Tp>
+void
----------------
Is there a reason you can't use `_VSTD::destroy_at`?



================
Comment at: include/memory:3712
+
+template <typename _Tp, std::size_t __n>
+void
----------------
Is there a reason you can't use `_VSTD::destroy_n`?


================
Comment at: include/memory:4358
+    // XXX is it okay to delete the line below? It seems like it is only checking if _Tp is an array.
+    // static_assert( is_constructible<_Tp, _Args...>::value, "Can't construct object in allocate_shared" );
     typedef __shared_ptr_emplace<_Tp, _Alloc> _CntrlBlk;
----------------
It's checking to see if you can construct a `_Tp` from that list of arguments.


================
Comment at: test/libcxx/memory/shared_ptr_array.pass.cpp:26
+
+    assert(typeid(**foo) == typeid(T) && "Type is not the same");
+    assert(typeid(**bar) == typeid(T) && "Type is not the same");
----------------
We actually have an `ASSERT_SAME_TYPE` macro in "test_macros.h"


================
Comment at: test/libcxx/memory/shared_ptr_array.pass.cpp:28
+    assert(typeid(**bar) == typeid(T) && "Type is not the same");
+}
+
----------------
I think you want to test that 64 constructors got called, and (afterwards) that 64 destructors got called.



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

https://reviews.llvm.org/D57403