diff options
author | Abseil Team <absl-team@google.com> | 2019-08-23T18·38-0700 |
---|---|---|
committer | Xiaoyi Zhang <zhangxy@google.com> | 2019-08-23T18·48-0400 |
commit | 2d2d7fbc283315b676159716376e739d3d23ed94 (patch) | |
tree | 6530edfdc7591c4f89d0aa48442cc6d452c47735 /absl | |
parent | 0302d1e5fa4fcdd1763b7d1bb3212943b1ae911d (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.h | 3 | ||||
-rw-r--r-- | absl/debugging/internal/stacktrace_x86-inl.inc | 8 | ||||
-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 | ||||
-rw-r--r-- | absl/random/distributions.h | 19 | ||||
-rw-r--r-- | absl/random/distributions_test.cc | 19 | ||||
-rw-r--r-- | absl/random/internal/uniform_helper.h | 57 |
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)()); |