[libcxx-commits] [PATCH] D126210: [libcxx] Fix allocator<void>::pointer in C++20 with removed members

Wed May 25 06:50:53 PDT 2022

ldionne added a comment.

In D126210#3537030 <https://reviews.llvm.org/D126210#3537030>, @ilya-biryukov wrote:

> @ldionne friendly ping. Just want to make sure this does not fall off your radar.
> Let me know if you need more time, no rush here from our side.

Thanks for the ping.

Let's also add a release note and document the option in `libcxx/docs/UsingLibcxx.rst`.

================
Comment at: libcxx/include/__memory/allocator.h:30

-#if _LIBCPP_STD_VER <= 17
+#if _LIBCPP_STD_VER <= 17 || defined(_LIBCPP_ENABLE_CXX20_REMOVED_ALLOCATOR_MEMBERS)
 template <>
----------------
philnik wrote:
> ilya-biryukov wrote:
> > philnik wrote:
> > > I think using `_LIBCPP_ENABLE_CXX20_REMOVED_ALLOCATOR_MEMBERS` is misleading. This doesn't just re-enable members, but an entire class specialization. Adding something like `_LIBCPP_ENABLE_CXX20_REMOVED_ALLOCATOR_VOID_SPECIALIZATION` would be the correct way IMO.
> > That keeps `allocator<void>::pointer` broken with `_LIBCPP_ENABLE_CXX20_REMOVED_ALLOCATOR_MEMBERS` defined and    `_LIBCPP_ENABLE_CXX20_REMOVED_ALLOCATOR_VOID_SPECIALIZATION` not defined. Are we okay with that?
> > 
> > This specialization was there in the `REMOVED_ALLOCATOR_MEMBERS` mode before D104323 too, this change restores this behavior.
> I think it's Ok to have two flags for this, since the two changes are orthogonal IMO. But let's see what @ldionne thinks.
Yes, I agree with using `_LIBCPP_ENABLE_CXX20_REMOVED_ALLOCATOR_VOID_SPECIALIZATION` for this.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126210