[libcxx-commits] [PATCH] D62641: Support arrays in make_shared and allocate_shared (P0674R1)

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/__memory/shared_ptr.h:1007-1008
+        // [1]: https://en.wikipedia.org/wiki/Data_structure_alignment#Computing_padding
+        size_t __bytes = __elements <= 1 ? sizeof(__unbounded_array_control_block)
+                                         : (__elements - 1) * sizeof(_Tp) + sizeof(__unbounded_array_control_block);
+        constexpr size_t __align = alignof(_Tp);
----------------
ldionne wrote:
> Quuxplusone wrote:
> > The condition could be `__elements == 0 ? ~~~`; the only singularity requiring a special case is at zero. It looks to me as if `make_shared<int[]>(0)` is intended to be UB: https://eel.is/c++draft/util.smartptr.shared#create-13 — but maybe we want to support it anyway for QoI, or maybe I'm misunderstanding the possibility of returning a pointer to an object of type `U[0]`.
> I actually didn't find anything explicitly prohibiting `make_shared<T[]>(0)`. Furthermore, I had never really thought about it, but based on https://stackoverflow.com/a/1087066/627587 it seems like dynamically-sized arrays of size 0 are definitely a thing and they are supported. Arrays with a static size 0 are not, though.
> 
> So yeah, I think `make_shared<int[]>(0)` & friends definitely need to work, and it must allocate an array with no members. I realized after reading this that I did have tests for this case already. I will add tests to ensure that we reject arrays with a static size of 0.
> 
> I reworded the comment and changed the condition to `__elements == 0`.
Oh yeah, also I did check with other implementations: https://godbolt.org/z/z8nacGhx1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62641