[libcxx-commits] [PATCH] D62760: allocate_shared should call allocator_traits::construct

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added a comment.

Friendly ping @ldionne.



================
Comment at: libcxx/include/memory:3611
     __compressed_pair<__compressed_pair<_Tp, _Dp>, _Alloc> __data_;
+    size_t __size;
+
----------------
zoecarver wrote:
> ldionne wrote:
> > Isn't this an ABI break? It's not clear to me why we need to store the size -- we should always be calling `allocator.deallocate` with one instance of `sizeof(__shared_ptr_pointer)`, no?
> I was worried this might be an ABI break. 
> 
> We need size, though. Because we need to know how big `__shared_ptr_aligned_block` is (and later we'll need to know how big `__shared_ptr_aligned_block` + elements is). 
> 
> `sizeof(__shared_ptr_pointer)` would just be the control block. If we knew that `__shared_ptr_pointer` would only be used by `allocate_shared` we could do something like `sizeof(__shared_ptr_aligned_block< __shared_ptr_pointer , remove_pointer_t<_Tp>>)`. To do that we'd probably have to duplicate `__shared_ptr_pointer` which would be an ABI break too, I think.
Is there a way to verify that this is in fact an ABI break? If this is an ABI break, I have an idea about how to implement this by storing the size at the beginning of `__shared_ptr_aligned_block` but, I think that will add a lot of complexity, so this is (IMHO) a much better implementation if we can get away with it. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62760