[libcxx-commits] [PATCH] D57403: Extending make_shared to Support Arrays (Partially) - P0674R1
Zoe Carver via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Feb 16 11:26:00 PST 2019
zoecarver marked 7 inline comments as done.
zoecarver added a comment.
Sorry about the delay updating this patch. I think I got everything except the additional overloads.
I spent some time trying to add the other overloads. Unless I am not seeing something (which is very possible) I think the `shared_ptr` class might have to have some serious work in order to support array types with undefined length.
================
Comment at: include/memory:3709
+{
+ t.~_Tp();
+}
----------------
ldionne wrote:
> This here should be `_VSTD::destroy_at(&t)`, and `t` needs to be mangled.
Could be wrong about this but I think that this needs to work in C++11 but `destroy` wasn't introduced until C++17.
================
Comment at: include/memory:3716
+{
+ for (auto i = __n; i > 0; i--) __shared_ptr_emplace__destroy(t[i]);
+}
----------------
ldionne wrote:
> I would use this instead:
>
> ```
> _VSTD::destroy(_VSTD::begin(t), _VSTD::end(t))
> ```
>
> Also, `t` needs to be mangled to `__t`.
>
Same as above comment.
================
Comment at: test/libcxx/memory/shared_ptr_array.pass.cpp:23
+
+struct test_onstructors
+{
----------------
ldionne wrote:
> `test_constructors` (typo)
How did I not see this 🤦♂️
================
Comment at: test/libcxx/memory/shared_ptr_array.pass.cpp:52
+ auto pi = &i;
+ ASSERT_SAME_TYPE(decltype(**pi), int);
+ */
----------------
ldionne wrote:
> https://wandbox.org/permlink/YuADEmFtbD8ZlNS3
>
> The result of dereferencing a pointer like `**pi` is a lvalue (`int&` in this case).
Very helpful thank you.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57403/new/
https://reviews.llvm.org/D57403
More information about the libcxx-commits
mailing list