[libcxx-commits] [PATCH] D145183: [libc++] Implement `stop_token`

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

Geez, this is more complicated than I expected. Thanks a lot for working on this, we'll get through the whole thing.



================
Comment at: libcxx/include/__stop_token/stop_state.h:53
+
+  struct __lock_guard {
+    std::atomic<uint32_t>& __state_;
----------------
Would it make sense to try to extract this into a more general utility? It's really tricky to review, as we both just discussed it for a while, and I think trying to extract it into its own utility would help us make gradual progress on the review.


================
Comment at: libcxx/include/__stop_token/stop_state.h:64-65
+
+    _LIBCPP_HIDE_FROM_ABI __lock_guard(std::atomic<uint32_t>& __state, bool __is_locked) noexcept
+        : __state_(__state), __is_locked_(__is_locked) {}
+
----------------
Are we ever calling this with `__is_locked == false`? If so, it seems like this class is kind of like an `optional<__lock_guard>`. I would have expected that this constructor instead takes something like a `__already_locked_t` tag to express that the lock has already been taken.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145183