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

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver marked an inline comment as done.
zoecarver added inline comments.


================
Comment at: libcxx/include/memory:3658
     __data_.second().~_Alloc();
-    __a.deallocate(_PTraits::pointer_to(*this), 1);
+    __a.deallocate(_PTraits::pointer_to(*reinterpret_cast<char*>(this)), __size);
 }
----------------
ldionne wrote:
> zoecarver wrote:
> > ldionne wrote:
> > > Can you explain why, in this patch, using `__a.deallocate(_PTraits::pointer_to(*this), 1);` is not sufficient?
> > Because `_Al` is rebound to `char` type and used in `_ATraits` which is used to get the pointer type for `_PTraits` so, `_PTraits:: pointer_to ` will be expecting a `char*`.
> But why do you need to rebind it to `char`?
What other type would you bind it to? //Sometimes// we're going to need to deallocate `sizeof(__shared_ptr_pointer)` but sometimes it will need to be `sizeof(__shared_ptr_aligned_block< __shared_ptr_pointer , remove_pointer_t<_Tp>>)` and later it will need to be that plus the size and number of elements we allocated. We don't really know. 

A temporary alternative could be adding another template argument which the allocator could rebind to but, we'd need to add back `size` in D62641 (to support unbounded array types). 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62760