Changes imported from Abseil "staging" branch:

  - b527a3e4b36b644ac424e3c525b1cd393f6f6c40 Fix some typos in the usage examples by Jorg Brown <jorg@google.com>
  - 82be4a9adf3bb0ddafc0d46274969c99afffe870 Fix typo in optional.h comment. by Abseil Team <absl-team@google.com>
  - d6ee63bf8fc51fba074c23b33cebc28c808d7f07 Remove internal-only identifiers from code. by Daniel Katz <katzdm@google.com>
  - f9c3ad2f0d73f53b21603638af8b4bed636e79f4 Use easier understandable names for absl::StartsWith and ... by Abseil Team <absl-team@google.com>
  - 7c16c14fefee89c927b8789d6043c4691bcffc9b Add -Wno-missing-prototypes back to the LLVM copts. by Derek Mauro <dmauro@google.com>
  - 2f4b7d2e50c7023240242f1e15db60ccd7e8768d IWYU | absl/strings by Juemin Yang <jueminyang@google.com>
  - a99cbcc1daa34a2d6a2bb26de275e05173cc77e9 IWYU | absl/type by Juemin Yang <jueminyang@google.com>
  - 12e1146d0fc76c071d7e0ebaabb62f0a984fae66 Use LLVM_FLAGS and LLVM_TEST_FLAGS when --compiler=llvm. by Derek Mauro <dmauro@google.com>
  - cd6bea616abda558d0bace5bd77455662a233688 IWYU | absl/debugging by Juemin Yang <jueminyang@google.com>
  - d9a7382e59d46a8581b6b7a31cd5a48bb89326e9 IWYU | absl/synchronization by Juemin Yang <jueminyang@google.com>
  - 07ec7d6d5a4a666f4183c5d0ed9c342baa7b24bc IWYU | absl/numeric by Juemin Yang <jueminyang@google.com>
  - 12bfe40051f4270f8707e191af5652f83f2f750c Remove the RoundTrip{Float,Double}ToBuffer routines from ... by Jorg Brown <jorg@google.com>
  - eeb4fd67c9d97f66cb9475c3c5e51ab132f1c810 Adds conversion functions for converting between absl/tim... by Greg Miller <jgm@google.com>
  - 59a2108d05d4ea85dc5cc11e49b2cd2335d4295a Change Substitute to use %.6g formatting rather than 15/1... by Jorg Brown <jorg@google.com>
  - 394becb48e0fcd161642cdaac5120d32567e0ef8 IWYU | absl/meta by Juemin Yang <jueminyang@google.com>
  - 1e5da6e8da336699b2469dcf6dda025b9b0ec4c9 Rewrite atomic_hook.h to not use std::atomic<T*> under Wi... by Greg Falcon <gfalcon@google.com>

GitOrigin-RevId: b527a3e4b36b644ac424e3c525b1cd393f6f6c40
Change-Id: I14e331d91c956ef045ac7927091a9f179716de0c

1 files changed, 66 insertions, 38 deletions

diff --git a/absl/base/internal/atomic_hook.h b/absl/base/internal/atomic_hook.h
index 1f9a8102f7ec..47d4013928a0 100644
--- a/absl/base/internal/atomic_hook.h
+++ b/absl/base/internal/atomic_hook.h
@@ -18,28 +18,12 @@
 
 #include <atomic>
 #include <cassert>
+#include <cstdint>
 #include <utility>
 
 namespace absl {
 namespace base_internal {
 
-// In current versions of MSVC (as of July 2017), a std::atomic<T> where T is a
-// pointer to function cannot be constant-initialized with an address constant
-// expression.  That is, the following code does not compile:
-//   void NoOp() {}
-//   constexpr std::atomic<void(*)()> ptr(NoOp);
-//
-// This is the only compiler we support that seems to have this issue.  We
-// conditionalize on MSVC here to use a fallback implementation.  But we
-// should revisit this occasionally.  If MSVC fixes this compiler bug, we
-// can then change this to be conditionalized on the value on _MSC_FULL_VER
-// instead.
-#ifdef _MSC_FULL_VER
-#define ABSL_HAVE_FUNCTION_ADDRESS_CONSTANT_EXPRESSION 0
-#else
-#define ABSL_HAVE_FUNCTION_ADDRESS_CONSTANT_EXPRESSION 1
-#endif
-
 template <typename T>
 class AtomicHook;
 
@@ -55,7 +39,7 @@ class AtomicHook<ReturnType (*)(Args...)> {
  public:
   using FnPtr = ReturnType (*)(Args...);
 
-  constexpr AtomicHook() : hook_(DummyFunction) {}
+  constexpr AtomicHook() : hook_(kInitialValue) {}
 
   // Stores the provided function pointer as the value for this hook.
   //
@@ -64,28 +48,16 @@ class AtomicHook<ReturnType (*)(Args...)> {
   // as a memory_order_release operation, and read accesses are implemented as
   // memory_order_acquire.
   void Store(FnPtr fn) {
-    assert(fn);
-    FnPtr expected = DummyFunction;
-    hook_.compare_exchange_strong(expected, fn, std::memory_order_acq_rel,
-                                  std::memory_order_acquire);
-    // If the compare and exchange failed, make sure that's because hook_ was
-    // already set to `fn` by an earlier call.  Any other state reflects an API
-    // violation (calling Store() multiple times with different values).
-    //
-    // Avoid ABSL_RAW_CHECK, since raw logging depends on AtomicHook.
-    assert(expected == DummyFunction || expected == fn);
+    bool success = DoStore(fn);
+    static_cast<void>(success);
+    assert(success);
   }
 
   // Invokes the registered callback.  If no callback has yet been registered, a
   // default-constructed object of the appropriate type is returned instead.
   template <typename... CallArgs>
   ReturnType operator()(CallArgs&&... args) const {
-    FnPtr hook = hook_.load(std::memory_order_acquire);
-    if (ABSL_HAVE_FUNCTION_ADDRESS_CONSTANT_EXPRESSION || hook) {
-      return hook(std::forward<CallArgs>(args)...);
-    } else {
-      return ReturnType();
-    }
+    return DoLoad()(std::forward<CallArgs>(args)...);
   }
 
   // Returns the registered callback, or nullptr if none has been registered.
@@ -98,23 +70,79 @@ class AtomicHook<ReturnType (*)(Args...)> {
   // Load()() unless you must conditionalize behavior on whether a hook was
   // registered.
   FnPtr Load() const {
-    FnPtr ptr = hook_.load(std::memory_order_acquire);
+    FnPtr ptr = DoLoad();
     return (ptr == DummyFunction) ? nullptr : ptr;
   }
 
  private:
-#if ABSL_HAVE_FUNCTION_ADDRESS_CONSTANT_EXPRESSION
   static ReturnType DummyFunction(Args...) {
     return ReturnType();
   }
+
+  // Current versions of MSVC (as of September 2017) have a broken
+  // implementation of std::atomic<T*>:  Its constructor attempts to do the
+  // equivalent of a reinterpret_cast in a constexpr context, which is not
+  // allowed.
+  //
+  // This causes an issue when building with LLVM under Windows.  To avoid this,
+  // we use a less-efficient, intptr_t-based implementation on Windows.
+
+#ifdef _MSC_FULL_VER
+#define ABSL_HAVE_WORKING_ATOMIC_POINTER 0
 #else
-  static constexpr FnPtr DummyFunction = nullptr;
+#define ABSL_HAVE_WORKING_ATOMIC_POINTER 1
 #endif
 
+#if ABSL_HAVE_WORKING_ATOMIC_POINTER
+  static constexpr FnPtr kInitialValue = &DummyFunction;
+
+  // Return the stored value, or DummyFunction if no value has been stored.
+  FnPtr DoLoad() const { return hook_.load(std::memory_order_acquire); }
+
+  // Store the given value.  Returns false if a different value was already
+  // stored to this object.
+  bool DoStore(FnPtr fn) {
+    assert(fn);
+    FnPtr expected = DummyFunction;
+    hook_.compare_exchange_strong(expected, fn, std::memory_order_acq_rel,
+                                  std::memory_order_acquire);
+    const bool store_succeeded = (expected == DummyFunction);
+    const bool same_value_already_stored = (expected == fn);
+    return store_succeeded || same_value_already_stored;
+  }
+
   std::atomic<FnPtr> hook_;
+#else  // !ABSL_HAVE_WORKING_ATOMIC_POINTER
+  // Use a sentinel value unlikely to be the address of an actual function.
+  static constexpr intptr_t kInitialValue = 0;
+
+  static_assert(sizeof(intptr_t) >= sizeof(FnPtr),
+                "intptr_t can't contain a function pointer");
+
+  FnPtr DoLoad() const {
+    const intptr_t value = hook_.load(std::memory_order_acquire);
+    if (value == 0) {
+      return DummyFunction;
+    }
+    return reinterpret_cast<FnPtr>(value);
+  }
+
+  bool DoStore(FnPtr fn) {
+    assert(fn);
+    const auto value = reinterpret_cast<intptr_t>(fn);
+    intptr_t expected = 0;
+    hook_.compare_exchange_strong(expected, value, std::memory_order_acq_rel,
+                                  std::memory_order_acquire);
+    const bool store_succeeded = (expected == 0);
+    const bool same_value_already_stored = (expected == value);
+    return store_succeeded || same_value_already_stored;
+  }
+
+  std::atomic<intptr_t> hook_;
+#endif
 };
 
-#undef ABSL_HAVE_FUNCTION_ADDRESS_CONSTANT_EXPRESSION
+#undef ABSL_HAVE_WORKING_ATOMIC_POINTER
 
 }  // namespace base_internal
 }  // namespace absl