[PATCH] D47111: <experimental/memory_resource>: Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator]

Quuxplusone added a comment.

@EricWF ping!
and oops, I never submitted these comments, I guess.



================
Comment at: include/experimental/memory_resource:427
+{
+    static const size_t __default_buffer_capacity = 1024;
+    static const size_t __default_buffer_alignment = 16;
----------------
EricWF wrote:
> `constexpr` these constants if they're const?
Compare `<ios>`, which has "constexpr" in the synopsis comment but "static const" in the code.
However, I updated the diff to follow `<random>`'s example and use `static _LIBCPP_CONSTEXPR const` everywhere.


================
Comment at: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp:32
+    void *ret = r1.allocate(2048, big_alignment);
+    assert(ret != nullptr);
+    assert(globalMemCounter.checkNewCalledEq(1));
----------------
EricWF wrote:
> This check isn't passing with ToT clang.
Agreed, it won't pass unless the underlying `new_delete_resource` supports overaligned alignments, which isn't true unless the underlying runtime supports aligned new. Which isn't true on my MacBook.

I think the fix would be for me to copy all the XFAIL lines from `test/libcxx/language.support/support.dynamic/libcpp_deallocate.sh.cpp`, is that right?


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D47111