[PATCH] D110349: [libcxx][SystemZ][z/OS] Added are_threads_enabled and has_spawned_other_threads to __threading_support

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__threading_support:293-298
+// On z/OS some posix functions can be enabled/disabled at runtime.
+_LIBCPP_THREAD_ABI_VISIBILITY
+bool __libcpp_are_threads_enabled();
+
+_LIBCPP_THREAD_ABI_VISIBILITY
+bool __libcpp_has_spawned_other_threads();
----------------
Peanut gallery says: It seems like it would be strictly easier to reason about this code if the user-facing API were called `bool __libcpp_might_have_multiple_threads()`. That is, IIUC, we don't care so much about what's "enabled" (potentia) so much as what is //the case// right now dynamically (actus).
If `!__libcpp_might_have_multiple_threads()`, then we can skip locking mutexes (as long as they'll definitely be unlocked before the user gets a chance to spawn a second thread). I think that's what's going on in D113069, for example.
Maybe this potentia/actus distinction is the same distinction as what you're calling `are_threads_enabled` (potentia) versus `has_spawned_other_threads` (actus); but if so, I haven't seen how you propose to use `has_spawned_other_threads`, and (FWLIW) I don't understand why D113069 used `are_threads_enabled` //instead// of `has_spawned_other_threads`.
I say "might have multiple threads" instead of "definitely has multiple threads" because I imagine it's generally not possible for libc++ to tell whether the user might have spawned threads via pthreads or whatever — basically as you say on line 385, but making sure that the name of the function reflects its semantics.

(We had a bad time with `__libcpp_is_constant_evaluated`, where sometimes you wanted to distinguish between `__libcpp_might_be_constant_evaluated` and `__libcpp_is_definitely_runtime_evaluated`, and sometimes you wanted to distinguish between `__libcpp_is_definitely_constant_evaluated` and `__libcpp_might_be_runtime_evaluated`; and it was never obvious at the call-site which of those two interpretations `__libcpp_is_constant_evaluated` was meant to convey.) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110349