[libcxx-commits] [PATCH] D120996: [libc++] Remove extension to support allocator<const T>

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__memory/shared_ptr.h:290-292
+        using _TpAlloc = typename __allocator_traits_rebind<_Alloc, remove_cv_t<_Tp>>::type;
         _TpAlloc __tmp(*__get_alloc());
         allocator_traits<_TpAlloc>::construct(__tmp, __get_elem(), _VSTD::forward<_Args>(__args)...);
----------------
ldionne wrote:
> Quuxplusone wrote:
> > Pre-existing: Why do we rebind the allocator at all? `construct` is supposed to work on any allocator in the same family, isn't it? (IIRC, it deduces the type-to-construct from the pointer type of `__get_elem()`, not from the type of `__tmp`.) Is this a QoI thing, or just "historical reasons"?
> I'm not sure I understand. `allocator_traits<>::construct(alloc, ptr, args...)` will call `alloc.construct(ptr, args...)`, and that requires `ptr` to be of type `alloc.value_type` [sic], doesn't it?
No, `ptr` is of type `T*`, where `T` is deduced independently:
https://en.cppreference.com/w/cpp/memory/allocator_traits/construct
`allocate`/`deallocate` are `value_type`-dependent, but `construct`/`destroy` are heterogeneous. Yes this makes no sense, but it's how it has always been. (I assume there was an original bad rationale related to node-based containers.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120996