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

Arthur O'Dwyer via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 3 22:02:20 PDT 2018


Quuxplusone marked 7 inline comments as done.
Quuxplusone added a comment.

> I'll take a pass at the tests tomorrow. Some pointers in general:
> 
> - Tests should be named after the component they test, not how they're testing it.
> - All the tests for a single component should be in the same file.

I'm certain I'm doing it fairly wrong and you'll have to give me specific handholds like "merge these two tests into one file", "rename this file to x.cpp", etc.
The current large number of files is only because *last* iteration, you told me to split up the tests from "one big monotonic_buffer.pass.cpp" into one file per test!
The problem with "one file per component" is that I don't understand what a "component" is in this context. From last iteration, I know that "all of `monotonic_buffer_resource`" is too big of a component! I also know that it's impossible to test `deallocate` without also calling `allocate`, but the reverse is not true.



================
Comment at: include/experimental/memory_resource:471
+
+    void release();
+
----------------
EricWF wrote:
> I think it would be nice for `release` to be inline. What do you think?
Good point. It does pull a ton of stuff into the public header, but it's probably worth it.
I'm also moving the virtual destructor into the public header, so it can be inlined. (`do_allocate` becomes the new key function.)


================
Comment at: test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp:1
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
----------------
EricWF wrote:
> We don't actually need this file, though older libc++ tests will often include it.
> 
> It's only needed to keep empty directories around, but this one has children.
I was starting to wonder about this! Removed.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111





More information about the cfe-commits mailing list