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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits