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

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Dec 13 12:56:18 PST 2021


ldionne requested changes to this revision.
ldionne added a subscriber: var-const.
ldionne added a comment.
This revision now requires changes to proceed.

I really like the introduction of named values like `once_flag::_Unset`. IMO, we should introduce them in a separate patch which would be trivial to approve.

Then, I would suggest we use a scope guard variable to refactor the messy `_LIBCPP_NO_EXCEPTIONS`. For example:

  void __call_once(volatile once_flag::_State_type& flag, void* arg, void (*func)(void*))
  {
  #if defined(_LIBCPP_HAS_NO_THREADS)
      if (flag == once_flag::_Unset)
      {
          __scope_guard guard = [&flag] { flag = once_flag::_Unset; };
          flag = once_flag::_Pending;
          func(arg);
          flag = once_flag::_Complete;
      }
  
  #else // !_LIBCPP_HAS_NO_THREADS
  
      __libcpp_mutex_lock(&mut);
      while (flag == once_flag::_Pending)
          __libcpp_condvar_wait(&cv, &mut);
  
      if (flag == once_flag::_Unset)
      {
          __scope_guard guard = [&mut, &flag, &cv] {
              __libcpp_mutex_lock(&mut);
              __libcpp_relaxed_store(&flag, once_flag::_Unset);
              __libcpp_mutex_unlock(&mut);
              __libcpp_condvar_broadcast(&cv);
          };
  
          __libcpp_relaxed_store(&flag, once_flag::_Pending);
          __libcpp_mutex_unlock(&mut);
          func(arg);
          __libcpp_mutex_lock(&mut);
          __libcpp_atomic_store(&flag, once_flag::_Complete, _AO_Release);
          __libcpp_mutex_unlock(&mut);
          __libcpp_condvar_broadcast(&cv);
      }
      else
      {
          __libcpp_mutex_unlock(&mut);
      }
  #endif // !_LIBCPP_HAS_NO_THREADS
  }

This is much cleaner IMO since we don't have any `#ifdef`s related to exceptions anymore, and we don't need to change the structure of the code, which obscures what's happening. Also, we've been meaning to introduce such a scope guard with @var-const to refactor some algorithms that look messy right now, so this would kill two birds with one stone. LMK what you think about this suggestion and I can open a patch for `__scope_guard` if you agree.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112319



More information about the libcxx-commits mailing list