[PATCH] D50860: [libc++][test] Remove non-portable assumption that thread's constructor allocates with ::new

Eric Fiselier via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Apr 27 21:51:48 PDT 2019


EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.

LGTM minus inline comments.



================
Comment at: test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp:171
 #ifndef TEST_HAS_NO_EXCEPTIONS
-    {
+    if (numAllocs > 0) {
         try
----------------
CaseyCarter wrote:
> EricWF wrote:
> > I'm not sure I understand this change either.
> > 
> If thread creation in `test_throwing_new_during_thread_creation` resulted in `0` calls to `::operator new`, the expectation is that the same will occur here when we create a thread. If `::operator new` isn't called, it can't throw the exception this test is expecting to catch.
Since libc++ seems to allocate, could we sanity test that?

```
// The test below is non-portable because it expects `std::thread` to call `new`, which may not be the case for all implementations.
LIBCPP_ASSERT(numAllocs > 0); // libc++ should call new. Sanity check `numAllocs`.
if (numAllocs > 0) { ... }
```


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

https://reviews.llvm.org/D50860





More information about the cfe-commits mailing list