[libcxx-commits] [PATCH] D93071: [libc++] NFCI: Implement make_shared as allocate_shared with std::allocator

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision as: libc++.
ldionne added a comment.
This revision is now accepted and ready to land.

In D93071#2447413 <https://reviews.llvm.org/D93071#2447413>, @mclow.lists wrote:

> In D93071#2447283 <https://reviews.llvm.org/D93071#2447283>, @ldionne wrote:
>
>> Indeed, however `std::allocator` is specified to use `operator new` and `operator delete`.
>
> I don't think that applies to user-provided specializations of `std::allocator`

Good point, I hadn't thought about specializations. But even then, I think it's fine. User-provided specializations are allowed as long as they depend on a user-defined type, and they satisfy the requirements of the base template. Here, `std::allocator::allocate` clearly says:

> Allocates `n * sizeof(T)` bytes of uninitialized storage by calling `::operator new(std::size_t)`

IDK whether that's considered "a requirement", but it's close enough. In any case, if it is an issue, then it's a pre-existing condition because we were using `std::allocator` already, so I think this is really a NFC. I just hope we're not violating the intent of the standard, but as I think of it, I believe we couldn't change that behavior without an ABI break (because it would require changing the control block used by `make_shared` so that it doesn't use an allocator anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93071