diff options
Diffstat (limited to 'absl/flags')
-rw-r--r-- | absl/flags/BUILD.bazel | 2 | ||||
-rw-r--r-- | absl/flags/CMakeLists.txt | 2 | ||||
-rw-r--r-- | absl/flags/declare.h | 5 | ||||
-rw-r--r-- | absl/flags/flag.cc | 16 | ||||
-rw-r--r-- | absl/flags/flag.h | 106 | ||||
-rw-r--r-- | absl/flags/internal/commandlineflag.cc | 93 | ||||
-rw-r--r-- | absl/flags/internal/commandlineflag.h | 64 | ||||
-rw-r--r-- | absl/flags/internal/flag.cc | 51 | ||||
-rw-r--r-- | absl/flags/internal/flag.h | 49 | ||||
-rw-r--r-- | absl/flags/internal/registry.cc | 62 | ||||
-rw-r--r-- | absl/flags/internal/registry.h | 21 | ||||
-rw-r--r-- | absl/flags/internal/usage_test.cc | 2 |
12 files changed, 294 insertions, 179 deletions
diff --git a/absl/flags/BUILD.bazel b/absl/flags/BUILD.bazel index c338dbee..e7742de6 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 9860979c..d9084a9c 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 556ec5e9..0a113a2b 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 f16cc75e..a02d069e 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 36c771cf..881f8502 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 98a46695..2063cda6 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 d82443a7..23512576 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 00000000..34629b30 --- /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 bc50de54..fad36877 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 4e5d1106..0d4eec98 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 27566fbf..884a8db5 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 781e1d2b..8538b2b6 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); |