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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D109056#2995692 <https://reviews.llvm.org/D109056#2995692>, @Quuxplusone wrote:

> 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?

Simply put, I disagree with Jonathan's call on https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87278. As he said there, the Committee is moving away from supporting `volatile` in the standard library, and I think that sort of benign breakage is acceptable. After all, the Standard says that `allocator<volatile T>` is IFNDR, so we shouldn't encourage people to use that type.



================
Comment at: libcxx/include/__memory/allocator.h:83
 {
+    static_assert(!__is_volatile(_Tp), "std::allocator does not support volatile types");
 public:
----------------
You can use the `is_volatile` type trait, since we support that trait even in C++03 mode. Generally speaking, libc++ supports all C++11 type traits in C++03 language mode as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109056