about summary refs log tree commit diff
path: root/absl/flags
diff options
context:
space:
mode:
Diffstat (limited to 'absl/flags')
-rw-r--r--absl/flags/internal/flag.cc81
-rw-r--r--absl/flags/internal/flag.h25
-rw-r--r--absl/flags/internal/type_erased_test.cc7
3 files changed, 58 insertions, 55 deletions
diff --git a/absl/flags/internal/flag.cc b/absl/flags/internal/flag.cc
index 83ec8df19149..5a921e28d74e 100644
--- a/absl/flags/internal/flag.cc
+++ b/absl/flags/internal/flag.cc
@@ -121,13 +121,21 @@ void FlagImpl::AssertValidType(FlagStaticTypeId type_id) const {
 std::unique_ptr<void, DynValueDeleter> FlagImpl::MakeInitValue() const {
   void* res = nullptr;
   if (DefaultKind() == FlagDefaultKind::kDynamicValue) {
-    res = flags_internal::Clone(op_, default_src_.dynamic_value);
+    res = flags_internal::Clone(op_, default_value_.dynamic_value);
   } else {
-    res = (*default_src_.gen_func)();
+    res = (*default_value_.gen_func)();
   }
   return {res, DynValueDeleter{op_}};
 }
 
+void FlagImpl::StoreValue(const void* src) {
+  flags_internal::Copy(op_, src, value_.dynamic);
+  StoreAtomic();
+  modified_ = true;
+  ++counter_;
+  InvokeCallback();
+}
+
 absl::string_view FlagImpl::Name() const { return name_; }
 
 std::string FlagImpl::Filename() const {
@@ -220,23 +228,19 @@ bool FlagImpl::RestoreState(const void* value, bool modified,
 // argument. If parsing successful, this function replaces the dst with newly
 // parsed value. In case if any error is encountered in either step, the error
 // message is stored in 'err'
-bool FlagImpl::TryParse(void** dst, absl::string_view value,
-                        std::string* err) const {
-  auto tentative_value = MakeInitValue();
+std::unique_ptr<void, DynValueDeleter> FlagImpl::TryParse(
+    absl::string_view value, std::string* err) const {
+  std::unique_ptr<void, DynValueDeleter> tentative_value = MakeInitValue();
 
   std::string parse_err;
   if (!flags_internal::Parse(op_, value, tentative_value.get(), &parse_err)) {
     absl::string_view err_sep = parse_err.empty() ? "" : "; ";
     *err = absl::StrCat("Illegal value '", value, "' specified for flag '",
                         Name(), "'", err_sep, parse_err);
-    return false;
+    return nullptr;
   }
 
-  void* old_val = *dst;
-  *dst = tentative_value.release();
-  tentative_value.reset(old_val);
-
-  return true;
+  return tentative_value;
 }
 
 void FlagImpl::Read(void* dst) const {
@@ -266,22 +270,17 @@ void FlagImpl::Write(const void* src) {
   absl::MutexLock l(DataGuard());
 
   if (ShouldValidateFlagValue(flags_internal::StaticTypeId(op_))) {
-    void* obj = flags_internal::Clone(op_, src);
+    std::unique_ptr<void, DynValueDeleter> obj{flags_internal::Clone(op_, src),
+                                               DynValueDeleter{op_}};
     std::string ignored_error;
     std::string src_as_str = flags_internal::Unparse(op_, src);
-    if (!flags_internal::Parse(op_, src_as_str, obj, &ignored_error)) {
+    if (!flags_internal::Parse(op_, src_as_str, obj.get(), &ignored_error)) {
       ABSL_INTERNAL_LOG(ERROR, absl::StrCat("Attempt to set flag '", Name(),
                                             "' to invalid value ", src_as_str));
     }
-    flags_internal::Delete(op_, obj);
   }
 
-  modified_ = true;
-  counter_++;
-  flags_internal::Copy(op_, src, value_.dynamic);
-
-  StoreAtomic();
-  InvokeCallback();
+  StoreValue(src);
 }
 
 // Sets the value of the flag based on specified string `value`. If the flag
@@ -299,11 +298,10 @@ 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(&value_.dynamic, value, err)) return false;
-      modified_ = true;
-      counter_++;
-      StoreAtomic();
-      InvokeCallback();
+      auto tentative_value = TryParse(value, err);
+      if (!tentative_value) return false;
+
+      StoreValue(tentative_value.get());
 
       if (source == kCommandLine) {
         on_command_line_ = true;
@@ -312,13 +310,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(&value_.dynamic, value, err)) return false;
-        modified_ = true;
-        counter_++;
-        StoreAtomic();
-        InvokeCallback();
-      } else {
+      if (modified_) {
         // TODO(rogeeff): review and fix this semantic. Currently we do not fail
         // in this case if flag is modified. This is misleading since the flag's
         // value is not updated even though we return true.
@@ -327,28 +319,29 @@ bool FlagImpl::SetFromString(absl::string_view value, FlagSettingMode set_mode,
         // return false;
         return true;
       }
+      auto tentative_value = TryParse(value, err);
+      if (!tentative_value) return false;
+
+      StoreValue(tentative_value.get());
       break;
     }
     case SET_FLAGS_DEFAULT: {
+      auto tentative_value = TryParse(value, err);
+      if (!tentative_value) return false;
+
       if (DefaultKind() == FlagDefaultKind::kDynamicValue) {
-        if (!TryParse(&default_src_.dynamic_value, value, err)) {
-          return false;
-        }
+        void* old_value = default_value_.dynamic_value;
+        default_value_.dynamic_value = tentative_value.release();
+        tentative_value.reset(old_value);
       } else {
-        void* new_default_val = nullptr;
-        if (!TryParse(&new_default_val, value, err)) {
-          return false;
-        }
-
-        default_src_.dynamic_value = new_default_val;
+        default_value_.dynamic_value = tentative_value.release();
         def_kind_ = static_cast<uint8_t>(FlagDefaultKind::kDynamicValue);
       }
 
       if (!modified_) {
         // Need to set both default value *and* current, in this case
-        flags_internal::Copy(op_, default_src_.dynamic_value, value_.dynamic);
-        StoreAtomic();
-        InvokeCallback();
+        StoreValue(default_value_.dynamic_value);
+        modified_ = false;
       }
       break;
     }
diff --git a/absl/flags/internal/flag.h b/absl/flags/internal/flag.h
index ec6734870c7e..c1beda8ebc00 100644
--- a/absl/flags/internal/flag.h
+++ b/absl/flags/internal/flag.h
@@ -363,7 +363,7 @@ struct DynValueDeleter {
     if (op != nullptr) Delete(op, ptr);
   }
 
-  const FlagOpFn op;
+  FlagOpFn op;
 };
 
 class FlagImpl {
@@ -380,7 +380,7 @@ class FlagImpl {
         on_command_line_(false),
         counter_(0),
         callback_(nullptr),
-        default_src_(default_value_gen),
+        default_value_(default_value_gen),
         data_guard_{} {}
 
   // Constant access methods
@@ -392,10 +392,6 @@ class FlagImpl {
   std::string DefaultValue() const ABSL_LOCKS_EXCLUDED(*DataGuard());
   std::string CurrentValue() const ABSL_LOCKS_EXCLUDED(*DataGuard());
   void Read(void* dst) 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());
 
   template <typename T, typename std::enable_if<
                             !IsAtomicFlagTypeTrait<T>::value, int>::type = 0>
@@ -466,8 +462,15 @@ class FlagImpl {
   // 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.
+  // Flag initialization called via absl::call_once.
   void Init();
+  // Attempts to parse supplied `value` std::string. If parsing is successful,
+  // returns new value. Otherwsie returns nullptr.
+  std::unique_ptr<void, DynValueDeleter> TryParse(absl::string_view value,
+                                                  std::string* err) const
+      ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard());
+  // Stores the flag value based on the pointer to the source.
+  void StoreValue(const void* src) ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard());
 
   FlagHelpKind HelpSourceKind() const {
     return static_cast<FlagHelpKind>(help_source_kind_);
@@ -511,7 +514,7 @@ class FlagImpl {
 
   // Mutable flag's state (guarded by `data_guard_`).
 
-  // If def_kind_ == kDynamicValue, default_src_ holds a dynamically allocated
+  // If def_kind_ == kDynamicValue, default_value_ holds a dynamically allocated
   // value.
   uint8_t def_kind_ : 1 ABSL_GUARDED_BY(*DataGuard());
   // Has this flag's value been modified?
@@ -527,7 +530,7 @@ class FlagImpl {
   // 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());
+  FlagDefaultSrc default_value_ ABSL_GUARDED_BY(*DataGuard());
   // Current Flag Value
   FlagValue value_;
 
@@ -542,8 +545,8 @@ class FlagImpl {
 };
 
 ///////////////////////////////////////////////////////////////////////////////
-// The "unspecified" implementation of Flag object parameterized by the
-// flag's value type.
+// The Flag object parameterized by the flag's value type. This class implements
+// flag reflection handle interface.
 
 template <typename T>
 class Flag final : public flags_internal::CommandLineFlag {
diff --git a/absl/flags/internal/type_erased_test.cc b/absl/flags/internal/type_erased_test.cc
index 033e00e42e8a..4ce5981047b6 100644
--- a/absl/flags/internal/type_erased_test.cc
+++ b/absl/flags/internal/type_erased_test.cc
@@ -119,6 +119,13 @@ TEST_F(TypeErasedTest, TestSetCommandLineOptionWithMode_SET_FLAGS_DEFAULT) {
   EXPECT_TRUE(flags::SetCommandLineOptionWithMode("int_flag", "101",
                                                   flags::SET_FLAGS_DEFAULT));
 
+  // Set it again to ensure that resetting logic is covered.
+  EXPECT_TRUE(flags::SetCommandLineOptionWithMode("int_flag", "102",
+                                                  flags::SET_FLAGS_DEFAULT));
+
+  EXPECT_TRUE(flags::SetCommandLineOptionWithMode("int_flag", "103",
+                                                  flags::SET_FLAGS_DEFAULT));
+
   EXPECT_TRUE(flags::SetCommandLineOptionWithMode("string_flag", "asdfgh",
                                                   flags::SET_FLAGS_DEFAULT));
   EXPECT_EQ(absl::GetFlag(FLAGS_string_flag), "asdfgh");
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-12-24T02ยท27+0000