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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik requested changes to this revision.
philnik added inline comments.
This revision now requires changes to proceed.


================
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 <>
----------------
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.


================
Comment at: libcxx/test/std/utilities/memory/default.allocator/allocator_types.void.compile.cxx20_with_removed_members.pass.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
This is a libc++ extension, so it should go in `libcxx/test/libcxx` and not `libcxx/test/std`.


================
Comment at: libcxx/test/std/utilities/memory/default.allocator/allocator_types.void.compile.cxx20_with_removed_members.pass.cpp:18-23
+int main() {
+  static_assert((std::is_same<std::allocator<void>::pointer, void*>::value), "");
+  static_assert((std::is_same<std::allocator<void>::const_pointer, const void*>::value), "");
+  static_assert((std::is_same<std::allocator<void>::value_type, void>::value), "");
+  static_assert((std::is_same<std::allocator<void>::rebind<int>::other, std::allocator<int> >::value), "");
+}
----------------
You can just make this a `.compile.pass.cpp` and remove the `main()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126210