[libcxx-commits] [PATCH] D112319: [NFC][libcxx] Clean up std::__call_once

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added a comment.

IMHO the "before" picture is pretty gross, but the "after" picture isn't very much improved. (Of course the diff makes the change look scarier than it probably really is, but still...)
Perhaps consider naming the helpers something like `call_once_update_from_zero_to_one(flag)`, `call_once_store_zero(flag)`, `call_once_store_minusone(flag)`, and maybe adding some comments about the flow: or maybe those comments are already present in `<mutex>`. The flow here, AIUI, is:

- `flag` starts as `0`
- While any one thread is climbing the glass hill <https://quuxplusone.github.io/blog/2020/10/23/once-flag/>, `flag` is `1`
- When that thread fails, it resets `flag` back to `0`
- Once any thread has reached the top of the hill, `flag` becomes `-1` and stays that way forever

Interestingly,

  std::once_flag f;
  int main() {
      std::call_once(f, [&]() { std::call_once(f, []() { puts("hello"); }); });
  }

is a deadlock when `!defined(_LIBCPP_HAS_NO_THREADS)` but finishes with no output when `defined(_LIBCPP_HAS_NO_THREADS)`. (This is probably intentional and a good idea, to keep the no-threads code even simpler than it would be by default, in a case that is presumably undefined behavior according to the Standard. OTOH, libstdc++ somehow manages in this case to throw `system_error` instead of deadlocking.)

Anyway, FWIW, I see no particular problem with this PR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112319