[libcxx-commits] [PATCH] D154425: [libc++] add basic runtime assertions to <latch>

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/latch:95-99
+        _LIBCPP_ASSERT_UNCATEGORIZED(
+            __update > __a_.load(memory_order_relaxed),
+            "__update is greater than the value of the internal counter");
+
         auto const __old = __a_.fetch_sub(__update, memory_order_release);
----------------
Could we instead move the `_LIBCPP_ASSERT` after the `fetch_sub`, i.e. do:

```
_LIBCPP_ASSERT_UNCATEGORIZED(
            __update >= 0, "latch::count_down called with a negative value");
auto const __old = __a_.fetch_sub(__update, memory_order_release);
_LIBCPP_ASSERT_UNCATEGORIZED(
            __update > __old,
            "__update is greater than the value of the internal counter");
```

This would not require an additional atomic load, yet we would catch that we just did something really really bad (i.e. that `__a_` is now negative and who knows what will happen next).


================
Comment at: libcxx/test/libcxx/thread/thread.latch/assert.arrive_and_wait.pass.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
Let's also add a `assert.ctor.pass.cpp` test to test the assertion you added in the constructor!


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

https://reviews.llvm.org/D154425