From b69c7d880caddfc25bf348dbcfe9d45fdd8bc6e6 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Fri, 21 Feb 2020 12:51:36 -0800 Subject: Export of internal Abseil changes -- 00f5301405423005d9129935c05f20155536cc1a by CJ Johnson : Removes usage of std::aligned_storage from Abseil implementation details PiperOrigin-RevId: 296492301 -- fc11d15f91764612fba080669d2381dc181df52b by Abseil Team : Fix absl::bind_front documentation. PiperOrigin-RevId: 296482945 -- 0164c595c129c46bf21ae74eba5399a1da5f140b by Gennadiy Rozental : Automated g4 rollback of changelist 296320700. PiperOrigin-RevId: 296439968 -- 1eb295700758ca0894d872b2de7c675b4ad679af by Abseil Team : Removes duplicate comments. PiperOrigin-RevId: 296433214 -- c30c01caae02d2fa4ef783d988de6bebb9757c39 by Derek Mauro : Merge GitHub #621: Add RISCV support to GetProgramCounter() Fixes #621 PiperOrigin-RevId: 296351174 -- 95d4498167596fd7543e025bdfe9a8da9e2ca3c8 by Abseil Team : Automated g4 rollback of changelist 296320700. PiperOrigin-RevId: 296348701 -- b193f0543e0cec54dddb2ed51f45dc489c8d06d5 by Gennadiy Rozental : Change TryParse interface to return managed value. In addition introduce companion StoreValue routine which consumes pointer to source value and stores the value inside of FlagImpl. In a follow up CL we will change StoreValue implementation to behave differently depending on "value storage kind". We also rename default_src_ to default_value_. PiperOrigin-RevId: 296320700 -- 57e942b485d12912a0a8d0d0b35fa2a62847020f by Derek Mauro : Merge GitHub #622 * Add missing #ifdef conditionals for ABSL_HAVE_VDSO_SUPPORT PiperOrigin-RevId: 296272830 GitOrigin-RevId: 00f5301405423005d9129935c05f20155536cc1a Change-Id: I1b05eeaf1280f95fb0a2c5f3654995a87c792893 --- absl/flags/internal/flag.cc | 81 +++++++++++++++------------------ absl/flags/internal/flag.h | 25 +++++----- absl/flags/internal/type_erased_test.cc | 7 +++ 3 files changed, 58 insertions(+), 55 deletions(-) (limited to 'absl/flags') diff --git a/absl/flags/internal/flag.cc b/absl/flags/internal/flag.cc index 83ec8df..5a921e2 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 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 FlagImpl::TryParse( + absl::string_view value, std::string* err) const { + std::unique_ptr 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 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(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 ec67348..c1beda8 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 ::value, int>::type = 0> @@ -466,8 +462,15 @@ class FlagImpl { // Returns heap allocated value of type T initialized with default value. std::unique_ptr 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 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(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 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 033e00e..4ce5981 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"); -- cgit v1.2.3