about summary refs log tree commit diff
path: root/absl
diff options
context:
space:
mode:
authorAbseil Team <absl-team@google.com>2019-08-23T18·38-0700
committerXiaoyi Zhang <zhangxy@google.com>2019-08-23T18·48-0400
commit2d2d7fbc283315b676159716376e739d3d23ed94 (patch)
tree6530edfdc7591c4f89d0aa48442cc6d452c47735 /absl
parent0302d1e5fa4fcdd1763b7d1bb3212943b1ae911d (diff)
Export of internal Abseil changes
--
d6748c733a70cd74ad9b76a0c9cd6b3fe2cecacf by Xiaoyi Zhang <zhangxy@google.com>:

Remove empty block, to address alerts reported in
https://github.com/abseil/abseil-cpp/issues/368.

PiperOrigin-RevId: 265099887

--
232e2036b5668d6d1296b881f9347756d84541ee by Derek Mauro <dmauro@google.com>:

Make the Linux Bazel CI scripts test with the exception mode explicitly set.

PiperOrigin-RevId: 265092105

--
942a40696c2c9b833be03e92d22a6ede7bccb6d4 by Xiaoyi Zhang <zhangxy@google.com>:

Import https://github.com/abseil/abseil-cpp/pull/372.
Suppress the unused variable warning on GCC, i.e. "-Wunused-variable".

PiperOrigin-RevId: 265063925

--
7ef90796b52cbdc260afc77cf47206f9356471d0 by Xiaoyi Zhang <zhangxy@google.com>:

Add quotes to `ABSL_COMMON_INCLUDE_DIRS` since it's a list and may contain a
`;`. This addresses https://github.com/abseil/abseil-cpp/issues/373.

PiperOrigin-RevId: 265059077

--
43f3ae742e00b83672ad6c5bc5b17fdb8f9fe6fe by Gennadiy Rozental <rogeeff@google.com>:

Internal re-organization

PiperOrigin-RevId: 264913945

--
6a2adf9c08ee1d98cc6b2855a676345c6495294a by Andy Soffer <asoffer@google.com>:

Publicly expose type names for uniform interval tags as in, for example, absl::IntervalClosedClosedTag, and add equality comparison operators.

PiperOrigin-RevId: 264861162

--
3c90c6e05fd61d56b419cd2d39dab8f17b8711b8 by Abseil Team <absl-team@google.com>:

Add validity check on returned frame pointer.

PiperOrigin-RevId: 264858823

--
2db87e0cfa0c6bea7ba81684b834cb8a73b7d748 by Gennadiy Rozental <rogeeff@google.com>:

Add MUST_USE_RESULT attribute to absl::GetFlag to prevent accidental misuse.

PiperOrigin-RevId: 264782762
GitOrigin-RevId: d6748c733a70cd74ad9b76a0c9cd6b3fe2cecacf
Change-Id: I169e9c5358e4f63000c1255e806d26b8afecf5ff
Diffstat (limited to 'absl')
-rw-r--r--absl/base/internal/spinlock.h3
-rw-r--r--absl/debugging/internal/stacktrace_x86-inl.inc8
-rw-r--r--absl/flags/BUILD.bazel2
-rw-r--r--absl/flags/CMakeLists.txt2
-rw-r--r--absl/flags/declare.h5
-rw-r--r--absl/flags/flag.cc16
-rw-r--r--absl/flags/flag.h106
-rw-r--r--absl/flags/internal/commandlineflag.cc93
-rw-r--r--absl/flags/internal/commandlineflag.h64
-rw-r--r--absl/flags/internal/flag.cc51
-rw-r--r--absl/flags/internal/flag.h49
-rw-r--r--absl/flags/internal/registry.cc62
-rw-r--r--absl/flags/internal/registry.h21
-rw-r--r--absl/flags/internal/usage_test.cc2
-rw-r--r--absl/random/distributions.h19
-rw-r--r--absl/random/distributions_test.cc19
-rw-r--r--absl/random/internal/uniform_helper.h57
17 files changed, 353 insertions, 226 deletions
diff --git a/absl/base/internal/spinlock.h b/absl/base/internal/spinlock.h
index a55aa20b4315..6ee60ac8c1ff 100644
--- a/absl/base/internal/spinlock.h
+++ b/absl/base/internal/spinlock.h
@@ -225,11 +225,10 @@ inline uint32_t SpinLock::TryLockInternal(uint32_t lock_value,
     }
   }
 
-  if (lockword_.compare_exchange_strong(
+  if (!lockword_.compare_exchange_strong(
           lock_value,
           kSpinLockHeld | lock_value | wait_cycles | sched_disabled_bit,
           std::memory_order_acquire, std::memory_order_relaxed)) {
-  } else {
     base_internal::SchedulingGuard::EnableRescheduling(sched_disabled_bit != 0);
   }
 
diff --git a/absl/debugging/internal/stacktrace_x86-inl.inc b/absl/debugging/internal/stacktrace_x86-inl.inc
index 25aa8bdf8c8c..9494441e1cff 100644
--- a/absl/debugging/internal/stacktrace_x86-inl.inc
+++ b/absl/debugging/internal/stacktrace_x86-inl.inc
@@ -36,6 +36,8 @@
 
 #include "absl/base/internal/raw_logging.h"
 
+using absl::debugging_internal::AddressIsReadable;
+
 #if defined(__linux__) && defined(__i386__)
 // Count "push %reg" instructions in VDSO __kernel_vsyscall(),
 // preceeding "syscall" or "sysenter".
@@ -81,7 +83,7 @@ static int CountPushInstructions(const unsigned char *const addr) {
       // "mov reg,reg"
       if (addr[i + 1] == 0xE5) {
         // Found "mov %esp,%ebp".
-        return 0;  
+        return 0;
       }
       ++i;  // Skip register encoding byte.
     } else if (addr[i] == 0x0F &&
@@ -222,7 +224,7 @@ static void **NextStackFrame(void **old_fp, const void *uc) {
           // "double fault" in case we hit the first fault due to e.g. stack
           // corruption.
           void *const reg_esp2 = reg_esp[num_push_instructions - 1];
-          if (absl::debugging_internal::AddressIsReadable(reg_esp2)) {
+          if (AddressIsReadable(reg_esp2)) {
             // Alleged %esp is readable, use it for further unwinding.
             new_fp = reinterpret_cast<void **>(reg_esp2);
           }
@@ -274,7 +276,7 @@ static void **NextStackFrame(void **old_fp, const void *uc) {
     // Note: NextStackFrame<false>() is only called while the program
     //       is already on its last leg, so it's ok to be slow here.
 
-    if (!absl::debugging_internal::AddressIsReadable(new_fp)) {
+    if (!AddressIsReadable(new_fp)) {
       return nullptr;
     }
   }
diff --git a/absl/flags/BUILD.bazel b/absl/flags/BUILD.bazel
index c338dbeef4a2..e7742de62407 100644
--- a/absl/flags/BUILD.bazel
+++ b/absl/flags/BUILD.bazel
@@ -136,6 +136,7 @@ cc_library(
     name = "flag",
     srcs = [
         "flag.cc",
+        "internal/flag.cc",
     ],
     hdrs = [
         "declare.h",
@@ -152,6 +153,7 @@ cc_library(
         "//absl/base",
         "//absl/base:core_headers",
         "//absl/strings",
+        "//absl/synchronization",
     ],
 )
 
diff --git a/absl/flags/CMakeLists.txt b/absl/flags/CMakeLists.txt
index 9860979c98e9..d9084a9c6aca 100644
--- a/absl/flags/CMakeLists.txt
+++ b/absl/flags/CMakeLists.txt
@@ -120,6 +120,7 @@ absl_cc_library(
     flags
   SRCS
     "flag.cc"
+    "internal/flag.cc"
   HDRS
     "declare.h"
     "flag.h"
@@ -136,6 +137,7 @@ absl_cc_library(
     absl::base
     absl::core_headers
     absl::strings
+    absl::synchronization
 )
 
 # Internal-only target, do not depend on directly.
diff --git a/absl/flags/declare.h b/absl/flags/declare.h
index 556ec5e9aef9..0a113a2b27a3 100644
--- a/absl/flags/declare.h
+++ b/absl/flags/declare.h
@@ -39,8 +39,13 @@ class Flag;
 // Flag
 //
 // Forward declaration of the `absl::Flag` type for use in defining the macro.
+#if defined(_MSC_VER)
+template <typename T>
+class Flag;
+#else
 template <typename T>
 using Flag = flags_internal::Flag<T>;
+#endif
 
 }  // namespace absl
 
diff --git a/absl/flags/flag.cc b/absl/flags/flag.cc
index f16cc75e3843..a02d069e9b00 100644
--- a/absl/flags/flag.cc
+++ b/absl/flags/flag.cc
@@ -41,4 +41,20 @@ ABSL_FLAGS_INTERNAL_FOR_EACH_LOCK_FREE(ABSL_FLAGS_ATOMIC_GET)
 
 #undef ABSL_FLAGS_ATOMIC_GET
 
+// This global nutex protects on-demand construction of flag objects in MSVC
+// builds.
+#if defined(_MSC_VER)
+
+namespace flags_internal {
+
+ABSL_CONST_INIT static absl::Mutex construction_guard(absl::kConstInit);
+
+void LockGlobalConstructionGuard() { construction_guard.Lock(); }
+
+void UnlockGlobalConstructionGuard() { construction_guard.Unlock(); }
+
+}  // namespace flags_internal
+
+#endif
+
 }  // namespace absl
diff --git a/absl/flags/flag.h b/absl/flags/flag.h
index 36c771cf49b1..881f8502ef01 100644
--- a/absl/flags/flag.h
+++ b/absl/flags/flag.h
@@ -63,8 +63,100 @@ namespace absl {
 //      ABSL_FLAG(int, count, 0, "Count of items to process");
 //
 // No public methods of `absl::Flag<T>` are part of the Abseil Flags API.
+#if !defined(_MSC_VER)
 template <typename T>
 using Flag = flags_internal::Flag<T>;
+#else
+// MSVC debug builds do not implement constexpr correctly for classes with
+// virtual methods. To work around this we adding level of indirection, so that
+// the class `absl::Flag` contains an `internal::Flag*` (instead of being an
+// alias to that class) and dynamically allocates an instance when necessary.
+// We also forward all calls to internal::Flag methods via trampoline methods.
+// In this setup the `absl::Flag` class does not have virtual methods and thus
+// MSVC is able to initialize it at link time. To deal with multiple threads
+// accessing the flag for the first time concurrently we use an atomic boolean
+// indicating if flag object is constructed. We also employ the double-checked
+// locking pattern where the second level of protection is a global Mutex, so
+// if two threads attempt to construct the flag concurrently only one wins.
+
+namespace flags_internal {
+void LockGlobalConstructionGuard();
+void UnlockGlobalConstructionGuard();
+}  // namespace flags_internal
+
+template <typename T>
+class Flag {
+ public:
+  constexpr Flag(const char* name, const flags_internal::HelpGenFunc help_gen,
+                 const char* filename,
+                 const flags_internal::FlagMarshallingOpFn marshalling_op,
+                 const flags_internal::InitialValGenFunc initial_value_gen)
+      : name_(name),
+        help_gen_(help_gen),
+        filename_(filename),
+        marshalling_op_(marshalling_op),
+        initial_value_gen_(initial_value_gen),
+        inited_(false) {}
+
+  flags_internal::Flag<T>* GetImpl() const {
+    if (!inited_.load(std::memory_order_acquire)) {
+      flags_internal::LockGlobalConstructionGuard();
+
+      if (inited_.load(std::memory_order_acquire)) {
+        return impl_;
+      }
+
+      impl_ = new flags_internal::Flag<T>(name_, help_gen_, filename_,
+                                          marshalling_op_, initial_value_gen_);
+      inited_.store(true, std::memory_order_release);
+
+      flags_internal::UnlockGlobalConstructionGuard();
+    }
+
+    return impl_;
+  }
+
+  // absl::Flag API
+  bool IsRetired() const { return GetImpl()->IsRetired(); }
+  bool IsAbseilFlag() const { return GetImpl()->IsAbseilFlag(); }
+  absl::string_view Name() const { return GetImpl()->Name(); }
+  std::string Help() const { return GetImpl()->Help(); }
+  bool IsModified() const { return GetImpl()->IsModified(); }
+  void SetModified(bool is_modified) { GetImpl()->SetModified(is_modified); }
+  bool IsSpecifiedOnCommandLine() const {
+    GetImpl()->IsSpecifiedOnCommandLine();
+  }
+  absl::string_view Typename() const { return GetImpl()->Typename(); }
+  std::string Filename() const { return GetImpl()->Filename(); }
+  std::string DefaultValue() const { return GetImpl()->DefaultValue(); }
+  std::string CurrentValue() const { return GetImpl()->CurrentValue(); }
+  bool HasValidatorFn() const { return GetImpl()->HasValidatorFn(); }
+  bool InvokeValidator(const void* value) const {
+    return GetImpl()->InvokeValidator(value);
+  }
+  template <typename T>
+  inline bool IsOfType() const {
+    return GetImpl()->IsOftype<T>();
+  }
+  T Get() const { return GetImpl()->Get(); }
+  bool AtomicGet(T* v) const { return GetImpl()->AtomicGet(v); }
+  void Set(const T& v) { GetImpl()->Set(v); }
+  void SetCallback(const flags_internal::FlagCallback mutation_callback) {
+    GetImpl()->SetCallback(mutation_callback);
+  }
+  void InvokeCallback() { GetImpl()->InvokeCallback(); }
+
+ private:
+  const char* name_;
+  const flags_internal::HelpGenFunc help_gen_;
+  const char* filename_;
+  const flags_internal::FlagMarshallingOpFn marshalling_op_;
+  const flags_internal::InitialValGenFunc initial_value_gen_;
+
+  mutable std::atomic<bool> inited_;
+  mutable flags_internal::Flag<T>* impl_ = nullptr;
+};
+#endif
 
 // GetFlag()
 //
@@ -83,7 +175,7 @@ using Flag = flags_internal::Flag<T>;
 //   // FLAGS_firstname is a Flag of type `std::string`
 //   std::string first_name = absl::GetFlag(FLAGS_firstname);
 template <typename T>
-T GetFlag(const absl::Flag<T>& flag) {
+ABSL_MUST_USE_RESULT T GetFlag(const absl::Flag<T>& flag) {
 #define ABSL_FLAGS_INTERNAL_LOCK_FREE_VALIDATE(BIT) \
   static_assert(                                    \
       !std::is_same<T, BIT>::value,                 \
@@ -96,7 +188,7 @@ T GetFlag(const absl::Flag<T>& flag) {
 
 // Overload for `GetFlag()` for types that support lock-free reads.
 #define ABSL_FLAGS_INTERNAL_LOCK_FREE_EXPORT(T) \
-  extern T GetFlag(const absl::Flag<T>& flag);
+  ABSL_MUST_USE_RESULT T GetFlag(const absl::Flag<T>& flag);
 ABSL_FLAGS_INTERNAL_FOR_EACH_LOCK_FREE(ABSL_FLAGS_INTERNAL_LOCK_FREE_EXPORT)
 #undef ABSL_FLAGS_INTERNAL_LOCK_FREE_EXPORT
 
@@ -184,13 +276,23 @@ void SetFlag(absl::Flag<T>* flag, const V& v) {
 #if ABSL_FLAGS_STRIP_NAMES
 #define ABSL_FLAG_IMPL_FLAGNAME(txt) ""
 #define ABSL_FLAG_IMPL_FILENAME() ""
+#if !defined(_MSC_VER)
 #define ABSL_FLAG_IMPL_REGISTRAR(T, flag) \
   absl::flags_internal::FlagRegistrar<T, false>(&flag)
 #else
+#define ABSL_FLAG_IMPL_REGISTRAR(T, flag) \
+  absl::flags_internal::FlagRegistrar<T, false>(flag.GetImpl())
+#endif
+#else
 #define ABSL_FLAG_IMPL_FLAGNAME(txt) txt
 #define ABSL_FLAG_IMPL_FILENAME() __FILE__
+#if !defined(_MSC_VER)
 #define ABSL_FLAG_IMPL_REGISTRAR(T, flag) \
   absl::flags_internal::FlagRegistrar<T, true>(&flag)
+#else
+#define ABSL_FLAG_IMPL_REGISTRAR(T, flag) \
+  absl::flags_internal::FlagRegistrar<T, true>(flag.GetImpl())
+#endif
 #endif
 
 // ABSL_FLAG_IMPL macro definition conditional on ABSL_FLAGS_STRIP_HELP
diff --git a/absl/flags/internal/commandlineflag.cc b/absl/flags/internal/commandlineflag.cc
index 98a46695304b..2063cda64d5d 100644
--- a/absl/flags/internal/commandlineflag.cc
+++ b/absl/flags/internal/commandlineflag.cc
@@ -68,7 +68,7 @@ absl::Mutex* InitFlag(CommandLineFlag* flag) {
   {
     absl::MutexLock lock(mu);
 
-    if (!flag->retired && flag->def == nullptr) {
+    if (!flag->IsRetired() && flag->def == nullptr) {
       // Need to initialize def and cur fields.
       flag->def = (*flag->make_init_value)();
       flag->cur = Clone(flag->op, flag->def);
@@ -94,16 +94,6 @@ absl::Mutex* CommandLineFlag::InitFlagIfNecessary() const
   return &this->locks->primary_mu;
 }
 
-void CommandLineFlag::Destroy() const {
-  // Values are heap allocated for retired and Abseil Flags.
-  if (IsRetired() || IsAbseilFlag()) {
-    if (this->cur) Delete(this->op, this->cur);
-    if (this->def) Delete(this->op, this->def);
-  }
-
-  delete this->locks;
-}
-
 bool CommandLineFlag::IsModified() const {
   absl::MutexLock l(InitFlagIfNecessary());
   return modified;
@@ -159,87 +149,6 @@ std::string CommandLineFlag::CurrentValue() const {
   return Unparse(this->marshalling_op, this->cur);
 }
 
-bool CommandLineFlag::HasValidatorFn() const {
-  absl::MutexLock l(InitFlagIfNecessary());
-
-  return this->validator != nullptr;
-}
-
-bool CommandLineFlag::SetValidatorFn(FlagValidator fn) {
-  absl::MutexLock l(InitFlagIfNecessary());
-
-  // ok to register the same function over and over again
-  if (fn == this->validator) return true;
-
-  // Can't set validator to a different function, unless reset first.
-  if (fn != nullptr && this->validator != nullptr) {
-    ABSL_INTERNAL_LOG(
-        WARNING, absl::StrCat("Ignoring SetValidatorFn() for flag '", Name(),
-                              "': validate-fn already registered"));
-
-    return false;
-  }
-
-  this->validator = fn;
-  return true;
-}
-
-bool CommandLineFlag::InvokeValidator(const void* value) const
-    EXCLUSIVE_LOCKS_REQUIRED(this->locks->primary_mu) {
-  if (!this->validator) {
-    return true;
-  }
-
-  (void)value;
-
-  ABSL_INTERNAL_LOG(
-      FATAL,
-      absl::StrCat("Flag '", Name(),
-                   "' of encapsulated type should not have a validator"));
-
-  return false;
-}
-
-void CommandLineFlag::SetCallback(
-    const flags_internal::FlagCallback mutation_callback) {
-  absl::MutexLock l(InitFlagIfNecessary());
-
-  callback = mutation_callback;
-
-  InvokeCallback();
-}
-
-// If the flag has a mutation callback this function invokes it. While the
-// callback is being invoked the primary flag's mutex is unlocked and it is
-// re-locked back after call to callback is completed. Callback invocation is
-// guarded by flag's secondary mutex instead which prevents concurrent callback
-// invocation. Note that it is possible for other thread to grab the primary
-// lock and update flag's value at any time during the callback invocation.
-// This is by design. Callback can get a value of the flag if necessary, but it
-// might be different from the value initiated the callback and it also can be
-// different by the time the callback invocation is completed.
-// Requires that *primary_lock be held in exclusive mode; it may be released
-// and reacquired by the implementation.
-void CommandLineFlag::InvokeCallback()
-    EXCLUSIVE_LOCKS_REQUIRED(this->locks->primary_mu) {
-  if (!this->callback) return;
-
-  // The callback lock is guaranteed initialized, because *locks->primary_mu
-  // exists.
-  absl::Mutex* callback_mu = &this->locks->callback_mu;
-
-  // When executing the callback we need the primary flag's mutex to be unlocked
-  // so that callback can retrieve the flag's value.
-  this->locks->primary_mu.Unlock();
-
-  {
-    absl::MutexLock lock(callback_mu);
-    this->callback();
-  }
-
-  this->locks->primary_mu.Lock();
-}
-
 // Attempts to parse supplied `value` string using parsing routine in the `flag`
 // argument. If parsing is successful, it will try to validate that the parsed
 // value is valid for the specified 'flag'. Finally this function stores the
diff --git a/absl/flags/internal/commandlineflag.h b/absl/flags/internal/commandlineflag.h
index d82443a7760e..235125762b27 100644
--- a/absl/flags/internal/commandlineflag.h
+++ b/absl/flags/internal/commandlineflag.h
@@ -71,13 +71,6 @@ using InitialValGenFunc = void* (*)();
 
 struct CommandLineFlagInfo;
 
-// Signature for the mutation callback used by watched Flags
-// The callback is noexcept.
-// TODO(rogeeff): add noexcept after C++17 support is added.
-using FlagCallback = void (*)();
-
-using FlagValidator = bool (*)();
-
 extern const char kStrippedFlagHelp[];
 
 // The per-type function
@@ -201,53 +194,55 @@ struct CommandLineFlag {
       const char* name_arg, HelpText help_text, const char* filename_arg,
       const flags_internal::FlagOpFn op_arg,
       const flags_internal::FlagMarshallingOpFn marshalling_op_arg,
-      const flags_internal::InitialValGenFunc initial_value_gen,
-      const bool retired_arg, void* def_arg, void* cur_arg)
+      const flags_internal::InitialValGenFunc initial_value_gen, void* def_arg,
+      void* cur_arg)
       : name(name_arg),
         help(help_text),
         filename(filename_arg),
         op(op_arg),
         marshalling_op(marshalling_op_arg),
         make_init_value(initial_value_gen),
-        retired(retired_arg),
         inited(false),
         modified(false),
         on_command_line(false),
-        validator(nullptr),
-        callback(nullptr),
         def(def_arg),
         cur(cur_arg),
         counter(0),
         atomic(kAtomicInit),
         locks(nullptr) {}
 
-  // Revert the init routine.
-  void Destroy() const;
+  // Virtual destructor
+  virtual void Destroy() const = 0;
 
   // Not copyable/assignable.
   CommandLineFlag(const CommandLineFlag&) = delete;
   CommandLineFlag& operator=(const CommandLineFlag&) = delete;
 
+  // Access methods.
+
+  // Returns true iff this object corresponds to retired flag
+  virtual bool IsRetired() const { return false; }
+  // Returns true iff this is a handle to an Abseil Flag.
+  virtual bool IsAbseilFlag() const { return true; }
+
   absl::string_view Name() const { return name; }
   std::string Help() const { return help.GetHelpText(); }
-  bool IsRetired() const { return this->retired; }
   bool IsModified() const;
   void SetModified(bool is_modified);
   bool IsSpecifiedOnCommandLine() const;
-  // Returns true iff this is a handle to an Abseil Flag.
-  bool IsAbseilFlag() const {
-    // Set to null for V1 flags
-    return this->make_init_value != nullptr;
-  }
 
   absl::string_view Typename() const;
   std::string Filename() const;
   std::string DefaultValue() const;
   std::string CurrentValue() const;
 
-  bool HasValidatorFn() const;
-  bool SetValidatorFn(FlagValidator fn);
-  bool InvokeValidator(const void* value) const;
+  // Interfaces to operate on validators.
+  virtual bool HasValidatorFn() const { return false; }
+  virtual bool InvokeValidator(const void* /*value*/) const { return true; }
+  // Invoke the flag validators for old flags.
+  // TODO(rogeeff): implement proper validators for Abseil Flags
+  bool ValidateDefaultValue() const;
+  bool ValidateInputValue(absl::string_view value) const;
 
   // Return true iff flag has type T.
   template <typename T>
@@ -268,8 +263,8 @@ struct CommandLineFlag {
     return res;
   }
 
-  void SetCallback(const flags_internal::FlagCallback mutation_callback);
-  void InvokeCallback();
+  // Interfaces to overate on callbacks.
+  virtual void InvokeCallback() {}
 
   // Sets the value of the flag based on specified std::string `value`. If the flag
   // was successfully set to new value, it returns true. Otherwise, sets `error`
@@ -286,29 +281,23 @@ struct CommandLineFlag {
   void StoreAtomic(size_t size);
 
   void CheckDefaultValueParsingRoundtrip() const;
-  // Invoke the flag validators for old flags.
-  // TODO(rogeeff): implement proper validators for Abseil Flags
-  bool ValidateDefaultValue() const;
-  bool ValidateInputValue(absl::string_view value) const;
 
   // Constant configuration for a particular flag.
- private:
+ protected:
+  ~CommandLineFlag() = default;
+
   const char* const name;
   const HelpText help;
   const char* const filename;
 
- protected:
   const FlagOpFn op;                         // Type-specific handler
   const FlagMarshallingOpFn marshalling_op;  // Marshalling ops handler
   const InitialValGenFunc make_init_value;   // Makes initial value for the flag
-  const bool retired;                        // Is the flag retired?
   std::atomic<bool> inited;  // fields have been lazily initialized
 
   // Mutable state (guarded by locks->primary_mu).
   bool modified;            // Has flag value been modified?
   bool on_command_line;     // Specified on command line.
-  FlagValidator validator;  // Validator function, or nullptr
-  FlagCallback callback;    // Mutation callback, or nullptr
   void* def;                // Lazily initialized pointer to default value
   void* cur;                // Lazily initialized pointer to current value
   int64_t counter;            // Mutation counter
@@ -326,7 +315,7 @@ struct CommandLineFlag {
 
   // Ensure that the lazily initialized fields of *flag have been initialized,
   // and return the lock which should be locked when flag's state is mutated.
-  absl::Mutex* InitFlagIfNecessary() const;
+  absl::Mutex* InitFlagIfNecessary() const LOCK_RETURNED(locks->primary_mu);
 
   // copy construct new value of flag's type in a memory referenced by dst
   // based on current flag's value
@@ -342,6 +331,11 @@ struct CommandLineFlag {
   friend bool TryParseLocked(CommandLineFlag* flag, void* dst,
                              absl::string_view value, std::string* err);
   friend absl::Mutex* InitFlag(CommandLineFlag* flag);
+
+  // This is a short term, until we completely rework persistent state
+  // storage API.
+  virtual void* GetValidator() const { return nullptr; }
+  virtual void SetValidator(void*) {}
 };
 
 // Update any copy of the flag value that is stored in an atomic word.
diff --git a/absl/flags/internal/flag.cc b/absl/flags/internal/flag.cc
new file mode 100644
index 000000000000..34629b304aa1
--- /dev/null
+++ b/absl/flags/internal/flag.cc
@@ -0,0 +1,51 @@
+//
+// Copyright 2019 The Abseil Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "absl/flags/internal/flag.h"
+
+#include "absl/synchronization/mutex.h"
+
+namespace absl {
+namespace flags_internal {
+
+// If the flag has a mutation callback this function invokes it. While the
+// callback is being invoked the primary flag's mutex is unlocked and it is
+// re-locked back after call to callback is completed. Callback invocation is
+// guarded by flag's secondary mutex instead which prevents concurrent
+// callback invocation. Note that it is possible for other thread to grab the
+// primary lock and update flag's value at any time during the callback
+// invocation. This is by design. Callback can get a value of the flag if
+// necessary, but it might be different from the value initiated the callback
+// and it also can be different by the time the callback invocation is
+// completed. Requires that *primary_lock be held in exclusive mode; it may be
+// released and reacquired by the implementation.
+void InvokeCallback(absl::Mutex* primary_mu, absl::Mutex* callback_mu,
+                    FlagCallback cb) EXCLUSIVE_LOCKS_REQUIRED(primary_mu) {
+  if (!cb) return;
+
+  // When executing the callback we need the primary flag's mutex to be
+  // unlocked so that callback can retrieve the flag's value.
+  primary_mu->Unlock();
+
+  {
+    absl::MutexLock lock(callback_mu);
+    cb();
+  }
+
+  primary_mu->Lock();
+}
+
+}  // namespace flags_internal
+}  // namespace absl
diff --git a/absl/flags/internal/flag.h b/absl/flags/internal/flag.h
index bc50de5486bc..fad36877f74e 100644
--- a/absl/flags/internal/flag.h
+++ b/absl/flags/internal/flag.h
@@ -22,20 +22,30 @@
 namespace absl {
 namespace flags_internal {
 
+// Signature for the mutation callback used by watched Flags
+// The callback is noexcept.
+// TODO(rogeeff): add noexcept after C++17 support is added.
+using FlagCallback = void (*)();
+
+void InvokeCallback(absl::Mutex* primary_mu, absl::Mutex* callback_mu,
+                    FlagCallback cb) EXCLUSIVE_LOCKS_REQUIRED(primary_mu);
+
 // This is "unspecified" implementation of absl::Flag<T> type.
 template <typename T>
-class Flag : public flags_internal::CommandLineFlag {
+class Flag final : public flags_internal::CommandLineFlag {
  public:
-  constexpr Flag(const char* name, const flags_internal::HelpGenFunc help_gen,
-                 const char* filename,
+  constexpr Flag(const char* name_arg,
+                 const flags_internal::HelpGenFunc help_gen,
+                 const char* filename_arg,
                  const flags_internal::FlagMarshallingOpFn marshalling_op_arg,
                  const flags_internal::InitialValGenFunc initial_value_gen)
       : flags_internal::CommandLineFlag(
-            name, flags_internal::HelpText::FromFunctionPointer(help_gen),
-            filename, &flags_internal::FlagOps<T>, marshalling_op_arg,
+            name_arg, flags_internal::HelpText::FromFunctionPointer(help_gen),
+            filename_arg, &flags_internal::FlagOps<T>, marshalling_op_arg,
             initial_value_gen,
-            /*retired_arg=*/false, /*def_arg=*/nullptr,
-            /*cur_arg=*/nullptr) {}
+            /*def_arg=*/nullptr,
+            /*cur_arg=*/nullptr),
+        callback_(nullptr) {}
 
   T Get() const {
     // Implementation notes:
@@ -76,6 +86,31 @@ class Flag : public flags_internal::CommandLineFlag {
   }
 
   void Set(const T& v) { this->Write(&v, &flags_internal::FlagOps<T>); }
+
+  void SetCallback(const flags_internal::FlagCallback mutation_callback) {
+    absl::MutexLock l(InitFlagIfNecessary());
+
+    callback_ = mutation_callback;
+
+    InvokeCallback();
+  }
+  void InvokeCallback() override
+      EXCLUSIVE_LOCKS_REQUIRED(this->locks->primary_mu) {
+    flags_internal::InvokeCallback(&this->locks->primary_mu,
+                                   &this->locks->callback_mu, callback_);
+  }
+
+ private:
+  void Destroy() const override {
+    // Values are heap allocated Abseil Flags.
+    if (this->cur) Delete(this->op, this->cur);
+    if (this->def) Delete(this->op, this->def);
+
+    delete this->locks;
+  }
+
+  // Flag's data
+  FlagCallback callback_;  // Mutation callback
 };
 
 // This class facilitates Flag object registration and tail expression-based
diff --git a/absl/flags/internal/registry.cc b/absl/flags/internal/registry.cc
index 4e5d1106b93c..0d4eec982ba7 100644
--- a/absl/flags/internal/registry.cc
+++ b/absl/flags/internal/registry.cc
@@ -31,18 +31,6 @@
 
 namespace absl {
 namespace flags_internal {
-namespace {
-
-void DestroyFlag(CommandLineFlag* flag) NO_THREAD_SAFETY_ANALYSIS {
-  flag->Destroy();
-
-  // CommandLineFlag handle object is heap allocated for non Abseil Flags.
-  if (!flag->IsAbseilFlag()) {
-    delete flag;
-  }
-}
-
-}  // namespace
 
 // --------------------------------------------------------------------
 // FlagRegistry
@@ -59,7 +47,7 @@ class FlagRegistry {
   FlagRegistry() = default;
   ~FlagRegistry() {
     for (auto& p : flags_) {
-      DestroyFlag(p.second);
+      p.second->Destroy();
     }
   }
 
@@ -141,7 +129,7 @@ void FlagRegistry::RegisterFlag(CommandLineFlag* flag) {
           true);
     } else if (old_flag->IsRetired()) {
       // Retired definitions are idempotent. Just keep the old one.
-      DestroyFlag(flag);
+      flag->Destroy();
       return;
     } else if (old_flag->Filename() != flag->Filename()) {
       flags_internal::ReportUsageError(
@@ -224,7 +212,7 @@ class FlagSaverImpl {
       saved.marshalling_op = flag->marshalling_op;
       {
         absl::MutexLock l(flag->InitFlagIfNecessary());
-        saved.validator = flag->validator;
+        saved.validator = flag->GetValidator();
         saved.modified = flag->modified;
         saved.on_command_line = flag->on_command_line;
         saved.current = Clone(saved.op, flag->cur);
@@ -250,7 +238,7 @@ class FlagSaverImpl {
       bool restored = false;
       {
         absl::MutexLock l(flag->InitFlagIfNecessary());
-        flag->validator = src.validator;
+        flag->SetValidator(src.validator);
         flag->modified = src.modified;
         flag->on_command_line = src.on_command_line;
         if (flag->counter != src.counter ||
@@ -290,9 +278,9 @@ class FlagSaverImpl {
     FlagOpFn op;
     FlagMarshallingOpFn marshalling_op;
     int64_t counter;
+    void* validator;
     bool modified;
     bool on_command_line;
-    bool (*validator)();
     const void* current;        // nullptr after restore
     const void* default_value;  // nullptr after restore
   };
@@ -414,14 +402,38 @@ bool RegisterCommandLineFlag(CommandLineFlag* flag) {
 
 // --------------------------------------------------------------------
 
-bool Retire(FlagOpFn ops, FlagMarshallingOpFn marshalling_ops,
-            const char* name) {
-  auto* flag = new CommandLineFlag(
-      name,
-      /*help_text=*/absl::flags_internal::HelpText::FromStaticCString(nullptr),
-      /*filename_arg=*/"RETIRED", ops, marshalling_ops,
-      /*initial_value_gen=*/nullptr,
-      /*retired_arg=*/true, nullptr, nullptr);
+namespace {
+
+class RetiredFlagObj final : public flags_internal::CommandLineFlag {
+ public:
+  constexpr RetiredFlagObj(const char* name_arg, FlagOpFn ops,
+                           FlagMarshallingOpFn marshalling_ops)
+      : flags_internal::CommandLineFlag(
+            name_arg, flags_internal::HelpText::FromStaticCString(nullptr),
+            /*filename_arg=*/"RETIRED", ops, marshalling_ops,
+            /*initial_value_gen=*/nullptr,
+            /*def_arg=*/nullptr,
+            /*cur_arg=*/nullptr) {}
+
+ private:
+  bool IsRetired() const override { return true; }
+
+  void Destroy() const override {
+    // Values are heap allocated for Retired Flags.
+    if (this->cur) Delete(this->op, this->cur);
+    if (this->def) Delete(this->op, this->def);
+
+    if (this->locks) delete this->locks;
+
+    delete this;
+  }
+};
+
+}  // namespace
+
+bool Retire(const char* name, FlagOpFn ops,
+            FlagMarshallingOpFn marshalling_ops) {
+  auto* flag = new flags_internal::RetiredFlagObj(name, ops, marshalling_ops);
   FlagRegistry::GlobalRegistry()->RegisterFlag(flag);
   return true;
 }
diff --git a/absl/flags/internal/registry.h b/absl/flags/internal/registry.h
index 27566fbf4ab7..884a8db542fa 100644
--- a/absl/flags/internal/registry.h
+++ b/absl/flags/internal/registry.h
@@ -106,29 +106,16 @@ bool RegisterCommandLineFlag(CommandLineFlag*);
 //   4: Remove the old_lib 'retired' registration.
 //   5: Eventually delete the graveyard registration entirely.
 //
-// Returns bool to enable use in namespace-scope initializers.
-// For example:
-//
-//   static const bool dummy = base::RetiredFlag<int32_t>("myflag");
-//
-// Or to declare several at once:
-//
-//   static bool dummies[] = {
-//       base::RetiredFlag<std::string>("some_string_flag"),
-//       base::RetiredFlag<double>("some_double_flag"),
-//       base::RetiredFlag<int32_t>("some_int32_flag")
-//   };
 
 // Retire flag with name "name" and type indicated by ops.
-bool Retire(FlagOpFn ops, FlagMarshallingOpFn marshalling_ops,
-            const char* name);
+bool Retire(const char* name, FlagOpFn ops,
+            FlagMarshallingOpFn marshalling_ops);
 
 // Registered a retired flag with name 'flag_name' and type 'T'.
 template <typename T>
 inline bool RetiredFlag(const char* flag_name) {
-  return flags_internal::Retire(flags_internal::FlagOps<T>,
-                                flags_internal::FlagMarshallingOps<T>,
-                                flag_name);
+  return flags_internal::Retire(flag_name, flags_internal::FlagOps<T>,
+                                flags_internal::FlagMarshallingOps<T>);
 }
 
 // If the flag is retired, returns true and indicates in |*type_is_bool|
diff --git a/absl/flags/internal/usage_test.cc b/absl/flags/internal/usage_test.cc
index 781e1d2b16ab..8538b2b60884 100644
--- a/absl/flags/internal/usage_test.cc
+++ b/absl/flags/internal/usage_test.cc
@@ -395,7 +395,7 @@ TEST_F(UsageReportingTest, TestUsageFlag_helpon) {
 }  // namespace
 
 int main(int argc, char* argv[]) {
-  absl::GetFlag(FLAGS_undefok);  // Force linking of parse.cc
+  (void)absl::GetFlag(FLAGS_undefok);  // Force linking of parse.cc
   flags::SetProgramInvocationName("usage_test");
   absl::SetProgramUsageMessage(kTestUsageMessage);
   ::testing::InitGoogleTest(&argc, argv);
diff --git a/absl/random/distributions.h b/absl/random/distributions.h
index d8ba3cdbe85e..18ff248477b5 100644
--- a/absl/random/distributions.h
+++ b/absl/random/distributions.h
@@ -68,18 +68,13 @@
 
 namespace absl {
 
-ABSL_INTERNAL_INLINE_CONSTEXPR(random_internal::IntervalClosedClosedT,
-                               IntervalClosedClosed, {});
-ABSL_INTERNAL_INLINE_CONSTEXPR(random_internal::IntervalClosedClosedT,
-                               IntervalClosed, {});
-ABSL_INTERNAL_INLINE_CONSTEXPR(random_internal::IntervalClosedOpenT,
-                               IntervalClosedOpen, {});
-ABSL_INTERNAL_INLINE_CONSTEXPR(random_internal::IntervalOpenOpenT,
-                               IntervalOpenOpen, {});
-ABSL_INTERNAL_INLINE_CONSTEXPR(random_internal::IntervalOpenOpenT,
-                               IntervalOpen, {});
-ABSL_INTERNAL_INLINE_CONSTEXPR(random_internal::IntervalOpenClosedT,
-                               IntervalOpenClosed, {});
+ABSL_INTERNAL_INLINE_CONSTEXPR(IntervalClosedClosedTag, IntervalClosedClosed,
+                               {});
+ABSL_INTERNAL_INLINE_CONSTEXPR(IntervalClosedClosedTag, IntervalClosed, {});
+ABSL_INTERNAL_INLINE_CONSTEXPR(IntervalClosedOpenTag, IntervalClosedOpen, {});
+ABSL_INTERNAL_INLINE_CONSTEXPR(IntervalOpenOpenTag, IntervalOpenOpen, {});
+ABSL_INTERNAL_INLINE_CONSTEXPR(IntervalOpenOpenTag, IntervalOpen, {});
+ABSL_INTERNAL_INLINE_CONSTEXPR(IntervalOpenClosedTag, IntervalOpenClosed, {});
 
 // -----------------------------------------------------------------------------
 // absl::Uniform<T>(tag, bitgen, lo, hi)
diff --git a/absl/random/distributions_test.cc b/absl/random/distributions_test.cc
index eb82868d5320..2d92723ad2b4 100644
--- a/absl/random/distributions_test.cc
+++ b/absl/random/distributions_test.cc
@@ -171,14 +171,11 @@ void CheckArgsInferType() {
       "");
   static_assert(
       absl::conjunction<
+          std::is_same<Expect, decltype(InferredTaggedUniformReturnT<
+                                        absl::IntervalOpenOpenTag, A, B>(0))>,
           std::is_same<Expect,
                        decltype(InferredTaggedUniformReturnT<
-                                absl::random_internal::IntervalOpenOpenT, A, B>(
-                           0))>,
-          std::is_same<Expect,
-                       decltype(InferredTaggedUniformReturnT<
-                                absl::random_internal::IntervalOpenOpenT, B, A>(
-                           0))>>::value,
+                                absl::IntervalOpenOpenTag, B, A>(0))>>::value,
       "");
 }
 
@@ -218,12 +215,10 @@ void CheckArgsReturnExpectedType() {
       absl::conjunction<
           std::is_same<Expect,
                        decltype(ExplicitTaggedUniformReturnT<
-                                absl::random_internal::IntervalOpenOpenT, A, B,
-                                Expect>(0))>,
-          std::is_same<Expect,
-                       decltype(ExplicitTaggedUniformReturnT<
-                                absl::random_internal::IntervalOpenOpenT, B, A,
-                                Expect>(0))>>::value,
+                                absl::IntervalOpenOpenTag, A, B, Expect>(0))>,
+          std::is_same<Expect, decltype(ExplicitTaggedUniformReturnT<
+                                        absl::IntervalOpenOpenTag, B, A,
+                                        Expect>(0))>>::value,
       "");
 }
 
diff --git a/absl/random/internal/uniform_helper.h b/absl/random/internal/uniform_helper.h
index ebcc3744b188..9e89e5268071 100644
--- a/absl/random/internal/uniform_helper.h
+++ b/absl/random/internal/uniform_helper.h
@@ -30,12 +30,33 @@ class uniform_real_distribution;
 
 // Interval tag types which specify whether the interval is open or closed
 // on either boundary.
+
 namespace random_internal {
-struct IntervalClosedClosedT {};
-struct IntervalClosedOpenT {};
-struct IntervalOpenClosedT {};
-struct IntervalOpenOpenT {};
+template <typename T>
+struct TagTypeCompare {};
+
+template <typename T>
+constexpr bool operator==(TagTypeCompare<T>, TagTypeCompare<T>) {
+  // Tags are mono-states. They always compare equal.
+  return true;
+}
+template <typename T>
+constexpr bool operator!=(TagTypeCompare<T>, TagTypeCompare<T>) {
+  return false;
+}
 
+}  // namespace random_internal
+
+struct IntervalClosedClosedTag
+    : public random_internal::TagTypeCompare<IntervalClosedClosedTag> {};
+struct IntervalClosedOpenTag
+    : public random_internal::TagTypeCompare<IntervalClosedOpenTag> {};
+struct IntervalOpenClosedTag
+    : public random_internal::TagTypeCompare<IntervalOpenClosedTag> {};
+struct IntervalOpenOpenTag
+    : public random_internal::TagTypeCompare<IntervalOpenOpenTag> {};
+
+namespace random_internal {
 // The functions
 //    uniform_lower_bound(tag, a, b)
 // and
@@ -56,8 +77,8 @@ template <typename IntType, typename Tag>
 typename absl::enable_if_t<
     absl::conjunction<
         std::is_integral<IntType>,
-        absl::disjunction<std::is_same<Tag, IntervalOpenClosedT>,
-                          std::is_same<Tag, IntervalOpenOpenT>>>::value,
+        absl::disjunction<std::is_same<Tag, IntervalOpenClosedTag>,
+                          std::is_same<Tag, IntervalOpenOpenTag>>>::value,
     IntType>
 uniform_lower_bound(Tag, IntType a, IntType) {
   return a + 1;
@@ -67,8 +88,8 @@ template <typename FloatType, typename Tag>
 typename absl::enable_if_t<
     absl::conjunction<
         std::is_floating_point<FloatType>,
-        absl::disjunction<std::is_same<Tag, IntervalOpenClosedT>,
-                          std::is_same<Tag, IntervalOpenOpenT>>>::value,
+        absl::disjunction<std::is_same<Tag, IntervalOpenClosedTag>,
+                          std::is_same<Tag, IntervalOpenOpenTag>>>::value,
     FloatType>
 uniform_lower_bound(Tag, FloatType a, FloatType b) {
   return std::nextafter(a, b);
@@ -76,8 +97,8 @@ uniform_lower_bound(Tag, FloatType a, FloatType b) {
 
 template <typename NumType, typename Tag>
 typename absl::enable_if_t<
-    absl::disjunction<std::is_same<Tag, IntervalClosedClosedT>,
-                      std::is_same<Tag, IntervalClosedOpenT>>::value,
+    absl::disjunction<std::is_same<Tag, IntervalClosedClosedTag>,
+                      std::is_same<Tag, IntervalClosedOpenTag>>::value,
     NumType>
 uniform_lower_bound(Tag, NumType a, NumType) {
   return a;
@@ -87,8 +108,8 @@ template <typename IntType, typename Tag>
 typename absl::enable_if_t<
     absl::conjunction<
         std::is_integral<IntType>,
-        absl::disjunction<std::is_same<Tag, IntervalClosedOpenT>,
-                          std::is_same<Tag, IntervalOpenOpenT>>>::value,
+        absl::disjunction<std::is_same<Tag, IntervalClosedOpenTag>,
+                          std::is_same<Tag, IntervalOpenOpenTag>>>::value,
     IntType>
 uniform_upper_bound(Tag, IntType, IntType b) {
   return b - 1;
@@ -98,8 +119,8 @@ template <typename FloatType, typename Tag>
 typename absl::enable_if_t<
     absl::conjunction<
         std::is_floating_point<FloatType>,
-        absl::disjunction<std::is_same<Tag, IntervalClosedOpenT>,
-                          std::is_same<Tag, IntervalOpenOpenT>>>::value,
+        absl::disjunction<std::is_same<Tag, IntervalClosedOpenTag>,
+                          std::is_same<Tag, IntervalOpenOpenTag>>>::value,
     FloatType>
 uniform_upper_bound(Tag, FloatType, FloatType b) {
   return b;
@@ -109,8 +130,8 @@ template <typename IntType, typename Tag>
 typename absl::enable_if_t<
     absl::conjunction<
         std::is_integral<IntType>,
-        absl::disjunction<std::is_same<Tag, IntervalClosedClosedT>,
-                          std::is_same<Tag, IntervalOpenClosedT>>>::value,
+        absl::disjunction<std::is_same<Tag, IntervalClosedClosedTag>,
+                          std::is_same<Tag, IntervalOpenClosedTag>>>::value,
     IntType>
 uniform_upper_bound(Tag, IntType, IntType b) {
   return b;
@@ -120,8 +141,8 @@ template <typename FloatType, typename Tag>
 typename absl::enable_if_t<
     absl::conjunction<
         std::is_floating_point<FloatType>,
-        absl::disjunction<std::is_same<Tag, IntervalClosedClosedT>,
-                          std::is_same<Tag, IntervalOpenClosedT>>>::value,
+        absl::disjunction<std::is_same<Tag, IntervalClosedClosedTag>,
+                          std::is_same<Tag, IntervalOpenClosedTag>>>::value,
     FloatType>
 uniform_upper_bound(Tag, FloatType, FloatType b) {
   return std::nextafter(b, (std::numeric_limits<FloatType>::max)());