[libcxx-commits] [PATCH] D109056: [libc++] Disallow volatile types in std::allocator

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

Quuxplusone requested changes to this revision.
Quuxplusone added a comment.
This revision now requires changes to proceed.

libstdc++ actually //broke and then deliberately restored// support for `make_shared<volatile T>` in GCC 8, circa September 2018.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87278
Therefore I think we need very strong justification for libc++ to stop supporting `make_shared<volatile T>`.

If the only problem is that `make_shared<T>` has an unnecessary dependency on `allocator<T>`, maybe we could do something like

  --- a/libcxx/include/__memory/shared_ptr.h
  +++ b/libcxx/include/__memory/shared_ptr.h
  @@ -1109,7 +1109,7 @@ template<class _Tp, class ..._Args, class = __enable_if_t<!is_array<_Tp>::value>
   _LIBCPP_HIDE_FROM_ABI
   shared_ptr<_Tp> make_shared(_Args&& ...__args)
   {
  -    return _VSTD::allocate_shared<_Tp>(allocator<_Tp>(), _VSTD::forward<_Args>(__args)...);
  +    return _VSTD::allocate_shared<_Tp>(allocator<int>(), _VSTD::forward<_Args>(__args)...);
   }

to work around that. The allocator gets rebound anyway, so there's no real reason to bother instantiating `allocator<_Tp>` in particular... and all `allocator<T>`s happen to have the same layout, so it wouldn't even break ABI to do this IMHO.

FWIW I also think the status quo is fine: so `allocator<volatile T>` is IFNDR instead of a static_assert; does anyone really care?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109056