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

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

Quuxplusone accepted this revision as: Quuxplusone.
Quuxplusone added a comment.

This is the sort of thing to alert libc++-vendors about, right?



================
Comment at: libcxx/include/__memory/allocator.h:72-73
 {
     static_assert(!is_volatile<_Tp>::value, "std::allocator does not support volatile types");
+    static_assert(!is_const<_Tp>::value, "std::allocator does not support const types");
 public:
----------------
Nit: Maybe swap these lines to alphabetize them? (also to put the shorter and more common cv-qualifier first)


================
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)...);
----------------
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"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120996