- This is section 32.6.4 of P0660R10
- https://eel.is/c++draft/thread.condvarany.intwait
Details
- Reviewers
- ldionne 
- Group Reviewers
- Restricted Project 
- Commits
- rG4fa812bb52a5: [libc++] Implement `std::condition_variable_any::wait[_for/until]` overloads…
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
In general looks good. I have some minor comments. I think it would be good to keep track how often predicates are called and whether that matches the (minimal) expectation. For example when a stop_requested() == true the predicate is called exactly one time.
| libcxx/include/condition_variable | ||
|---|---|---|
| 106 | Can you align the // since C++20 ? | |
| 319 | ||
| 328 | ||
| libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for_token_pred.pass.cpp | ||
| 96 | This is our typical code style. | |
| 135 | I wonder here and at other places whether 2 ms is long enough. Would it make sense to validate how often the predicate is executed? When it's less than 2 the loop was executed 0 times. | |
| libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_token_pred.pass.cpp | ||
| 43 | Please include <concepts>. | |
Commandeering per discussion w/ Hui.
| libcxx/include/condition_variable | ||
|---|---|---|
| 95 | Let's mark this as experimental. | |
| libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for_token_pred.pass.cpp | ||
|---|---|---|
| 135 | If the thread creation is really slow, this test could be vacuous (it would test the same as the test that does request_stop() before we wait_for). Would it make sense to instead do something like: auto thread = support::make_test_thread([&]() {
    // wait until we're ready
    some_atomic.wait(...);
    ss.request_stop();
    while (!done) {
        cv.notify_all();
        std::this_thread::sleep_for(2ms);
      }
});
std::same_as<bool> auto r = cv.wait_for(lock, ss.get_token(), 1h, [&]() {
  some_atomic.notify_all(); // here signal that we're ready for the other thread to request a stop
  return false;
});
assert(!r);
done = true;
thread.join();Does this make sense at all? That way the pred will be the one to trigger the fact that we request_stop() on the other thread. Would that be more robust? | |
| libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_token_pred.pass.cpp | ||
| 80 | Whatever we do for the other test we should do here too. | |
| libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp | ||
| 130 | And here. | |
LGTM with minor nitpick. You can ignore the CI failures since they are unrelated and have been fixed since then. Thanks!
| libcxx/docs/Status/Cxx20.rst | ||
|---|---|---|
| 51 | ||