[NFC][libcxxabi] Run clang-format on libcxxabi/src/cxa_guard_impl.h

[DanielMcIntosh-IBM]

Summary

I'm about to submit a change which involves re-writing most of
cxa_guard_impl.h. Running clang-format on the whole file first seems like a
good idea.

[ldionne]

I'd normally frown upon this since we generally don't clang-format whole files at once, but since the changes are pretty small, it might be OK to do that. But we definitely want to clang-format it right though.

libcxxabi/src/cxa_guard_impl.h

44–47

We definitely want to tweak our clang-format configuration such that nested #ifs are indented. It's really difficult to read otherwise (kind of like if we did not indent regular nested if (...) statements).

133

The line length here is 80, but we use 120 in libc++ unless I'm mistaken. I think we should be using the same configuration for libc++ and libc++abi.

[DanielMcIntosh-IBM]

Address Louis's comments

[DanielMcIntosh-IBM]

In D108231#2950768, @ldionne wrote:

I'd normally frown upon this since we generally don't clang-format whole files at once, but since the changes are pretty small, it might be OK to do that. But we definitely want to clang-format it right though.

I know we're supposed to make the whitespace/formatting changes "highly localized" to the area we're about to make changes to, but since I'm going to be making changes to %80 of the file anyways, leaving the nonconformant formatting in the remaining %20 felt rather silly. Was this the wrong decision? Is there ever a situation where running clang-format on a whole file like this is recommend?

[ldionne]

LGTM, but just for anyone watching, don't consider this as setting precedent for clang-formating full files arbitrarily. We normally don't do this, but since the author is going to almost rewrite the file, this seems somewhat more reasonable.

[ldionne]

Also, consider dropping [SystemZ][z/OS] from the commit message since it has nothing to do with those -- those tags are useful insofar as they are used only when a commit is really related to the tags, otherwise they lose their power.

[DanielMcIntosh-IBM]

Update commit tags

libcxxabi/.clang-format

 BasedOnStyle: LLVM
 
 ---
 Language: Cpp
 
 AlwaysBreakTemplateDeclarations: true
 PointerAlignment: Left
 
 # Disable formatting options which may break tests.
 SortIncludes: false
 ReflowComments: false
+
+IndentPPDirectives: AfterHash
+
+# libc++ has some long names so we need more than the 80 column limit imposed by LLVM style, for sensible formatting
+ColumnLimit: 120
 ---

libcxxabi/src/cxa_guard_impl.h

  *    independent of the C++ runtime or platform support.
  *
  */
 
 #include "__cxxabi_config.h"
 #include "include/atomic_support.h"
 #include <unistd.h>
 #if defined(__has_include)
-# if __has_include(<sys/syscall.h>)
-#   include <sys/syscall.h>
-# endif
+#  if __has_include(<sys/syscall.h>)
+#    include <sys/syscall.h>
+#  endif
 #endif
 
 #include <stdlib.h>
 #include <__threading_support>
 #ifndef _LIBCXXABI_HAS_NO_THREADS
-#if defined(__ELF__) && defined(_LIBCXXABI_LINK_PTHREAD_LIB)
-#pragma comment(lib, "pthread")
-#endif
+#  if defined(__ELF__) && defined(_LIBCXXABI_LINK_PTHREAD_LIB)
+#    pragma comment(lib, "pthread")
+#  endif
 #endif
 
 #if defined(__clang__)
-# pragma clang diagnostic push
-# pragma clang diagnostic ignored "-Wtautological-pointer-compare"
+#  pragma clang diagnostic push
+#  pragma clang diagnostic ignored "-Wtautological-pointer-compare"
 #elif defined(__GNUC__)
-# pragma GCC diagnostic push
-# pragma GCC diagnostic ignored "-Waddress"
+#  pragma GCC diagnostic push
+#  pragma GCC diagnostic ignored "-Waddress"
 #endif
 
 // To make testing possible, this header is included from both cxa_guard.cpp
 // and a number of tests.
 //
 // For this reason we place everything in an anonymous namespace -- even though
 // we're in a header. We want the actual implementation and the tests to have
 // unique definitions of the types in this header (since the tests may depend
 // on function local statics).
 //
 // To enforce this either `BUILDING_CXA_GUARD` or `TESTING_CXA_GUARD` must be
 // defined when including this file. Only `src/cxa_guard.cpp` should define
 // the former.
 #ifdef BUILDING_CXA_GUARD
-# include "abort_message.h"
-# define ABORT_WITH_MESSAGE(...) ::abort_message(__VA_ARGS__)
+#  include "abort_message.h"
+#  define ABORT_WITH_MESSAGE(...) ::abort_message(__VA_ARGS__)
 #elif defined(TESTING_CXA_GUARD)
-# define ABORT_WITH_MESSAGE(...) ::abort()
+#  define ABORT_WITH_MESSAGE(...) ::abort()
 #else
-# error "Either BUILDING_CXA_GUARD or TESTING_CXA_GUARD must be defined"
+#  error "Either BUILDING_CXA_GUARD or TESTING_CXA_GUARD must be defined"
 #endif
 
 #if __has_feature(thread_sanitizer)
 extern "C" void __tsan_acquire(void*);
 extern "C" void __tsan_release(void*);
 #else
-#define __tsan_acquire(addr) ((void)0)
-#define __tsan_release(addr) ((void)0)
+#  define __tsan_acquire(addr) ((void)0)
+#  define __tsan_release(addr) ((void)0)
 #endif
 
 namespace __cxxabiv1 {
 // Use an anonymous namespace to ensure that the tests and actual implementation
 // have unique definitions of these symbols.
 namespace {
 
 //===----------------------------------------------------------------------===//
 //                          Misc Utilities
 //===----------------------------------------------------------------------===//
 
-template <class T, T(*Init)()>
+template <class T, T (*Init)()>
 struct LazyValue {
   LazyValue() : is_init(false) {}
 
   T& get() {
     if (!is_init) {
       value = Init();
       is_init = true;
     }
     return value;
   }
- private:
+
   T value;
   bool is_init = false;
 };
 
 template <class IntType>
 class AtomicInt {
 public:
   using MemoryOrder = std::__libcpp_atomic_order;
 
-  explicit AtomicInt(IntType *b) : b_(b) {}
+  explicit AtomicInt(IntType* b) : b_(b) {}
   AtomicInt(AtomicInt const&) = delete;
   AtomicInt& operator=(AtomicInt const&) = delete;
 
-  IntType load(MemoryOrder ord) {
-    return std::__libcpp_atomic_load(b_, ord);
-  }
-  void store(IntType val, MemoryOrder ord) {
-    std::__libcpp_atomic_store(b_, val, ord);
-  }
-  IntType exchange(IntType new_val, MemoryOrder ord) {
-    return std::__libcpp_atomic_exchange(b_, new_val, ord);
-  }
-  bool compare_exchange(IntType *expected, IntType desired, MemoryOrder ord_success, MemoryOrder ord_failure) {
+  IntType load(MemoryOrder ord) { return std::__libcpp_atomic_load(b_, ord); }
+  void store(IntType val, MemoryOrder ord) { std::__libcpp_atomic_store(b_, val, ord); }
+  IntType exchange(IntType new_val, MemoryOrder ord) { return std::__libcpp_atomic_exchange(b_, new_val, ord); }
+  bool compare_exchange(IntType* expected, IntType desired, MemoryOrder ord_success, MemoryOrder ord_failure) {
     return std::__libcpp_atomic_compare_exchange(b_, expected, desired, ord_success, ord_failure);
   }
 
 private:
-  IntType *b_;
+  IntType* b_;
 };
 
 //===----------------------------------------------------------------------===//
 //                       PlatformGetThreadID
 //===----------------------------------------------------------------------===//
 
 #if defined(__APPLE__) && defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
 uint32_t PlatformThreadID() {
   static_assert(sizeof(mach_port_t) == sizeof(uint32_t), "");
-  return static_cast<uint32_t>(
-      pthread_mach_thread_np(std::__libcpp_thread_get_current_id()));
+  return static_cast<uint32_t>(pthread_mach_thread_np(std::__libcpp_thread_get_current_id()));
 }
 #elif defined(SYS_gettid) && defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
 uint32_t PlatformThreadID() {
   static_assert(sizeof(pid_t) == sizeof(uint32_t), "");
   return static_cast<uint32_t>(syscall(SYS_gettid));
 }
 #else
 constexpr uint32_t (*PlatformThreadID)() = nullptr;
 #endif
 
-
-constexpr bool PlatformSupportsThreadID() {
-  return +PlatformThreadID != nullptr;
-}
+constexpr bool PlatformSupportsThreadID() { return +PlatformThreadID != nullptr; }
 
 //===----------------------------------------------------------------------===//
 //                          GuardBase
 //===----------------------------------------------------------------------===//
 
 enum class AcquireResult {
   INIT_IS_DONE,
   INIT_IS_PENDING,

 template <class Derived>
 struct GuardObject {
   GuardObject() = delete;
   GuardObject(GuardObject const&) = delete;
   GuardObject& operator=(GuardObject const&) = delete;
 
   explicit GuardObject(uint32_t* g)
       : base_address(g), guard_byte_address(reinterpret_cast<uint8_t*>(g)),
-        init_byte_address(reinterpret_cast<uint8_t*>(g) + 1),
-        thread_id_address(nullptr) {}
+        init_byte_address(reinterpret_cast<uint8_t*>(g) + 1), thread_id_address(nullptr) {}
 
   explicit GuardObject(uint64_t* g)
       : base_address(g), guard_byte_address(reinterpret_cast<uint8_t*>(g)),
-        init_byte_address(reinterpret_cast<uint8_t*>(g) + 1),
-        thread_id_address(reinterpret_cast<uint32_t*>(g) + 1) {}
+        init_byte_address(reinterpret_cast<uint8_t*>(g) + 1), thread_id_address(reinterpret_cast<uint32_t*>(g) + 1) {}
 
 public:
   /// Implements __cxa_guard_acquire
   AcquireResult cxa_guard_acquire() {
     AtomicInt<uint8_t> guard_byte(guard_byte_address);
     if (guard_byte.load(std::_AO_Acquire) != UNSET)
       return INIT_IS_DONE;
     return derived()->acquire_init_byte();
AcquireResult acquire_init_byte() {
     *init_byte_address = PENDING_BIT;
     return INIT_IS_PENDING;
   }
 
   void release_init_byte() { *init_byte_address = COMPLETE_BIT; }
   void abort_init_byte() { *init_byte_address = UNSET; }
 };
 
-
 //===----------------------------------------------------------------------===//
 //                     Global Mutex Implementation
 //===----------------------------------------------------------------------===//
 
 struct LibcppMutex;
 struct LibcppCondVar;
 
 #ifndef _LIBCXXABI_HAS_NO_THREADS
private:
   std::__libcpp_mutex_t mutex = _LIBCPP_MUTEX_INITIALIZER;
 };
 
 struct LibcppCondVar {
   LibcppCondVar() = default;
   LibcppCondVar(LibcppCondVar const&) = delete;
   LibcppCondVar& operator=(LibcppCondVar const&) = delete;
 
-  bool wait(LibcppMutex& mut) {
-    return std::__libcpp_condvar_wait(&cond, &mut.mutex);
-  }
+  bool wait(LibcppMutex& mut) { return std::__libcpp_condvar_wait(&cond, &mut.mutex); }
   bool broadcast() { return std::__libcpp_condvar_broadcast(&cond); }
 
 private:
   std::__libcpp_condvar_t cond = _LIBCPP_CONDVAR_INITIALIZER;
 };
 #else
 struct LibcppMutex {};
 struct LibcppCondVar {};
 #endif // !defined(_LIBCXXABI_HAS_NO_THREADS)
 
-
 template <class Mutex, class CondVar, Mutex& global_mutex, CondVar& global_cond,
           uint32_t (*GetThreadID)() = PlatformThreadID>
-struct InitByteGlobalMutex
-    : GuardObject<InitByteGlobalMutex<Mutex, CondVar, global_mutex, global_cond,
-                                    GetThreadID>> {
+struct InitByteGlobalMutex : GuardObject<InitByteGlobalMutex<Mutex, CondVar, global_mutex, global_cond, GetThreadID>> {
 
   using BaseT = typename InitByteGlobalMutex::GuardObject;
   using BaseT::BaseT;
 
-  explicit InitByteGlobalMutex(uint32_t *g)
-    : BaseT(g), has_thread_id_support(false) {}
-  explicit InitByteGlobalMutex(uint64_t *g)
-    : BaseT(g), has_thread_id_support(PlatformSupportsThreadID()) {}
+  explicit InitByteGlobalMutex(uint32_t* g) : BaseT(g), has_thread_id_support(false) {}
+  explicit InitByteGlobalMutex(uint64_t* g) : BaseT(g), has_thread_id_support(PlatformSupportsThreadID()) {}
 
 public:
   AcquireResult acquire_init_byte() {
     LockGuard g("__cxa_guard_acquire");
     // Check for possible recursive initialization.
     if (has_thread_id_support && (*init_byte_address & PENDING_BIT)) {
       if (*thread_id_address == current_thread_id.get())
         ABORT_WITH_MESSAGE("__cxa_guard_acquire detected recursive initialization");
     }
 
     // Wait until the pending bit is not set.
     while (*init_byte_address & PENDING_BIT) {
       *init_byte_address |= WAITING_BIT;
       global_cond.wait(global_mutex);
     }
 
private:
   LazyValue<uint32_t, GetThreadID> current_thread_id;
 
 private:
   struct LockGuard {
     LockGuard() = delete;
     LockGuard(LockGuard const&) = delete;
     LockGuard& operator=(LockGuard const&) = delete;
 
-    explicit LockGuard(const char* calling_func)
-        : calling_func_(calling_func)  {
+    explicit LockGuard(const char* calling_func) : calling_func_(calling_func) {
       if (global_mutex.lock())
         ABORT_WITH_MESSAGE("%s failed to acquire mutex", calling_func_);
     }
 
     ~LockGuard() {
       if (global_mutex.unlock())
         ABORT_WITH_MESSAGE("%s failed to release mutex", calling_func_);
     }
void PlatformFutexWake(int* addr) {
   __tsan_release(addr);
   syscall(SYS_futex, addr, WAKE, INT_MAX);
 }
 #else
 constexpr void (*PlatformFutexWait)(int*, int) = nullptr;
 constexpr void (*PlatformFutexWake)(int*) = nullptr;
 #endif
 
-constexpr bool PlatformSupportsFutex() {
-  return +PlatformFutexWait != nullptr;
-}
+constexpr bool PlatformSupportsFutex() { return +PlatformFutexWait != nullptr; }
 
 /// InitByteFutex - Manages initialization using atomics and the futex syscall
 /// for waiting and waking.
-template <void (*Wait)(int*, int) = PlatformFutexWait,
-          void (*Wake)(int*) = PlatformFutexWake,
+template <void (*Wait)(int*, int) = PlatformFutexWait, void (*Wake)(int*) = PlatformFutexWake,
           uint32_t (*GetThreadIDArg)() = PlatformThreadID>
 struct InitByteFutex : GuardObject<InitByteFutex<Wait, Wake, GetThreadIDArg>> {
   using BaseT = typename InitByteFutex::GuardObject;
 
   /// ARM Constructor
-  explicit InitByteFutex(uint32_t *g) : BaseT(g),
-    init_byte(this->init_byte_address),
-    has_thread_id_support(this->thread_id_address && GetThreadIDArg),
+  explicit InitByteFutex(uint32_t* g)
+      : BaseT(g), init_byte(this->init_byte_address), has_thread_id_support(this->thread_id_address && GetThreadIDArg),
         thread_id(this->thread_id_address) {}
 
   /// Itanium Constructor
-  explicit InitByteFutex(uint64_t *g) : BaseT(g),
-    init_byte(this->init_byte_address),
-    has_thread_id_support(this->thread_id_address && GetThreadIDArg),
+  explicit InitByteFutex(uint64_t* g)
+      : BaseT(g), init_byte(this->init_byte_address), has_thread_id_support(this->thread_id_address && GetThreadIDArg),
         thread_id(this->thread_id_address) {}
 
 public:
   AcquireResult acquire_init_byte() {
     while (true) {
       uint8_t last_val = UNSET;
-      if (init_byte.compare_exchange(&last_val, PENDING_BIT, std::_AO_Acq_Rel,
-                                     std::_AO_Acquire)) {
+      if (init_byte.compare_exchange(&last_val, PENDING_BIT, std::_AO_Acq_Rel, std::_AO_Acquire)) {
         if (has_thread_id_support) {
           thread_id.store(current_thread_id.get(), std::_AO_Relaxed);
         }
         return INIT_IS_PENDING;
       }
 
       if (last_val == COMPLETE_BIT)
         return INIT_IS_DONE;
 
       if (last_val & PENDING_BIT) {
 
         // Check for recursive initialization
         if (has_thread_id_support && thread_id.load(std::_AO_Relaxed) == current_thread_id.get()) {
-            ABORT_WITH_MESSAGE("__cxa_guard_acquire detected recursive initialization");
         }
 
         if ((last_val & WAITING_BIT) == 0) {
           // This compare exchange can fail for several reasons
           // (1) another thread finished the whole thing before we got here
           // (2) another thread set the waiting bit we were trying to thread
           // (3) another thread had an exception and failed to finish
-          if (!init_byte.compare_exchange(&last_val, PENDING_BIT | WAITING_BIT,
-                                          std::_AO_Acq_Rel, std::_AO_Release)) {
+          if (!init_byte.compare_exchange(&last_val, PENDING_BIT | WAITING_BIT, std::_AO_Acq_Rel, std::_AO_Release)) {
             // (1) success, via someone else's work!
             if (last_val == COMPLETE_BIT)
               return INIT_IS_DONE;
 
             // (3) someone else, bailed on doing the work, retry from the start!
             if (last_val == UNSET)
               continue;
 
void release_init_byte() {
     if (old & WAITING_BIT)
       wake_all();
   }
 
   void abort_init_byte() {
     if (has_thread_id_support)
       thread_id.store(0, std::_AO_Relaxed);
 
-    uint8_t old = init_byte.exchange(0, std::_AO_Acq_Rel);
+    uint8_t old = init_byte.exchange(UNSET, std::_AO_Acq_Rel);
     if (old & WAITING_BIT)
       wake_all();
   }
 
 private:
   /// Use the futex to wait on the current guard variable. Futex expects a
   /// 32-bit 4-byte aligned address as the first argument, so we have to use use
   /// the base address of the guard variable (not the init byte).
   void wait_on_initialization() {
-    Wait(static_cast<int*>(this->base_address),
-         expected_value_for_futex(PENDING_BIT | WAITING_BIT));
+    Wait(static_cast<int*>(this->base_address), expected_value_for_futex(PENDING_BIT | WAITING_BIT));
   }
   void wake_all() { Wake(static_cast<int*>(this->base_address)); }
 
 private:
   AtomicInt<uint8_t> init_byte;
 
   const bool has_thread_id_support;
   // Unsafe to use unless has_thread_id_support

 
 template <class T>
 struct GlobalStatic {
   static T instance;
 };
 template <class T>
 _LIBCPP_SAFE_STATIC T GlobalStatic<T>::instance = {};
 
-enum class Implementation {
-  NoThreads,
-  GlobalLock,
-  Futex
-};
+enum class Implementation { NoThreads, GlobalLock, Futex };
 
 template <Implementation Impl>
 struct SelectImplementation;
 
 template <>
 struct SelectImplementation<Implementation::NoThreads> {
   using type = InitByteNoThreads;
 };
 
 template <>
 struct SelectImplementation<Implementation::GlobalLock> {
-  using type = InitByteGlobalMutex<
-      LibcppMutex, LibcppCondVar, GlobalStatic<LibcppMutex>::instance,
+  using type = InitByteGlobalMutex<LibcppMutex, LibcppCondVar, GlobalStatic<LibcppMutex>::instance,
                                    GlobalStatic<LibcppCondVar>::instance, PlatformThreadID>;
 };
 
 template <>
 struct SelectImplementation<Implementation::Futex> {
-  using type =
-      InitByteFutex<PlatformFutexWait, PlatformFutexWake, PlatformThreadID>;
+  using type = InitByteFutex<PlatformFutexWait, PlatformFutexWake, PlatformThreadID>;
 };
 
 // TODO(EricWF): We should prefer the futex implementation when available. But
 // it should be done in a separate step from adding the implementation.
 constexpr Implementation CurrentImplementation =
 #if defined(_LIBCXXABI_HAS_NO_THREADS)
     Implementation::NoThreads;
 #elif defined(_LIBCXXABI_USE_FUTEX)
     Implementation::Futex;
 #else
     Implementation::GlobalLock;
 #endif
 
-static_assert(CurrentImplementation != Implementation::Futex
-           || PlatformSupportsFutex(), "Futex selected but not supported");
+static_assert(CurrentImplementation != Implementation::Futex || PlatformSupportsFutex(),
+              "Futex selected but not supported");
 
-using SelectedImplementation =
-    SelectImplementation<CurrentImplementation>::type;
+using SelectedImplementation = SelectImplementation<CurrentImplementation>::type;
 
 } // end namespace
 } // end namespace __cxxabiv1
 
 #if defined(__clang__)
-# pragma clang diagnostic pop
+#  pragma clang diagnostic pop
 #elif defined(__GNUC__)
-# pragma GCC diagnostic pop
+#  pragma GCC diagnostic pop
 #endif
 
 #endif // LIBCXXABI_SRC_INCLUDE_CXA_GUARD_IMPL_H