[libcxx-commits] [PATCH] D137739: [libc++] Implement P0339R6 (polymorphic_allocator<> as a vocabulary type)

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

In D137739#3956055 <https://reviews.llvm.org/D137739#3956055>, @LRFLEW wrote:

> The implementation of `polymorphic_allocator::allocate_object` here doesn't follow the changes in LWG 3237 <https://cplusplus.github.io/LWG/issue3237> and LWG 3310 <https://cplusplus.github.io/LWG/issue3310>. `SIZE_MAX` should be replaced with `numeric_limits<size_t>::max()` (perhaps using a templated version of `__max_size()`), and the error condition should throw `bad_array_new_length` instead of `length_error`.

Thanks for the info! I'll apply them in this patch, since they are almost trivial. It looks like we accidentally marked LWG3237 as done already.

> Also, on the topic of LWG 3304, it looks like `polymorphic_allocator::allocate` is supposed to be marked as `[[nodiscard]]` in C++20. Would it make sense for this patch to also include that change as well?

See D137597 <https://reviews.llvm.org/D137597> for that.

> Lastly, I noticed that the existing code is using `#if _LIBCPP_STD_VER > 14`, but the code added here is using `#if _LIBCPP_STD_VER >= 20`. Would it make more sense to use `#if _LIBCPP_STD_VER > 17` instead?

We decided to use `_LIBCPP_STD_VER >= x` for new code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137739