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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to