Export of internal Abseil changes

--
dea3e4f33f16bdb1d89cad1f8055b81c0c0cb554 by Andy Getzendanner <durandal@google.com>:

Validate in log_severity_test that flags of type absl::LogSeverity are lock-free.

PiperOrigin-RevId: 293454285

--
2a0cd2d8dc193a0cbff4ffa6c5c7037745507419 by Derek Mauro <dmauro@google.com>:

Update the testing instructions in CONTRIBUTING.md

PiperOrigin-RevId: 293436013

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

Introduce struct to represent storage for flag value and normalize naming of internal structs in Flag implementation.

There is no semantic changes in this CL. All the internal structs are now named as Flag... We also stop using flags_internal:: qualifications for most of them since the names are unique enough by themselves.

PiperOrigin-RevId: 293251467
GitOrigin-RevId: dea3e4f33f16bdb1d89cad1f8055b81c0c0cb554
Change-Id: I161aecc9509edae3e4b77eead02df684b2ce7087

CONTRIBUTING.md

@@ -123,10 +123,13 @@ will be expected to conform to the style outlined
 
 ## Running Tests
 
-Use "bazel test <>" functionality to run the unit tests.
+If you have [Bazel](https://bazel.build/) installed, use `bazel test
+--test_tag_filters="-benchmark" ...` to run the unit tests.
 
-Prerequisites for building and running tests are listed in
+If you are running the Linux operating system and have
-[README.md](README.md)
+[Docker](https://www.docker.com/) installed, you can also run the `linux_*.sh`
+scripts under the `ci/`(https://github.com/abseil/abseil-cpp/tree/master/ci)
+directory to test Abseil under a variety of conditions.
 
 ## Abseil Committers
 

absl/base/BUILD.bazel

@@ -675,6 +675,7 @@ cc_test(
     linkopts = ABSL_DEFAULT_LINKOPTS,
     deps = [
         ":log_severity",
+        "//absl/flags:flag_internal",
         "//absl/flags:marshalling",
         "//absl/strings",
         "@com_google_googletest//:gtest_main",

absl/base/CMakeLists.txt

@@ -610,6 +610,7 @@ absl_cc_test(
   SRCS
     "log_severity_test.cc"
   DEPS
+    absl::flags_internal
     absl::flags_marshalling
     absl::log_severity
     absl::strings

absl/base/log_severity_test.cc

@@ -24,6 +24,7 @@
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include "absl/flags/internal/flag.h"
 #include "absl/flags/marshalling.h"
 #include "absl/strings/str_cat.h"
 
@@ -51,6 +52,10 @@ TEST(StreamTest, Works) {
               Eq("absl::LogSeverity(4)"));
 }
 
+static_assert(
+    absl::flags_internal::IsAtomicFlagTypeTrait<absl::LogSeverity>::value,
+    "Flags of type absl::LogSeverity ought to be lock-free.");
+
 using ParseFlagFromOutOfRangeIntegerTest = TestWithParam<int64_t>;
 INSTANTIATE_TEST_SUITE_P(
     Instantiation, ParseFlagFromOutOfRangeIntegerTest,

absl/flags/BUILD.bazel

@@ -36,7 +36,7 @@ cc_library(
     ],
     copts = ABSL_DEFAULT_COPTS,
     linkopts = ABSL_DEFAULT_LINKOPTS,
-    visibility = ["//visibility:private"],
+    visibility = ["//absl/base:__subpackages__"],
     deps = [
         ":config",
         ":handle",

absl/flags/flag.h

@@ -120,11 +120,11 @@ class Flag {
         return impl_;
       }
 
-      impl_ = new flags_internal::Flag<T>(
+      impl_ =
-          name_, filename_, marshalling_op_,
+          new flags_internal::Flag<T>(name_, filename_, marshalling_op_,
-          {flags_internal::FlagHelpSrc(help_gen_),
+                                      {flags_internal::FlagHelpMsg(help_gen_),
-           flags_internal::FlagHelpSrcKind::kGenFunc},
+                                       flags_internal::FlagHelpKind::kGenFunc},
-          default_value_gen_);
+                                      default_value_gen_);
       inited_.store(true, std::memory_order_release);
     }
 
@@ -152,7 +152,7 @@ class Flag {
   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) {
+  void SetCallback(const flags_internal::FlagCallbackFunc mutation_callback) {
     GetImpl()->SetCallback(mutation_callback);
   }
   void InvokeCallback() { GetImpl()->InvokeCallback(); }

absl/flags/flag_test.cc

@@ -51,8 +51,8 @@ void TestCallback() {}
 
 template <typename T>
 bool TestConstructionFor() {
-  constexpr flags::HelpInitArg help_arg{flags::FlagHelpSrc("literal help"),
+  constexpr flags::FlagHelpArg help_arg{flags::FlagHelpMsg("literal help"),
-                                        flags::FlagHelpSrcKind::kLiteral};
+                                        flags::FlagHelpKind::kLiteral};
   constexpr flags::Flag<T> f1("f1", "file", &flags::FlagMarshallingOps<T>,
                               help_arg, &TestMakeDflt<T>);
   EXPECT_EQ(f1.Name(), "f1");
@@ -61,7 +61,7 @@ bool TestConstructionFor() {
 
   ABSL_CONST_INIT static flags::Flag<T> f2(
       "f2", "file", &flags::FlagMarshallingOps<T>,
-      {flags::FlagHelpSrc(&TestHelpMsg), flags::FlagHelpSrcKind::kGenFunc},
+      {flags::FlagHelpMsg(&TestHelpMsg), flags::FlagHelpKind::kGenFunc},
       &TestMakeDflt<T>);
   flags::FlagRegistrar<T, false>(&f2).OnUpdate(TestCallback);
 

absl/flags/internal/flag.cc

@@ -91,11 +91,11 @@ void FlagImpl::Init() {
 
   absl::MutexLock lock(reinterpret_cast<absl::Mutex*>(&data_guard_));
 
-  if (cur_ != nullptr) {
+  if (value_.dynamic != nullptr) {
     inited_.store(true, std::memory_order_release);
   } else {
     // Need to initialize cur field.
-    cur_ = MakeInitValue().release();
+    value_.dynamic = MakeInitValue().release();
     StoreAtomic();
     inited_.store(true, std::memory_order_release);
   }
@@ -117,7 +117,7 @@ void FlagImpl::Destroy() {
     absl::MutexLock l(DataGuard());
 
     // Values are heap allocated for Abseil Flags.
-    if (cur_) Delete(op_, cur_);
+    if (value_.dynamic) Delete(op_, value_.dynamic);
 
     // Release the dynamically allocated default value if any.
     if (def_kind_ == FlagDefaultSrcKind::kDynamicValue) {
@@ -125,7 +125,7 @@ void FlagImpl::Destroy() {
     }
 
     // If this flag has an assigned callback, release callback data.
-    if (callback_data_) delete callback_data_;
+    if (callback_) delete callback_;
   }
 
   absl::MutexLock l(&flag_mutex_lifetime_guard);
@@ -150,8 +150,8 @@ std::string FlagImpl::Filename() const {
 }
 
 std::string FlagImpl::Help() const {
-  return help_source_kind_ == FlagHelpSrcKind::kLiteral ? help_.literal
+  return help_source_kind_ == FlagHelpKind::kLiteral ? help_.literal
-                                                        : help_.gen_func();
+                                                     : help_.gen_func();
 }
 
 bool FlagImpl::IsModified() const {
@@ -174,27 +174,26 @@ std::string FlagImpl::DefaultValue() const {
 std::string FlagImpl::CurrentValue() const {
   absl::MutexLock l(DataGuard());
 
-  return Unparse(marshalling_op_, cur_);
+  return Unparse(marshalling_op_, value_.dynamic);
 }
 
-void FlagImpl::SetCallback(
+void FlagImpl::SetCallback(const FlagCallbackFunc mutation_callback) {
-    const flags_internal::FlagCallback mutation_callback) {
   absl::MutexLock l(DataGuard());
 
-  if (callback_data_ == nullptr) {
+  if (callback_ == nullptr) {
-    callback_data_ = new CallbackData;
+    callback_ = new FlagCallback;
   }
-  callback_data_->func = mutation_callback;
+  callback_->func = mutation_callback;
 
   InvokeCallback();
 }
 
 void FlagImpl::InvokeCallback() const {
-  if (!callback_data_) return;
+  if (!callback_) return;
 
   // Make a copy of the C-style function pointer that we are about to invoke
   // before we release the lock guarding it.
-  FlagCallback cb = callback_data_->func;
+  FlagCallbackFunc cb = callback_->func;
 
   // 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
@@ -208,7 +207,7 @@ void FlagImpl::InvokeCallback() const {
   // completed. Requires that *primary_lock be held in exclusive mode; it may be
   // released and reacquired by the implementation.
   MutexRelock relock(DataGuard());
-  absl::MutexLock lock(&callback_data_->guard);
+  absl::MutexLock lock(&callback_->guard);
   cb();
 }
 
@@ -267,7 +266,7 @@ void FlagImpl::Read(void* dst, const flags_internal::FlagOpFn dst_op) const {
         absl::StrCat("Flag '", Name(),
                      "' is defined as one type and declared as another"));
   }
-  CopyConstruct(op_, cur_, dst);
+  CopyConstruct(op_, value_.dynamic, dst);
 }
 
 void FlagImpl::StoreAtomic() {
@@ -275,14 +274,14 @@ void FlagImpl::StoreAtomic() {
 
   if (data_size <= sizeof(int64_t)) {
     int64_t t = 0;
-    std::memcpy(&t, cur_, data_size);
+    std::memcpy(&t, value_.dynamic, data_size);
-    atomics_.small_atomic.store(t, std::memory_order_release);
+    value_.atomics.small_atomic.store(t, std::memory_order_release);
   }
 #if defined(ABSL_FLAGS_INTERNAL_ATOMIC_DOUBLE_WORD)
   else if (data_size <= sizeof(FlagsInternalTwoWordsType)) {
     FlagsInternalTwoWordsType t{0, 0};
-    std::memcpy(&t, cur_, data_size);
+    std::memcpy(&t, value_.dynamic, data_size);
-    atomics_.big_atomic.store(t, std::memory_order_release);
+    value_.atomics.big_atomic.store(t, std::memory_order_release);
   }
 #endif
 }
@@ -313,7 +312,7 @@ void FlagImpl::Write(const void* src, const flags_internal::FlagOpFn src_op) {
 
   modified_ = true;
   counter_++;
-  Copy(op_, src, cur_);
+  Copy(op_, src, value_.dynamic);
 
   StoreAtomic();
   InvokeCallback();
@@ -334,7 +333,7 @@ bool FlagImpl::SetFromString(absl::string_view value, FlagSettingMode set_mode,
   switch (set_mode) {
     case SET_FLAGS_VALUE: {
       // set or modify the flag's value
-      if (!TryParse(&cur_, value, err)) return false;
+      if (!TryParse(&value_.dynamic, value, err)) return false;
       modified_ = true;
       counter_++;
       StoreAtomic();
@@ -348,7 +347,7 @@ bool FlagImpl::SetFromString(absl::string_view value, FlagSettingMode set_mode,
     case SET_FLAG_IF_DEFAULT: {
       // set the flag's value, but only if it hasn't been set by someone else
       if (!modified_) {
-        if (!TryParse(&cur_, value, err)) return false;
+        if (!TryParse(&value_.dynamic, value, err)) return false;
         modified_ = true;
         counter_++;
         StoreAtomic();
@@ -381,7 +380,7 @@ bool FlagImpl::SetFromString(absl::string_view value, FlagSettingMode set_mode,
 
       if (!modified_) {
         // Need to set both default value *and* current, in this case
-        Copy(op_, default_src_.dynamic_value, cur_);
+        Copy(op_, default_src_.dynamic_value, value_.dynamic);
         StoreAtomic();
         InvokeCallback();
       }

absl/flags/internal/flag.h

@@ -22,6 +22,7 @@
 #include <cstring>
 #include <memory>
 #include <string>
+#include <type_traits>
 
 #include "absl/base/config.h"
 #include "absl/base/thread_annotations.h"
@@ -37,6 +38,120 @@ namespace absl {
 ABSL_NAMESPACE_BEGIN
 namespace flags_internal {
 
+template <typename T>
+class Flag;
+
+///////////////////////////////////////////////////////////////////////////////
+// Persistent state of the flag data.
+
+template <typename T>
+class FlagState : public flags_internal::FlagStateInterface {
+ public:
+  FlagState(Flag<T>* flag, T&& cur, bool modified, bool on_command_line,
+            int64_t counter)
+      : flag_(flag),
+        cur_value_(std::move(cur)),
+        modified_(modified),
+        on_command_line_(on_command_line),
+        counter_(counter) {}
+
+  ~FlagState() override = default;
+
+ private:
+  friend class Flag<T>;
+
+  // Restores the flag to the saved state.
+  void Restore() const override;
+
+  // Flag and saved flag data.
+  Flag<T>* flag_;
+  T cur_value_;
+  bool modified_;
+  bool on_command_line_;
+  int64_t counter_;
+};
+
+///////////////////////////////////////////////////////////////////////////////
+// Flag help auxiliary structs.
+
+// This is help argument for absl::Flag encapsulating the string literal pointer
+// or pointer to function generating it as well as enum descriminating two
+// cases.
+using HelpGenFunc = std::string (*)();
+
+union FlagHelpMsg {
+  constexpr explicit FlagHelpMsg(const char* help_msg) : literal(help_msg) {}
+  constexpr explicit FlagHelpMsg(HelpGenFunc help_gen) : gen_func(help_gen) {}
+
+  const char* literal;
+  HelpGenFunc gen_func;
+};
+
+enum class FlagHelpKind : int8_t { kLiteral, kGenFunc };
+
+struct FlagHelpArg {
+  FlagHelpMsg source;
+  FlagHelpKind kind;
+};
+
+extern const char kStrippedFlagHelp[];
+
+// HelpConstexprWrap is used by struct AbslFlagHelpGenFor##name generated by
+// ABSL_FLAG macro. It is only used to silence the compiler in the case where
+// help message expression is not constexpr and does not have type const char*.
+// If help message expression is indeed constexpr const char* HelpConstexprWrap
+// is just a trivial identity function.
+template <typename T>
+const char* HelpConstexprWrap(const T&) {
+  return nullptr;
+}
+constexpr const char* HelpConstexprWrap(const char* p) { return p; }
+constexpr const char* HelpConstexprWrap(char* p) { return p; }
+
+// These two HelpArg overloads allows us to select at compile time one of two
+// way to pass Help argument to absl::Flag. We'll be passing
+// AbslFlagHelpGenFor##name as T and integer 0 as a single argument to prefer
+// first overload if possible. If T::Const is evaluatable on constexpr
+// context (see non template int parameter below) we'll choose first overload.
+// In this case the help message expression is immediately evaluated and is used
+// to construct the absl::Flag. No additionl code is generated by ABSL_FLAG.
+// Otherwise SFINAE kicks in and first overload is dropped from the
+// consideration, in which case the second overload will be used. The second
+// overload does not attempt to evaluate the help message expression
+// immediately and instead delays the evaluation by returing the function
+// pointer (&T::NonConst) genering the help message when necessary. This is
+// evaluatable in constexpr context, but the cost is an extra function being
+// generated in the ABSL_FLAG code.
+template <typename T, int = (T::Const(), 1)>
+constexpr FlagHelpArg HelpArg(int) {
+  return {FlagHelpMsg(T::Const()), FlagHelpKind::kLiteral};
+}
+
+template <typename T>
+constexpr FlagHelpArg HelpArg(char) {
+  return {FlagHelpMsg(&T::NonConst), FlagHelpKind::kGenFunc};
+}
+
+///////////////////////////////////////////////////////////////////////////////
+// Flag default value auxiliary structs.
+
+// Signature for the function generating the initial flag value (usually
+// based on default value supplied in flag's definition)
+using FlagDfltGenFunc = void* (*)();
+
+union FlagDefaultSrc {
+  constexpr explicit FlagDefaultSrc(FlagDfltGenFunc gen_func_arg)
+      : gen_func(gen_func_arg) {}
+
+  void* dynamic_value;
+  FlagDfltGenFunc gen_func;
+};
+
+enum class FlagDefaultSrcKind : int8_t { kDynamicValue, kGenFunc };
+
+///////////////////////////////////////////////////////////////////////////////
+// Flag current value auxiliary structs.
+
 // The minimum atomic size we believe to generate lock free code, i.e. all
 // trivially copyable types not bigger this size generate lock free code.
 static constexpr int kMinLockFreeAtomicSize = 8;
@@ -93,260 +208,9 @@ struct BestAtomicType<
   }
 };
 
-template <typename T>
+struct FlagValue {
-class Flag;
+  // Heap allocated value.
-
+  void* dynamic = nullptr;
-template <typename T>
-class FlagState : public flags_internal::FlagStateInterface {
- public:
-  FlagState(Flag<T>* flag, T&& cur, bool modified, bool on_command_line,
-            int64_t counter)
-      : flag_(flag),
-        cur_value_(std::move(cur)),
-        modified_(modified),
-        on_command_line_(on_command_line),
-        counter_(counter) {}
-
-  ~FlagState() override = default;
-
- private:
-  friend class Flag<T>;
-
-  // Restores the flag to the saved state.
-  void Restore() const override;
-
-  // Flag and saved flag data.
-  Flag<T>* flag_;
-  T cur_value_;
-  bool modified_;
-  bool on_command_line_;
-  int64_t counter_;
-};
-
-// This is help argument for absl::Flag encapsulating the string literal pointer
-// or pointer to function generating it as well as enum descriminating two
-// cases.
-using HelpGenFunc = std::string (*)();
-
-union FlagHelpSrc {
-  constexpr explicit FlagHelpSrc(const char* help_msg) : literal(help_msg) {}
-  constexpr explicit FlagHelpSrc(HelpGenFunc help_gen) : gen_func(help_gen) {}
-
-  const char* literal;
-  HelpGenFunc gen_func;
-};
-
-enum class FlagHelpSrcKind : int8_t { kLiteral, kGenFunc };
-
-struct HelpInitArg {
-  FlagHelpSrc source;
-  FlagHelpSrcKind kind;
-};
-
-extern const char kStrippedFlagHelp[];
-
-// HelpConstexprWrap is used by struct AbslFlagHelpGenFor##name generated by
-// ABSL_FLAG macro. It is only used to silence the compiler in the case where
-// help message expression is not constexpr and does not have type const char*.
-// If help message expression is indeed constexpr const char* HelpConstexprWrap
-// is just a trivial identity function.
-template <typename T>
-const char* HelpConstexprWrap(const T&) {
-  return nullptr;
-}
-constexpr const char* HelpConstexprWrap(const char* p) { return p; }
-constexpr const char* HelpConstexprWrap(char* p) { return p; }
-
-// These two HelpArg overloads allows us to select at compile time one of two
-// way to pass Help argument to absl::Flag. We'll be passing
-// AbslFlagHelpGenFor##name as T and integer 0 as a single argument to prefer
-// first overload if possible. If T::Const is evaluatable on constexpr
-// context (see non template int parameter below) we'll choose first overload.
-// In this case the help message expression is immediately evaluated and is used
-// to construct the absl::Flag. No additionl code is generated by ABSL_FLAG.
-// Otherwise SFINAE kicks in and first overload is dropped from the
-// consideration, in which case the second overload will be used. The second
-// overload does not attempt to evaluate the help message expression
-// immediately and instead delays the evaluation by returing the function
-// pointer (&T::NonConst) genering the help message when necessary. This is
-// evaluatable in constexpr context, but the cost is an extra function being
-// generated in the ABSL_FLAG code.
-template <typename T, int = (T::Const(), 1)>
-constexpr flags_internal::HelpInitArg HelpArg(int) {
-  return {flags_internal::FlagHelpSrc(T::Const()),
-          flags_internal::FlagHelpSrcKind::kLiteral};
-}
-
-template <typename T>
-constexpr flags_internal::HelpInitArg HelpArg(char) {
-  return {flags_internal::FlagHelpSrc(&T::NonConst),
-          flags_internal::FlagHelpSrcKind::kGenFunc};
-}
-
-// Signature for the function generating the initial flag value (usually
-// based on default value supplied in flag's definition)
-using FlagDfltGenFunc = void* (*)();
-
-union FlagDefaultSrc {
-  constexpr explicit FlagDefaultSrc(FlagDfltGenFunc gen_func_arg)
-      : gen_func(gen_func_arg) {}
-
-  void* dynamic_value;
-  FlagDfltGenFunc gen_func;
-};
-
-enum class FlagDefaultSrcKind : int8_t { kDynamicValue, kGenFunc };
-
-// 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 (*)();
-
-struct DynValueDeleter {
-  void operator()(void* ptr) const { Delete(op, ptr); }
-
-  const FlagOpFn op;
-};
-
-// The class encapsulates the Flag's data and safe access to it.
-class FlagImpl {
- public:
-  constexpr FlagImpl(const char* name, const char* filename,
-                     const flags_internal::FlagOpFn op,
-                     const flags_internal::FlagMarshallingOpFn marshalling_op,
-                     const HelpInitArg help,
-                     const flags_internal::FlagDfltGenFunc default_value_gen)
-      : name_(name),
-        filename_(filename),
-        op_(op),
-        marshalling_op_(marshalling_op),
-        help_(help.source),
-        help_source_kind_(help.kind),
-        def_kind_(flags_internal::FlagDefaultSrcKind::kGenFunc),
-        default_src_(default_value_gen),
-        data_guard_{} {}
-
-  // Forces destruction of the Flag's data.
-  void Destroy();
-
-  // Constant access methods
-  absl::string_view Name() const;
-  std::string Filename() const;
-  std::string Help() const;
-  bool IsModified() const ABSL_LOCKS_EXCLUDED(*DataGuard());
-  bool IsSpecifiedOnCommandLine() const ABSL_LOCKS_EXCLUDED(*DataGuard());
-  std::string DefaultValue() const ABSL_LOCKS_EXCLUDED(*DataGuard());
-  std::string CurrentValue() const ABSL_LOCKS_EXCLUDED(*DataGuard());
-  void Read(void* dst, const flags_internal::FlagOpFn dst_op) const
-      ABSL_LOCKS_EXCLUDED(*DataGuard());
-  // Attempts to parse supplied `value` std::string. If parsing is successful, then
-  // it replaces `dst` with the new value.
-  bool TryParse(void** dst, absl::string_view value, std::string* err) const
-      ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard());
-
-#ifndef NDEBUG
-  template <typename T>
-  void Get(T* dst) const {
-    Read(dst, &flags_internal::FlagOps<T>);
-  }
-#else
-  template <typename T, typename std::enable_if<
-                            !flags_internal::IsAtomicFlagTypeTrait<T>::value,
-                            int>::type = 0>
-  void Get(T* dst) const {
-    Read(dst, &flags_internal::FlagOps<T>);
-  }
-  // Overload for `GetFlag()` for types that support lock-free reads.
-  template <typename T,
-            typename std::enable_if<
-                flags_internal::IsAtomicFlagTypeTrait<T>::value, int>::type = 0>
-  void Get(T* dst) const {
-    using U = flags_internal::BestAtomicType<T>;
-    const typename U::type r = atomics_.template load<T>();
-    if (r != U::AtomicInit()) {
-      std::memcpy(static_cast<void*>(dst), &r, sizeof(T));
-    } else {
-      Read(dst, &flags_internal::FlagOps<T>);
-    }
-  }
-#endif
-
-  // Mutating access methods
-  void Write(const void* src, const flags_internal::FlagOpFn src_op)
-      ABSL_LOCKS_EXCLUDED(*DataGuard());
-  bool SetFromString(absl::string_view value, FlagSettingMode set_mode,
-                     ValueSource source, std::string* err)
-      ABSL_LOCKS_EXCLUDED(*DataGuard());
-  // If possible, updates copy of the Flag's value that is stored in an
-  // atomic word.
-  void StoreAtomic() ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard());
-
-  // Interfaces to operate on callbacks.
-  void SetCallback(const flags_internal::FlagCallback mutation_callback)
-      ABSL_LOCKS_EXCLUDED(*DataGuard());
-  void InvokeCallback() const ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard());
-
-  // Interfaces to save/restore mutable flag data
-  template <typename T>
-  std::unique_ptr<flags_internal::FlagStateInterface> SaveState(
-      Flag<T>* flag) const ABSL_LOCKS_EXCLUDED(*DataGuard()) {
-    T&& cur_value = flag->Get();
-    absl::MutexLock l(DataGuard());
-
-    return absl::make_unique<flags_internal::FlagState<T>>(
-        flag, std::move(cur_value), modified_, on_command_line_, counter_);
-  }
-  bool RestoreState(const void* value, bool modified, bool on_command_line,
-                    int64_t counter) ABSL_LOCKS_EXCLUDED(*DataGuard());
-
-  // Value validation interfaces.
-  void CheckDefaultValueParsingRoundtrip() const
-      ABSL_LOCKS_EXCLUDED(*DataGuard());
-  bool ValidateInputValue(absl::string_view value) const
-      ABSL_LOCKS_EXCLUDED(*DataGuard());
-
- private:
-  // Lazy initialization of the Flag's data.
-  void Init();
-  // Ensures that the lazily initialized data is initialized,
-  // and returns pointer to the mutex guarding flags data.
-  absl::Mutex* DataGuard() const ABSL_LOCK_RETURNED((absl::Mutex*)&data_guard_);
-  // Returns heap allocated value of type T initialized with default value.
-  std::unique_ptr<void, DynValueDeleter> MakeInitValue() const
-      ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard());
-
-  // Immutable Flag's data.
-  // Constant configuration for a particular flag.
-  const char* const name_;      // Flags name passed to ABSL_FLAG as second arg.
-  const char* const filename_;  // The file name where ABSL_FLAG resides.
-  const FlagOpFn op_;           // Type-specific handler.
-  const FlagMarshallingOpFn marshalling_op_;  // Marshalling ops handler.
-  const FlagHelpSrc help_;  // Help message literal or function to generate it.
-  // Indicates if help message was supplied as literal or generator func.
-  const FlagHelpSrcKind help_source_kind_;
-
-  // Indicates that the Flag state is initialized.
-  std::atomic<bool> inited_{false};
-  // Mutable Flag state (guarded by data_guard_).
-  // Additional bool to protect against multiple concurrent constructions
-  // of `data_guard_`.
-  bool is_data_guard_inited_ = false;
-  // Has flag value been modified?
-  bool modified_ ABSL_GUARDED_BY(*DataGuard()) = false;
-  // Specified on command line.
-  bool on_command_line_ ABSL_GUARDED_BY(*DataGuard()) = false;
-  // If def_kind_ == kDynamicValue, default_src_ holds a dynamically allocated
-  // value.
-  FlagDefaultSrcKind def_kind_ ABSL_GUARDED_BY(*DataGuard());
-  // Either a pointer to the function generating the default value based on the
-  // value specified in ABSL_FLAG or pointer to the dynamically set default
-  // value via SetCommandLineOptionWithMode. def_kind_ is used to distinguish
-  // these two cases.
-  FlagDefaultSrc default_src_ ABSL_GUARDED_BY(*DataGuard());
-  // Lazily initialized pointer to current value
-  void* cur_ ABSL_GUARDED_BY(*DataGuard()) = nullptr;
-  // Mutation counter
-  int64_t counter_ ABSL_GUARDED_BY(*DataGuard()) = 0;
   // For some types, a copy of the current value is kept in an atomically
   // accessible field.
   union Atomics {
@@ -376,13 +240,181 @@ class FlagImpl {
     constexpr Atomics() : small_atomic{SmallAtomicInit()} {}
 #endif
   };
-  Atomics atomics_{};
+  Atomics atomics{};
+};
+
+///////////////////////////////////////////////////////////////////////////////
+// Flag callback auxiliary structs.
+
+// Signature for the mutation callback used by watched Flags
+// The callback is noexcept.
+// TODO(rogeeff): add noexcept after C++17 support is added.
+using FlagCallbackFunc = void (*)();
+
+struct FlagCallback {
+  FlagCallbackFunc func;
+  absl::Mutex guard;  // Guard for concurrent callback invocations.
+};
+
+///////////////////////////////////////////////////////////////////////////////
+// Flag implementation, which does not depend on flag value type.
+// The class encapsulates the Flag's data and access to it.
+
+struct DynValueDeleter {
+  explicit DynValueDeleter(FlagOpFn op_arg = nullptr) : op(op_arg) {}
+  void operator()(void* ptr) const {
+    if (op != nullptr) Delete(op, ptr);
+  }
+
+  const FlagOpFn op;
+};
+
+class FlagImpl {
+ public:
+  constexpr FlagImpl(const char* name, const char* filename, FlagOpFn op,
+                     FlagMarshallingOpFn marshalling_op, FlagHelpArg help,
+                     FlagDfltGenFunc default_value_gen)
+      : name_(name),
+        filename_(filename),
+        op_(op),
+        marshalling_op_(marshalling_op),
+        help_(help.source),
+        help_source_kind_(help.kind),
+        def_kind_(FlagDefaultSrcKind::kGenFunc),
+        default_src_(default_value_gen),
+        data_guard_{} {}
+
+  // Forces destruction of the Flag's data.
+  void Destroy();
+
+  // Constant access methods
+  absl::string_view Name() const;
+  std::string Filename() const;
+  std::string Help() const;
+  bool IsModified() const ABSL_LOCKS_EXCLUDED(*DataGuard());
+  bool IsSpecifiedOnCommandLine() const ABSL_LOCKS_EXCLUDED(*DataGuard());
+  std::string DefaultValue() const ABSL_LOCKS_EXCLUDED(*DataGuard());
+  std::string CurrentValue() const ABSL_LOCKS_EXCLUDED(*DataGuard());
+  void Read(void* dst, const FlagOpFn dst_op) const
+      ABSL_LOCKS_EXCLUDED(*DataGuard());
+  // Attempts to parse supplied `value` std::string. If parsing is successful, then
+  // it replaces `dst` with the new value.
+  bool TryParse(void** dst, absl::string_view value, std::string* err) const
+      ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard());
+
+#ifndef NDEBUG
+  template <typename T>
+  void Get(T* dst) const {
+    Read(dst, &FlagOps<T>);
+  }
+#else
+  template <typename T, typename std::enable_if<
+                            !IsAtomicFlagTypeTrait<T>::value, int>::type = 0>
+  void Get(T* dst) const {
+    Read(dst, &FlagOps<T>);
+  }
+  // Overload for `GetFlag()` for types that support lock-free reads.
+  template <typename T, typename std::enable_if<IsAtomicFlagTypeTrait<T>::value,
+                                                int>::type = 0>
+  void Get(T* dst) const {
+    using U = BestAtomicType<T>;
+    const typename U::type r = value_.atomics.template load<T>();
+    if (r != U::AtomicInit()) {
+      std::memcpy(static_cast<void*>(dst), &r, sizeof(T));
+    } else {
+      Read(dst, &FlagOps<T>);
+    }
+  }
+#endif
+
+  // Mutating access methods
+  void Write(const void* src, const FlagOpFn src_op)
+      ABSL_LOCKS_EXCLUDED(*DataGuard());
+  bool SetFromString(absl::string_view value, FlagSettingMode set_mode,
+                     ValueSource source, std::string* err)
+      ABSL_LOCKS_EXCLUDED(*DataGuard());
+  // If possible, updates copy of the Flag's value that is stored in an
+  // atomic word.
+  void StoreAtomic() ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard());
+
+  // Interfaces to operate on callbacks.
+  void SetCallback(const FlagCallbackFunc mutation_callback)
+      ABSL_LOCKS_EXCLUDED(*DataGuard());
+  void InvokeCallback() const ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard());
+
+  // Interfaces to save/restore mutable flag data
+  template <typename T>
+  std::unique_ptr<FlagStateInterface> SaveState(Flag<T>* flag) const
+      ABSL_LOCKS_EXCLUDED(*DataGuard()) {
+    T&& cur_value = flag->Get();
+    absl::MutexLock l(DataGuard());
+
+    return absl::make_unique<FlagState<T>>(
+        flag, std::move(cur_value), modified_, on_command_line_, counter_);
+  }
+  bool RestoreState(const void* value, bool modified, bool on_command_line,
+                    int64_t counter) ABSL_LOCKS_EXCLUDED(*DataGuard());
+
+  // Value validation interfaces.
+  void CheckDefaultValueParsingRoundtrip() const
+      ABSL_LOCKS_EXCLUDED(*DataGuard());
+  bool ValidateInputValue(absl::string_view value) const
+      ABSL_LOCKS_EXCLUDED(*DataGuard());
+
+ private:
+  // Ensures that `data_guard_` is initialized and returns it.
+  absl::Mutex* DataGuard() const ABSL_LOCK_RETURNED((absl::Mutex*)&data_guard_);
+  // Returns heap allocated value of type T initialized with default value.
+  std::unique_ptr<void, DynValueDeleter> MakeInitValue() const
+      ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard());
+  // Lazy initialization of the Flag's data.
+  void Init();
+
+  // Immutable flag's state.
+
+  // Flags name passed to ABSL_FLAG as second arg.
+  const char* const name_;
+  // The file name where ABSL_FLAG resides.
+  const char* const filename_;
+  // Type-specific handler.
+  const FlagOpFn op_;
+  // Marshalling ops handler.
+  const FlagMarshallingOpFn marshalling_op_;
+  // Help message literal or function to generate it.
+  const FlagHelpMsg help_;
+  // Indicates if help message was supplied as literal or generator func.
+  const FlagHelpKind help_source_kind_;
+
+  // Indicates that the Flag state is initialized.
+  std::atomic<bool> inited_{false};
+
+  // Mutable flag's state (guarded by `data_guard_`).
+
+  // Protects against multiple concurrent constructions of `data_guard_`.
+  bool is_data_guard_inited_ = false;
+  // Has this flag's value been modified?
+  bool modified_ ABSL_GUARDED_BY(*DataGuard()) = false;
+  // Has this flag been specified on command line.
+  bool on_command_line_ ABSL_GUARDED_BY(*DataGuard()) = false;
+
+  // Mutation counter
+  int64_t counter_ ABSL_GUARDED_BY(*DataGuard()) = 0;
+
+  // Optional flag's callback and absl::Mutex to guard the invocations.
+  FlagCallback* callback_ ABSL_GUARDED_BY(*DataGuard()) = nullptr;
+
+  // If def_kind_ == kDynamicValue, default_src_ holds a dynamically allocated
+  // value.
+  FlagDefaultSrcKind def_kind_ ABSL_GUARDED_BY(*DataGuard());
+  // Either a pointer to the function generating the default value based on the
+  // value specified in ABSL_FLAG or pointer to the dynamically set default
+  // value via SetCommandLineOptionWithMode. def_kind_ is used to distinguish
+  // these two cases.
+  FlagDefaultSrc default_src_ ABSL_GUARDED_BY(*DataGuard());
+
+  // Current Flag Value
+  FlagValue value_;
 
-  struct CallbackData {
-    FlagCallback func;
-    absl::Mutex guard;  // Guard for concurrent callback invocations.
-  };
-  CallbackData* callback_data_ ABSL_GUARDED_BY(*DataGuard()) = nullptr;
   // This is reserved space for an absl::Mutex to guard flag data. It will be
   // initialized in FlagImpl::Init via placement new.
   // We can't use "absl::Mutex data_guard_", since this class is not literal.
@@ -393,15 +425,18 @@ class FlagImpl {
   alignas(absl::Mutex) mutable char data_guard_[sizeof(absl::Mutex)];
 };
 
-// This is "unspecified" implementation of absl::Flag<T> type.
+///////////////////////////////////////////////////////////////////////////////
+// The "unspecified" implementation of Flag object parameterized by the
+// flag's value type.
+
 template <typename T>
 class Flag final : public flags_internal::CommandLineFlag {
  public:
   constexpr Flag(const char* name, const char* filename,
-                 const flags_internal::FlagMarshallingOpFn marshalling_op,
+                 const FlagMarshallingOpFn marshalling_op,
-                 const flags_internal::HelpInitArg help,
+                 const FlagHelpArg help,
-                 const flags_internal::FlagDfltGenFunc default_value_gen)
+                 const FlagDfltGenFunc default_value_gen)
-      : impl_(name, filename, &flags_internal::FlagOps<T>, marshalling_op, help,
+      : impl_(name, filename, &FlagOps<T>, marshalling_op, help,
               default_value_gen) {}
 
   T Get() const {
@@ -417,9 +452,9 @@ class Flag final : public flags_internal::CommandLineFlag {
     return std::move(u.value);
   }
 
-  void Set(const T& v) { impl_.Write(&v, &flags_internal::FlagOps<T>); }
+  void Set(const T& v) { impl_.Write(&v, &FlagOps<T>); }
 
-  void SetCallback(const flags_internal::FlagCallback mutation_callback) {
+  void SetCallback(const FlagCallbackFunc mutation_callback) {
     impl_.SetCallback(mutation_callback);
   }
 
@@ -434,7 +469,6 @@ class Flag final : public flags_internal::CommandLineFlag {
   }
   std::string DefaultValue() const override { return impl_.DefaultValue(); }
   std::string CurrentValue() const override { return impl_.CurrentValue(); }
-
   bool ValidateInputValue(absl::string_view value) const override {
     return impl_.ValidateInputValue(value);
   }
@@ -442,24 +476,20 @@ class Flag final : public flags_internal::CommandLineFlag {
   // Interfaces to save and restore flags to/from persistent state.
   // Returns current flag state or nullptr if flag does not support
   // saving and restoring a state.
-  std::unique_ptr<flags_internal::FlagStateInterface> SaveState() override {
+  std::unique_ptr<FlagStateInterface> SaveState() override {
     return impl_.SaveState(this);
   }
 
   // Restores the flag state to the supplied state object. If there is
   // nothing to restore returns false. Otherwise returns true.
-  bool RestoreState(const flags_internal::FlagState<T>& flag_state) {
+  bool RestoreState(const FlagState<T>& flag_state) {
     return impl_.RestoreState(&flag_state.cur_value_, flag_state.modified_,
                               flag_state.on_command_line_, flag_state.counter_);
   }
-
+  bool SetFromString(absl::string_view value, FlagSettingMode set_mode,
-  bool SetFromString(absl::string_view value,
+                     ValueSource source, std::string* error) override {
-                     flags_internal::FlagSettingMode set_mode,
-                     flags_internal::ValueSource source,
-                     std::string* error) override {
     return impl_.SetFromString(value, set_mode, source, error);
   }
-
   void CheckDefaultValueParsingRoundtrip() const override {
     impl_.CheckDefaultValueParsingRoundtrip();
   }
@@ -469,14 +499,10 @@ class Flag final : public flags_internal::CommandLineFlag {
 
   void Destroy() override { impl_.Destroy(); }
 
-  void Read(void* dst) const override {
+  void Read(void* dst) const override { impl_.Read(dst, &FlagOps<T>); }
-    impl_.Read(dst, &flags_internal::FlagOps<T>);
+  FlagOpFn TypeId() const override { return &FlagOps<T>; }
-  }
-  flags_internal::FlagOpFn TypeId() const override {
-    return &flags_internal::FlagOps<T>;
-  }
 
-  // Flag's data
+  // Flag's implementation with value type abstracted out.
   FlagImpl impl_;
 };
 
@@ -499,7 +525,7 @@ class FlagRegistrar {
     if (do_register) flags_internal::RegisterCommandLineFlag(flag_);
   }
 
-  FlagRegistrar& OnUpdate(flags_internal::FlagCallback cb) && {
+  FlagRegistrar& OnUpdate(FlagCallbackFunc cb) && {
     flag_->SetCallback(cb);
     return *this;
   }