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

Arthur O'Dwyer via Phabricator reviews at reviews.llvm.org
Sat Dec 29 16:34:19 PST 2018

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?

  rCXX libc++



More information about the libcxx-commits mailing list