From ca9856cabc23d771bcce634677650eb6fc4363ae Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Thu, 30 Apr 2020 17:44:10 -0700 Subject: [PATCH] Export of internal Abseil changes -- 53550735f5a943dfb99225e7c53f211c2d6e7951 by Gennadiy Rozental : Import of CCTZ from GitHub. PiperOrigin-RevId: 309333648 -- 847bbf8a1d9cd322ec058c6f932d1f687fd3d331 by Gennadiy Rozental : Make Validation interfaces private in CommandLineFlag. Calls are rewired via private interface access struct. PiperOrigin-RevId: 309323013 -- a600fc5051e0a0af50a7850450fd3ed1aef3f316 by Matthew Brown : Internal Change. PiperOrigin-RevId: 309292207 -- 937d00ce3cf62c5f23f59b5377471fd01d6bfbc7 by Gennadiy Rozental : Make TypeId interface private in CommandLineFlag. We also rewire the SaveState via the new PrivateHandleInterface trampoline class. This class will be the only way to access private methods of class CommandLineFlag. PiperOrigin-RevId: 309282547 -- 796c4bd35073b6a8337762bdb13603dae12a4df1 by Derek Mauro : Cleanup uses of kLinkerInitialized PiperOrigin-RevId: 309274734 -- c831446c52d9ef4bdcb1ea369840904620abc4b9 by Gennadiy Rozental : Eliminate the interface IsModified of CommndLineFlag. PiperOrigin-RevId: 309256248 -- a1db59d7f7aa39cb0a37dbf80f8c04e371da8465 by Gennadiy Rozental : Avoid default value generator if default value expression is constexpr. If possible, we detect constexpr-ness of default value expression and avoid storing default value generator in side of flag and instead set the flag's value to the value of that expression at const initialization time of flag objects. At the moment we only do this for flags of (all) integral, float and double value types PiperOrigin-RevId: 309110630 -- ae3b4a139aacd8fc165c9acd2a3cbae1f9e26af4 by Gennadiy Rozental : Make SaveState a private method of the CommandLineFlag and make it only accessible from FlagSaverImpl. There is no other call sites for this call. PiperOrigin-RevId: 309073989 -- cbc24b4dcc166dd6b0208e9d7620484eaaaa7ee0 by Abseil Team : Eliminate the interface IsModified of CommndLineFlag. PiperOrigin-RevId: 309064639 -- 08e79645a89d71785c5381cea9c413357db9824a by Gennadiy Rozental : Eliminate the interface IsModified of CommndLineFlag. PiperOrigin-RevId: 309054430 -- 4a6c70233c60dc8c39b7fa9beb5fa687c215261f by Gennadiy Rozental : Internal change PiperOrigin-RevId: 308900784 -- 13160efdf7710f142778d5a1e4c85aa309f019b6 by Abseil Team : Provide definitions of static member variables -- improved C++11 support. PiperOrigin-RevId: 308900290 -- 0343b8228657b9b313afdfe88c4a7b2137d56db4 by Gennadiy Rozental : Rename method Get to TryGet per approved spec before making interface public. PiperOrigin-RevId: 308889113 -- 7b84e27fb857fc1296a05504970f506d47d2f2c1 by Derek Mauro : Remove node_hash_* methods that were deprecated on release PiperOrigin-RevId: 308837933 -- 599d44ee72c02b6bb6e1c1a1db72873841441416 by Gennadiy Rozental : Eliminate CommandLineFlag::Typename interface per approved spec before making CommandLineFlag public. PiperOrigin-RevId: 308814376 GitOrigin-RevId: 53550735f5a943dfb99225e7c53f211c2d6e7951 Change-Id: Iae52c65b7322152c7e58f222d60eb5a21699a2cb --- absl/base/internal/spinlock.cc | 7 + absl/base/internal/spinlock.h | 5 + absl/base/internal/sysinfo.cc | 11 +- absl/base/internal/thread_identity_test.cc | 9 +- absl/base/spinlock_test_common.cc | 14 +- absl/container/node_hash_map.h | 6 - absl/container/node_hash_set.h | 6 - absl/flags/flag.h | 61 ++-- absl/flags/flag_test.cc | 65 ++-- absl/flags/internal/commandlineflag.cc | 15 +- absl/flags/internal/commandlineflag.h | 46 ++- absl/flags/internal/commandlineflag_test.cc | 3 - absl/flags/internal/flag.cc | 61 +++- absl/flags/internal/flag.h | 99 +++++-- absl/flags/internal/registry.cc | 12 +- absl/flags/internal/type_erased.cc | 4 +- absl/flags/internal/type_erased.h | 2 +- absl/flags/internal/usage.cc | 32 +- absl/strings/internal/str_format/arg.cc | 28 +- absl/strings/internal/str_format/arg.h | 85 +++--- absl/strings/internal/str_format/bind.h | 5 +- absl/strings/internal/str_format/checker.h | 24 +- absl/strings/internal/str_format/extension.h | 277 +++++++++++++----- .../internal/str_format/extension_test.cc | 9 - .../internal/str_format/float_conversion.cc | 110 ++++--- absl/strings/internal/str_format/parser.cc | 10 +- absl/strings/internal/str_format/parser.h | 12 +- .../internal/str_format/parser_test.cc | 27 +- absl/strings/str_format_test.cc | 2 +- .../internal/create_thread_identity.cc | 6 +- absl/synchronization/internal/graphcycles.cc | 6 +- absl/synchronization/mutex.cc | 17 +- absl/time/clock.cc | 6 +- absl/time/internal/cctz/src/cctz_benchmark.cc | 1 + .../cctz/src/time_zone_lookup_test.cc | 1 + 35 files changed, 645 insertions(+), 439 deletions(-) diff --git a/absl/base/internal/spinlock.cc b/absl/base/internal/spinlock.cc index fd0c733e2..7cac72f97 100644 --- a/absl/base/internal/spinlock.cc +++ b/absl/base/internal/spinlock.cc @@ -66,6 +66,13 @@ void RegisterSpinLockProfiler(void (*fn)(const void *contendedlock, submit_profile_data.Store(fn); } +// Static member variable definitions. +constexpr uint32_t SpinLock::kSpinLockHeld; +constexpr uint32_t SpinLock::kSpinLockCooperative; +constexpr uint32_t SpinLock::kSpinLockDisabledScheduling; +constexpr uint32_t SpinLock::kSpinLockSleeper; +constexpr uint32_t SpinLock::kWaitTimeMask; + // Uncommon constructors. SpinLock::SpinLock(base_internal::SchedulingMode mode) : lockword_(IsCooperative(mode) ? kSpinLockCooperative : 0) { diff --git a/absl/base/internal/spinlock.h b/absl/base/internal/spinlock.h index 89e93aad0..2b08a2d6e 100644 --- a/absl/base/internal/spinlock.h +++ b/absl/base/internal/spinlock.h @@ -36,6 +36,7 @@ #include #include "absl/base/attributes.h" +#include "absl/base/const_init.h" #include "absl/base/dynamic_annotations.h" #include "absl/base/internal/low_level_scheduling.h" #include "absl/base/internal/raw_logging.h" @@ -77,6 +78,10 @@ class ABSL_LOCKABLE SpinLock { SpinLock(base_internal::LinkerInitialized, base_internal::SchedulingMode mode); + // Constructor for global SpinLock instances. See absl/base/const_init.h. + constexpr SpinLock(absl::ConstInitType, base_internal::SchedulingMode mode) + : lockword_(IsCooperative(mode) ? kSpinLockCooperative : 0) {} + ~SpinLock() { ABSL_TSAN_MUTEX_DESTROY(this, __tsan_mutex_not_static); } // Acquire this SpinLock. diff --git a/absl/base/internal/sysinfo.cc b/absl/base/internal/sysinfo.cc index c3f275ca8..6c69683fa 100644 --- a/absl/base/internal/sysinfo.cc +++ b/absl/base/internal/sysinfo.cc @@ -344,15 +344,16 @@ pid_t GetTID() { #else // Fallback implementation of GetTID using pthread_getspecific. -static once_flag tid_once; -static pthread_key_t tid_key; -static absl::base_internal::SpinLock tid_lock( - absl::base_internal::kLinkerInitialized); +ABSL_CONST_INIT static once_flag tid_once; +ABSL_CONST_INIT static pthread_key_t tid_key; +ABSL_CONST_INIT static absl::base_internal::SpinLock tid_lock( + absl::kConstInit, base_internal::SCHEDULE_KERNEL_ONLY); // We set a bit per thread in this array to indicate that an ID is in // use. ID 0 is unused because it is the default value returned by // pthread_getspecific(). -static std::vector* tid_array ABSL_GUARDED_BY(tid_lock) = nullptr; +ABSL_CONST_INIT static std::vector *tid_array + ABSL_GUARDED_BY(tid_lock) = nullptr; static constexpr int kBitsPerWord = 32; // tid_array is uint32_t. // Returns the TID to tid_array. diff --git a/absl/base/internal/thread_identity_test.cc b/absl/base/internal/thread_identity_test.cc index 3685779ce..624d5b96b 100644 --- a/absl/base/internal/thread_identity_test.cc +++ b/absl/base/internal/thread_identity_test.cc @@ -21,6 +21,7 @@ #include "absl/base/attributes.h" #include "absl/base/internal/spinlock.h" #include "absl/base/macros.h" +#include "absl/base/thread_annotations.h" #include "absl/synchronization/internal/per_thread_sem.h" #include "absl/synchronization/mutex.h" @@ -29,10 +30,9 @@ ABSL_NAMESPACE_BEGIN namespace base_internal { namespace { -// protects num_identities_reused -static absl::base_internal::SpinLock map_lock( - absl::base_internal::kLinkerInitialized); -static int num_identities_reused; +ABSL_CONST_INIT static absl::base_internal::SpinLock map_lock( + absl::kConstInit, base_internal::SCHEDULE_KERNEL_ONLY); +ABSL_CONST_INIT static int num_identities_reused ABSL_GUARDED_BY(map_lock); static const void* const kCheckNoIdentity = reinterpret_cast(1); @@ -90,6 +90,7 @@ TEST(ThreadIdentityTest, BasicIdentityWorksThreaded) { // We should have recycled ThreadIdentity objects above; while (external) // library threads allocating their own identities may preclude some // reuse, we should have sufficient repetitions to exclude this. + absl::base_internal::SpinLockHolder l(&map_lock); EXPECT_LT(kNumThreads, num_identities_reused); } diff --git a/absl/base/spinlock_test_common.cc b/absl/base/spinlock_test_common.cc index 08f61ba86..b68c51a1d 100644 --- a/absl/base/spinlock_test_common.cc +++ b/absl/base/spinlock_test_common.cc @@ -56,12 +56,10 @@ namespace { static constexpr int kArrayLength = 10; static uint32_t values[kArrayLength]; -static SpinLock static_spinlock(base_internal::kLinkerInitialized); -static SpinLock static_cooperative_spinlock( - base_internal::kLinkerInitialized, - base_internal::SCHEDULE_COOPERATIVE_AND_KERNEL); -static SpinLock static_noncooperative_spinlock( - base_internal::kLinkerInitialized, base_internal::SCHEDULE_KERNEL_ONLY); +ABSL_CONST_INIT static SpinLock static_cooperative_spinlock( + absl::kConstInit, base_internal::SCHEDULE_COOPERATIVE_AND_KERNEL); +ABSL_CONST_INIT static SpinLock static_noncooperative_spinlock( + absl::kConstInit, base_internal::SCHEDULE_KERNEL_ONLY); // Simple integer hash function based on the public domain lookup2 hash. // http://burtleburtle.net/bob/c/lookup2.c @@ -191,10 +189,6 @@ TEST(SpinLock, WaitCyclesEncoding) { EXPECT_GT(expected_max_value_decoded, before_max_value_decoded); } -TEST(SpinLockWithThreads, StaticSpinLock) { - ThreadedTest(&static_spinlock); -} - TEST(SpinLockWithThreads, StackSpinLock) { SpinLock spinlock; ThreadedTest(&spinlock); diff --git a/absl/container/node_hash_map.h b/absl/container/node_hash_map.h index fccea1841..174b971e9 100644 --- a/absl/container/node_hash_map.h +++ b/absl/container/node_hash_map.h @@ -514,12 +514,6 @@ class node_hash_map // // Returns the function used for comparing keys equality. using Base::key_eq; - - ABSL_DEPRECATED("Call `hash_function()` instead.") - typename Base::hasher hash_funct() { return this->hash_function(); } - - ABSL_DEPRECATED("Call `rehash()` instead.") - void resize(typename Base::size_type hint) { this->rehash(hint); } }; // erase_if(node_hash_map<>, Pred) diff --git a/absl/container/node_hash_set.h b/absl/container/node_hash_set.h index ad54b6dcc..56bab5c2c 100644 --- a/absl/container/node_hash_set.h +++ b/absl/container/node_hash_set.h @@ -427,12 +427,6 @@ class node_hash_set // // Returns the function used for comparing keys equality. using Base::key_eq; - - ABSL_DEPRECATED("Call `hash_function()` instead.") - typename Base::hasher hash_funct() { return this->hash_function(); } - - ABSL_DEPRECATED("Call `rehash()` instead.") - void resize(typename Base::size_type hint) { this->rehash(hint); } }; // erase_if(node_hash_set<>, Pred) diff --git a/absl/flags/flag.h b/absl/flags/flag.h index 194a9d314..8dd1b9be7 100644 --- a/absl/flags/flag.h +++ b/absl/flags/flag.h @@ -118,11 +118,12 @@ class Flag { return impl_; } - impl_ = - new flags_internal::Flag(name_, filename_, - {flags_internal::FlagHelpMsg(help_gen_), - flags_internal::FlagHelpKind::kGenFunc}, - default_value_gen_); + impl_ = new flags_internal::Flag( + name_, filename_, + {flags_internal::FlagHelpMsg(help_gen_), + flags_internal::FlagHelpKind::kGenFunc}, + {flags_internal::FlagDefaultSrc(default_value_gen_), + flags_internal::FlagDefaultKind::kGenFunc}); inited_.store(true, std::memory_order_release); } @@ -132,14 +133,12 @@ class Flag { // Public methods of `absl::Flag` are NOT part of the Abseil Flags API. // See https://abseil.io/docs/cpp/guides/flags 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(); } bool IsSpecifiedOnCommandLine() const { return 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(); } @@ -311,9 +310,12 @@ ABSL_NAMESPACE_END static std::string NonConst() { return ABSL_FLAG_IMPL_FLAGHELP(txt); } \ } -#define ABSL_FLAG_IMPL_DECLARE_DEF_VAL_WRAPPER(name, Type, default_value) \ - static void AbslFlagsInitFlag##name(void* dst) { \ - absl::flags_internal::MakeFromDefaultValue(dst, default_value); \ +#define ABSL_FLAG_IMPL_DECLARE_DEF_VAL_WRAPPER(name, Type, default_value) \ + struct AbslFlagDefaultGenFor##name { \ + Type value = absl::flags_internal::InitDefaultValue(default_value); \ + static void Gen(void* p) { \ + new (p) Type(AbslFlagDefaultGenFor##name{}.value); \ + } \ } // ABSL_FLAG_IMPL @@ -322,29 +324,30 @@ ABSL_NAMESPACE_END // global name for FLAGS_no symbol, thus preventing the possibility // of defining two flags with names foo and nofoo. #if !defined(_MSC_VER) || defined(__clang__) -#define ABSL_FLAG_IMPL(Type, name, default_value, help) \ - namespace absl /* block flags in namespaces */ {} \ - ABSL_FLAG_IMPL_DECLARE_DEF_VAL_WRAPPER(name, Type, default_value) \ - ABSL_FLAG_IMPL_DECLARE_HELP_WRAPPER(name, help); \ - ABSL_CONST_INIT absl::Flag FLAGS_##name{ \ - ABSL_FLAG_IMPL_FLAGNAME(#name), ABSL_FLAG_IMPL_FILENAME(), \ - absl::flags_internal::HelpArg(0), \ - &AbslFlagsInitFlag##name}; \ - extern absl::flags_internal::FlagRegistrarEmpty FLAGS_no##name; \ - absl::flags_internal::FlagRegistrarEmpty FLAGS_no##name = \ + +#define ABSL_FLAG_IMPL(Type, name, default_value, help) \ + namespace absl /* block flags in namespaces */ {} \ + ABSL_FLAG_IMPL_DECLARE_DEF_VAL_WRAPPER(name, Type, default_value); \ + ABSL_FLAG_IMPL_DECLARE_HELP_WRAPPER(name, help); \ + ABSL_CONST_INIT absl::Flag FLAGS_##name{ \ + ABSL_FLAG_IMPL_FLAGNAME(#name), ABSL_FLAG_IMPL_FILENAME(), \ + absl::flags_internal::HelpArg(0), \ + absl::flags_internal::DefaultArg(0)}; \ + extern absl::flags_internal::FlagRegistrarEmpty FLAGS_no##name; \ + absl::flags_internal::FlagRegistrarEmpty FLAGS_no##name = \ ABSL_FLAG_IMPL_REGISTRAR(Type, FLAGS_##name) #else // MSVC version uses aggregate initialization. We also do not try to // optimize away help wrapper. -#define ABSL_FLAG_IMPL(Type, name, default_value, help) \ - namespace absl /* block flags in namespaces */ {} \ - ABSL_FLAG_IMPL_DECLARE_DEF_VAL_WRAPPER(name, Type, default_value) \ - ABSL_FLAG_IMPL_DECLARE_HELP_WRAPPER(name, help); \ - ABSL_CONST_INIT absl::Flag FLAGS_##name{ \ - ABSL_FLAG_IMPL_FLAGNAME(#name), ABSL_FLAG_IMPL_FILENAME(), \ - &AbslFlagHelpGenFor##name::NonConst, &AbslFlagsInitFlag##name}; \ - extern absl::flags_internal::FlagRegistrarEmpty FLAGS_no##name; \ - absl::flags_internal::FlagRegistrarEmpty FLAGS_no##name = \ +#define ABSL_FLAG_IMPL(Type, name, default_value, help) \ + namespace absl /* block flags in namespaces */ {} \ + ABSL_FLAG_IMPL_DECLARE_DEF_VAL_WRAPPER(name, Type, default_value); \ + ABSL_FLAG_IMPL_DECLARE_HELP_WRAPPER(name, help); \ + ABSL_CONST_INIT absl::Flag FLAGS_##name{ \ + ABSL_FLAG_IMPL_FLAGNAME(#name), ABSL_FLAG_IMPL_FILENAME(), \ + &AbslFlagHelpGenFor##name::NonConst, &AbslFlagDefaultGenFor##name::Gen}; \ + extern absl::flags_internal::FlagRegistrarEmpty FLAGS_no##name; \ + absl::flags_internal::FlagRegistrarEmpty FLAGS_no##name = \ ABSL_FLAG_IMPL_REGISTRAR(Type, FLAGS_##name) #endif diff --git a/absl/flags/flag_test.cc b/absl/flags/flag_test.cc index 6fa178f1f..015b1fc9d 100644 --- a/absl/flags/flag_test.cc +++ b/absl/flags/flag_test.cc @@ -128,26 +128,27 @@ constexpr flags::FlagHelpArg help_arg{flags::FlagHelpMsg("literal help"), using String = std::string; -#define DEFINE_CONSTRUCTED_FLAG(T) \ - constexpr flags::Flag f1##T("f1", "file", help_arg, &TestMakeDflt); \ - ABSL_CONST_INIT flags::Flag f2##T( \ - "f2", "file", \ - {flags::FlagHelpMsg(&TestHelpMsg), flags::FlagHelpKind::kGenFunc}, \ - &TestMakeDflt) +#define DEFINE_CONSTRUCTED_FLAG(T, dflt, dflt_kind) \ + constexpr flags::FlagDefaultArg f1default##T{ \ + flags::FlagDefaultSrc{dflt}, flags::FlagDefaultKind::dflt_kind}; \ + constexpr flags::Flag f1##T("f1", "file", help_arg, f1default##T); \ + ABSL_CONST_INIT flags::Flag f2##T( \ + "f2", "file", \ + {flags::FlagHelpMsg(&TestHelpMsg), flags::FlagHelpKind::kGenFunc}, \ + flags::FlagDefaultArg{flags::FlagDefaultSrc(&TestMakeDflt), \ + flags::FlagDefaultKind::kGenFunc}) -#define TEST_CONSTRUCTED_FLAG(T) TestConstructionFor(f1##T, &f2##T); - -DEFINE_CONSTRUCTED_FLAG(bool); -DEFINE_CONSTRUCTED_FLAG(int16_t); -DEFINE_CONSTRUCTED_FLAG(uint16_t); -DEFINE_CONSTRUCTED_FLAG(int32_t); -DEFINE_CONSTRUCTED_FLAG(uint32_t); -DEFINE_CONSTRUCTED_FLAG(int64_t); -DEFINE_CONSTRUCTED_FLAG(uint64_t); -DEFINE_CONSTRUCTED_FLAG(float); -DEFINE_CONSTRUCTED_FLAG(double); -DEFINE_CONSTRUCTED_FLAG(String); -DEFINE_CONSTRUCTED_FLAG(UDT); +DEFINE_CONSTRUCTED_FLAG(bool, true, kOneWord); +DEFINE_CONSTRUCTED_FLAG(int16_t, 1, kOneWord); +DEFINE_CONSTRUCTED_FLAG(uint16_t, 2, kOneWord); +DEFINE_CONSTRUCTED_FLAG(int32_t, 3, kOneWord); +DEFINE_CONSTRUCTED_FLAG(uint32_t, 4, kOneWord); +DEFINE_CONSTRUCTED_FLAG(int64_t, 5, kOneWord); +DEFINE_CONSTRUCTED_FLAG(uint64_t, 6, kOneWord); +DEFINE_CONSTRUCTED_FLAG(float, 7.8, kFloat); +DEFINE_CONSTRUCTED_FLAG(double, 9.10, kDouble); +DEFINE_CONSTRUCTED_FLAG(String, &TestMakeDflt, kGenFunc); +DEFINE_CONSTRUCTED_FLAG(UDT, &TestMakeDflt, kGenFunc); template bool TestConstructionFor(const flags::Flag& f1, flags::Flag* f2) { @@ -164,6 +165,8 @@ bool TestConstructionFor(const flags::Flag& f1, flags::Flag* f2) { return true; } +#define TEST_CONSTRUCTED_FLAG(T) TestConstructionFor(f1##T, &f2##T); + TEST_F(FlagTest, TestConstruction) { TEST_CONSTRUCTED_FLAG(bool); TEST_CONSTRUCTED_FLAG(int16_t); @@ -443,29 +446,29 @@ TEST_F(FlagTest, TestGetSet) { TEST_F(FlagTest, TestGetViaReflection) { auto* handle = flags::FindCommandLineFlag("test_flag_01"); - EXPECT_EQ(*handle->Get(), true); + EXPECT_EQ(*handle->TryGet(), true); handle = flags::FindCommandLineFlag("test_flag_02"); - EXPECT_EQ(*handle->Get(), 1234); + EXPECT_EQ(*handle->TryGet(), 1234); handle = flags::FindCommandLineFlag("test_flag_03"); - EXPECT_EQ(*handle->Get(), -34); + EXPECT_EQ(*handle->TryGet(), -34); handle = flags::FindCommandLineFlag("test_flag_04"); - EXPECT_EQ(*handle->Get(), 189); + EXPECT_EQ(*handle->TryGet(), 189); handle = flags::FindCommandLineFlag("test_flag_05"); - EXPECT_EQ(*handle->Get(), 10765); + EXPECT_EQ(*handle->TryGet(), 10765); handle = flags::FindCommandLineFlag("test_flag_06"); - EXPECT_EQ(*handle->Get(), 40000); + EXPECT_EQ(*handle->TryGet(), 40000); handle = flags::FindCommandLineFlag("test_flag_07"); - EXPECT_EQ(*handle->Get(), -1234567); + EXPECT_EQ(*handle->TryGet(), -1234567); handle = flags::FindCommandLineFlag("test_flag_08"); - EXPECT_EQ(*handle->Get(), 9876543); + EXPECT_EQ(*handle->TryGet(), 9876543); handle = flags::FindCommandLineFlag("test_flag_09"); - EXPECT_NEAR(*handle->Get(), -9.876e-50, 1e-55); + EXPECT_NEAR(*handle->TryGet(), -9.876e-50, 1e-55); handle = flags::FindCommandLineFlag("test_flag_10"); - EXPECT_NEAR(*handle->Get(), 1.234e12f, 1e5f); + EXPECT_NEAR(*handle->TryGet(), 1.234e12f, 1e5f); handle = flags::FindCommandLineFlag("test_flag_11"); - EXPECT_EQ(*handle->Get(), ""); + EXPECT_EQ(*handle->TryGet(), ""); handle = flags::FindCommandLineFlag("test_flag_12"); - EXPECT_EQ(*handle->Get(), absl::Minutes(10)); + EXPECT_EQ(*handle->TryGet(), absl::Minutes(10)); } // -------------------------------------------------------------------- diff --git a/absl/flags/internal/commandlineflag.cc b/absl/flags/internal/commandlineflag.cc index 90765a3eb..de588c131 100644 --- a/absl/flags/internal/commandlineflag.cc +++ b/absl/flags/internal/commandlineflag.cc @@ -22,7 +22,20 @@ namespace flags_internal { FlagStateInterface::~FlagStateInterface() {} bool CommandLineFlag::IsRetired() const { return false; } -bool CommandLineFlag::IsAbseilFlag() const { return true; } + +FlagFastTypeId PrivateHandleInterface::TypeId(const CommandLineFlag& flag) { + return flag.TypeId(); +} + +std::unique_ptr PrivateHandleInterface::SaveState( + CommandLineFlag* flag) { + return flag->SaveState(); +} + +bool PrivateHandleInterface::ValidateInputValue(const CommandLineFlag& flag, + absl::string_view value) { + return flag.ValidateInputValue(value); +} } // namespace flags_internal ABSL_NAMESPACE_END diff --git a/absl/flags/internal/commandlineflag.h b/absl/flags/internal/commandlineflag.h index ef992f7f4..f807fb9ab 100644 --- a/absl/flags/internal/commandlineflag.h +++ b/absl/flags/internal/commandlineflag.h @@ -93,7 +93,7 @@ class CommandLineFlag { // Attempts to retrieve the flag value. Returns value on success, // absl::nullopt otherwise. template - absl::optional Get() const { + absl::optional TryGet() const { if (IsRetired() || !IsOfType()) { return absl::nullopt; } @@ -130,29 +130,14 @@ class CommandLineFlag { virtual absl::string_view Name() const = 0; // Returns name of the file where this flag is defined. virtual std::string Filename() const = 0; - // Returns name of the flag's value type for some built-in types or empty - // string. - virtual absl::string_view Typename() const = 0; // Returns help message associated with this flag. virtual std::string Help() const = 0; // Returns true iff this object corresponds to retired flag. virtual bool IsRetired() const; - // Returns true iff this is a handle to an Abseil Flag. - virtual bool IsAbseilFlag() const; - // Returns id of the flag's value type. - virtual FlagFastTypeId TypeId() const = 0; - virtual bool IsModified() const = 0; virtual bool IsSpecifiedOnCommandLine() const = 0; virtual std::string DefaultValue() const = 0; virtual std::string CurrentValue() const = 0; - // Interfaces to operate on validators. - virtual bool ValidateInputValue(absl::string_view value) const = 0; - - // Interface to save flag to some persistent state. Returns current flag state - // or nullptr if flag does not support saving and restoring a state. - virtual std::unique_ptr SaveState() = 0; - // Sets the value of the flag based on specified string `value`. If the flag // was successfully set to new value, it returns true. Otherwise, sets `error` // to indicate the error, leaves the flag unchanged, and returns false. There @@ -174,9 +159,38 @@ class CommandLineFlag { ~CommandLineFlag() = default; private: + friend class PrivateHandleInterface; + + // Returns id of the flag's value type. + virtual FlagFastTypeId TypeId() const = 0; + + // Interface to save flag to some persistent state. Returns current flag state + // or nullptr if flag does not support saving and restoring a state. + virtual std::unique_ptr SaveState() = 0; + // Copy-construct a new value of the flag's type in a memory referenced by // the dst based on the current flag's value. virtual void Read(void* dst) const = 0; + + // Interfaces to operate on validators. + // Validates supplied value usign validator or parseflag routine + virtual bool ValidateInputValue(absl::string_view value) const = 0; +}; + +// This class serves as a trampoline to access private methods of +// CommandLineFlag. This class is intended for use exclusively internally inside +// of the Abseil Flags implementation +class PrivateHandleInterface { + public: + // Access to CommandLineFlag::TypeId. + static FlagFastTypeId TypeId(const CommandLineFlag& flag); + + // Access to CommandLineFlag::SaveState. + static std::unique_ptr SaveState(CommandLineFlag* flag); + + // Access to CommandLineFlag::ValidateInputValue. + static bool ValidateInputValue(const CommandLineFlag& flag, + absl::string_view value); }; // This macro is the "source of truth" for the list of supported flag built-in diff --git a/absl/flags/internal/commandlineflag_test.cc b/absl/flags/internal/commandlineflag_test.cc index c1142b7c5..c31679f95 100644 --- a/absl/flags/internal/commandlineflag_test.cc +++ b/absl/flags/internal/commandlineflag_test.cc @@ -67,7 +67,6 @@ TEST_F(CommandLineFlagTest, TestAttributesAccessMethods) { ASSERT_TRUE(flag_01); EXPECT_EQ(flag_01->Name(), "int_flag"); EXPECT_EQ(flag_01->Help(), "int_flag help"); - EXPECT_EQ(flag_01->Typename(), ""); EXPECT_TRUE(!flag_01->IsRetired()); EXPECT_TRUE(flag_01->IsOfType()); EXPECT_TRUE( @@ -80,7 +79,6 @@ TEST_F(CommandLineFlagTest, TestAttributesAccessMethods) { ASSERT_TRUE(flag_02); EXPECT_EQ(flag_02->Name(), "string_flag"); EXPECT_EQ(flag_02->Help(), "string_flag help"); - EXPECT_EQ(flag_02->Typename(), ""); EXPECT_TRUE(!flag_02->IsRetired()); EXPECT_TRUE(flag_02->IsOfType()); EXPECT_TRUE( @@ -93,7 +91,6 @@ TEST_F(CommandLineFlagTest, TestAttributesAccessMethods) { ASSERT_TRUE(flag_03); EXPECT_EQ(flag_03->Name(), "bool_retired_flag"); EXPECT_EQ(flag_03->Help(), ""); - EXPECT_EQ(flag_03->Typename(), ""); EXPECT_TRUE(flag_03->IsRetired()); EXPECT_TRUE(flag_03->IsOfType()); EXPECT_EQ(flag_03->Filename(), "RETIRED"); diff --git a/absl/flags/internal/flag.cc b/absl/flags/internal/flag.cc index 089567f7a..8f0777fa1 100644 --- a/absl/flags/internal/flag.cc +++ b/absl/flags/internal/flag.cc @@ -139,19 +139,43 @@ void DynValueDeleter::operator()(void* ptr) const { void FlagImpl::Init() { new (&data_guard_) absl::Mutex; - // At this point the default_value_ always points to gen_func. + auto def_kind = static_cast(def_kind_); + switch (ValueStorageKind()) { case FlagValueStorageKind::kAlignedBuffer: + // For this storage kind the default_value_ always points to gen_func + // during initialization. + assert(def_kind == FlagDefaultKind::kGenFunc); (*default_value_.gen_func)(AlignedBufferValue()); break; case FlagValueStorageKind::kOneWordAtomic: { alignas(int64_t) std::array buf{}; - (*default_value_.gen_func)(buf.data()); - auto value = absl::bit_cast(buf); - OneWordValue().store(value, std::memory_order_release); + switch (def_kind) { + case FlagDefaultKind::kOneWord: + std::memcpy(buf.data(), &default_value_.one_word, + sizeof(default_value_.one_word)); + break; + case FlagDefaultKind::kFloat: + std::memcpy(buf.data(), &default_value_.float_value, + sizeof(default_value_.float_value)); + break; + case FlagDefaultKind::kDouble: + std::memcpy(buf.data(), &default_value_.double_value, + sizeof(default_value_.double_value)); + break; + default: + assert(def_kind == FlagDefaultKind::kGenFunc); + (*default_value_.gen_func)(buf.data()); + break; + } + OneWordValue().store(absl::bit_cast(buf), + std::memory_order_release); break; } case FlagValueStorageKind::kTwoWordsAtomic: { + // For this storage kind the default_value_ always points to gen_func + // during initialization. + assert(def_kind == FlagDefaultKind::kGenFunc); alignas(AlignedTwoWords) std::array buf{}; (*default_value_.gen_func)(buf.data()); auto atomic_value = absl::bit_cast(buf); @@ -196,11 +220,23 @@ void FlagImpl::AssertValidType(FlagFastTypeId rhs_type_id, std::unique_ptr FlagImpl::MakeInitValue() const { void* res = nullptr; - if (DefaultKind() == FlagDefaultKind::kDynamicValue) { - res = flags_internal::Clone(op_, default_value_.dynamic_value); - } else { - res = flags_internal::Alloc(op_); - (*default_value_.gen_func)(res); + switch (DefaultKind()) { + case FlagDefaultKind::kDynamicValue: + res = flags_internal::Clone(op_, default_value_.dynamic_value); + break; + case FlagDefaultKind::kGenFunc: + res = flags_internal::Alloc(op_); + (*default_value_.gen_func)(res); + break; + case FlagDefaultKind::kOneWord: + res = flags_internal::Clone(op_, &default_value_.one_word); + break; + case FlagDefaultKind::kFloat: + res = flags_internal::Clone(op_, &default_value_.float_value); + break; + case FlagDefaultKind::kDouble: + res = flags_internal::Clone(op_, &default_value_.double_value); + break; } return {res, DynValueDeleter{op_}}; } @@ -235,8 +271,6 @@ std::string FlagImpl::Filename() const { return flags_internal::GetUsageConfig().normalize_filename(filename_); } -absl::string_view FlagImpl::Typename() const { return ""; } - std::string FlagImpl::Help() const { return HelpSourceKind() == FlagHelpKind::kLiteral ? help_.literal : help_.gen_func(); @@ -246,11 +280,6 @@ FlagFastTypeId FlagImpl::TypeId() const { return flags_internal::FastTypeId(op_); } -bool FlagImpl::IsModified() const { - absl::MutexLock l(DataGuard()); - return modified_; -} - bool FlagImpl::IsSpecifiedOnCommandLine() const { absl::MutexLock l(DataGuard()); return on_command_line_; diff --git a/absl/flags/internal/flag.h b/absl/flags/internal/flag.h index 6da25aa94..b22088245 100644 --- a/absl/flags/internal/flag.h +++ b/absl/flags/internal/flag.h @@ -206,12 +206,72 @@ using FlagDfltGenFunc = void (*)(void*); union FlagDefaultSrc { constexpr explicit FlagDefaultSrc(FlagDfltGenFunc gen_func_arg) : gen_func(gen_func_arg) {} + template + constexpr explicit FlagDefaultSrc(T one_word_value) + : one_word(static_cast(one_word_value)) {} + constexpr explicit FlagDefaultSrc(float f) : float_value(f) {} + constexpr explicit FlagDefaultSrc(double d) : double_value(d) {} void* dynamic_value; FlagDfltGenFunc gen_func; + int64_t one_word; + float float_value; + double double_value; }; -enum class FlagDefaultKind : uint8_t { kDynamicValue = 0, kGenFunc = 1 }; +enum class FlagDefaultKind : uint8_t { + kDynamicValue = 0, + kGenFunc = 1, + kOneWord = 2, + kFloat = 3, + kDouble = 4 +}; + +struct FlagDefaultArg { + FlagDefaultSrc source; + FlagDefaultKind kind; +}; + +// This struct and corresponding overload to InitDefaultValue are used to +// facilitate usage of {} as default value in ABSL_FLAG macro. +// TODO(rogeeff): Fix handling types with explicit constructors. +struct EmptyBraces {}; + +template +constexpr T InitDefaultValue(T t) { + return t; +} + +template +constexpr T InitDefaultValue(EmptyBraces) { + return T{}; +} + +template ::value, int>::type = + (GenT{}, 0)> +constexpr FlagDefaultArg DefaultArg(int) { + return {FlagDefaultSrc(GenT{}.value), FlagDefaultKind::kOneWord}; +} + +template ::value, + int>::type = (GenT{}, 0)> +constexpr FlagDefaultArg DefaultArg(int) { + return {FlagDefaultSrc(GenT{}.value), FlagDefaultKind::kFloat}; +} + +template ::value, + int>::type = (GenT{}, 0)> +constexpr FlagDefaultArg DefaultArg(int) { + return {FlagDefaultSrc(GenT{}.value), FlagDefaultKind::kDouble}; +} + +template +constexpr FlagDefaultArg DefaultArg(char) { + return {FlagDefaultSrc(&GenT::Gen), FlagDefaultKind::kGenFunc}; +} /////////////////////////////////////////////////////////////////////////////// // Flag current value auxiliary structs. @@ -356,19 +416,19 @@ class FlagImpl final : public flags_internal::CommandLineFlag { public: constexpr FlagImpl(const char* name, const char* filename, FlagOpFn op, FlagHelpArg help, FlagValueStorageKind value_kind, - FlagDfltGenFunc default_value_gen) + FlagDefaultArg default_arg) : name_(name), filename_(filename), op_(op), help_(help.source), help_source_kind_(static_cast(help.kind)), value_storage_kind_(static_cast(value_kind)), - def_kind_(static_cast(FlagDefaultKind::kGenFunc)), + def_kind_(static_cast(default_arg.kind)), modified_(false), on_command_line_(false), counter_(0), callback_(nullptr), - default_value_(default_value_gen), + default_value_(default_arg.source), data_guard_{} {} // Constant access methods @@ -444,10 +504,8 @@ class FlagImpl final : public flags_internal::CommandLineFlag { // CommandLineFlag interface implementation absl::string_view Name() const override; std::string Filename() const override; - absl::string_view Typename() const override; std::string Help() const override; FlagFastTypeId TypeId() const override; - bool IsModified() const override ABSL_LOCKS_EXCLUDED(*DataGuard()); bool IsSpecifiedOnCommandLine() const override ABSL_LOCKS_EXCLUDED(*DataGuard()); std::string DefaultValue() const override ABSL_LOCKS_EXCLUDED(*DataGuard()); @@ -492,9 +550,10 @@ class FlagImpl final : public flags_internal::CommandLineFlag { // Mutable flag's state (guarded by `data_guard_`). - // If def_kind_ == kDynamicValue, default_value_ holds a dynamically allocated - // value. - uint8_t def_kind_ : 1 ABSL_GUARDED_BY(*DataGuard()); + // def_kind_ is not guard by DataGuard() since it is accessed in Init without + // locks. If necessary we can decrease number of bits used to 2 by folding + // one_word storage cases. + uint8_t def_kind_ : 3; // Has this flag's value been modified? bool modified_ : 1 ABSL_GUARDED_BY(*DataGuard()); // Has this flag been specified on command line. @@ -530,10 +589,10 @@ class FlagImpl final : public flags_internal::CommandLineFlag { template class Flag { public: - constexpr Flag(const char* name, const char* filename, const FlagHelpArg help, - const FlagDfltGenFunc default_value_gen) + constexpr Flag(const char* name, const char* filename, FlagHelpArg help, + const FlagDefaultArg default_arg) : impl_(name, filename, &FlagOps, help, - flags_internal::StorageKind(), default_value_gen), + flags_internal::StorageKind(), default_arg), value_() {} T Get() const { @@ -560,9 +619,7 @@ class Flag { // CommandLineFlag interface absl::string_view Name() const { return impl_.Name(); } std::string Filename() const { return impl_.Filename(); } - absl::string_view Typename() const { return ""; } std::string Help() const { return impl_.Help(); } - bool IsModified() const { return impl_.IsModified(); } bool IsSpecifiedOnCommandLine() const { return impl_.IsSpecifiedOnCommandLine(); } @@ -662,20 +719,6 @@ class FlagRegistrar { Flag* flag_; // Flag being registered (not owned). }; -// This struct and corresponding overload to MakeDefaultValue are used to -// facilitate usage of {} as default value in ABSL_FLAG macro. -struct EmptyBraces {}; - -template -void MakeFromDefaultValue(void* dst, T t) { - new (dst) T(std::move(t)); -} - -template -void MakeFromDefaultValue(void* dst, EmptyBraces) { - new (dst) T{}; -} - } // namespace flags_internal ABSL_NAMESPACE_END } // namespace absl diff --git a/absl/flags/internal/registry.cc b/absl/flags/internal/registry.cc index eb619c708..62b5b40d8 100644 --- a/absl/flags/internal/registry.cc +++ b/absl/flags/internal/registry.cc @@ -127,14 +127,13 @@ void FlagRegistry::RegisterFlag(CommandLineFlag* flag) { (flag->IsRetired() ? old_flag->Filename() : flag->Filename()), "'."), true); - } else if (flag->TypeId() != old_flag->TypeId()) { + } else if (flags_internal::PrivateHandleInterface::TypeId(*flag) != + flags_internal::PrivateHandleInterface::TypeId(*old_flag)) { flags_internal::ReportUsageError( absl::StrCat("Flag '", flag->Name(), "' was defined more than once but with " "differing types. Defined in files '", - old_flag->Filename(), "' and '", flag->Filename(), - "' with types '", old_flag->Typename(), "' and '", - flag->Typename(), "', respectively."), + old_flag->Filename(), "' and '", flag->Filename(), "'."), true); } else if (old_flag->IsRetired()) { // Retired flag can just be deleted. @@ -206,7 +205,8 @@ class FlagSaverImpl { void SaveFromRegistry() { assert(backup_registry_.empty()); // call only once! flags_internal::ForEachFlag([&](flags_internal::CommandLineFlag* flag) { - if (auto flag_state = flag->SaveState()) { + if (auto flag_state = + flags_internal::PrivateHandleInterface::SaveState(flag)) { backup_registry_.emplace_back(std::move(flag_state)); } }); @@ -290,11 +290,9 @@ class RetiredFlagObj final : public flags_internal::CommandLineFlag { private: absl::string_view Name() const override { return name_; } std::string Filename() const override { return "RETIRED"; } - absl::string_view Typename() const override { return ""; } FlagFastTypeId TypeId() const override { return type_id_; } std::string Help() const override { return ""; } bool IsRetired() const override { return true; } - bool IsModified() const override { return false; } bool IsSpecifiedOnCommandLine() const override { return false; } std::string DefaultValue() const override { return ""; } std::string CurrentValue() const override { return ""; } diff --git a/absl/flags/internal/type_erased.cc b/absl/flags/internal/type_erased.cc index 75b4cdf89..5038625be 100644 --- a/absl/flags/internal/type_erased.cc +++ b/absl/flags/internal/type_erased.cc @@ -72,7 +72,9 @@ bool IsValidFlagValue(absl::string_view name, absl::string_view value) { CommandLineFlag* flag = flags_internal::FindCommandLineFlag(name); return flag != nullptr && - (flag->IsRetired() || flag->ValidateInputValue(value)); + (flag->IsRetired() || + flags_internal::PrivateHandleInterface::ValidateInputValue(*flag, + value)); } // -------------------------------------------------------------------- diff --git a/absl/flags/internal/type_erased.h b/absl/flags/internal/type_erased.h index 188429c77..ffe319bad 100644 --- a/absl/flags/internal/type_erased.h +++ b/absl/flags/internal/type_erased.h @@ -75,7 +75,7 @@ inline bool GetByName(absl::string_view name, T* dst) { CommandLineFlag* flag = flags_internal::FindCommandLineFlag(name); if (!flag) return false; - if (auto val = flag->Get()) { + if (auto val = flag->TryGet()) { *dst = *val; return true; } diff --git a/absl/flags/internal/usage.cc b/absl/flags/internal/usage.cc index a9a5cba94..9d856c87a 100644 --- a/absl/flags/internal/usage.cc +++ b/absl/flags/internal/usage.cc @@ -54,27 +54,6 @@ ABSL_NAMESPACE_BEGIN namespace flags_internal { namespace { -absl::string_view TypenameForHelp(const flags_internal::CommandLineFlag& flag) { - // Only report names of v1 built-in types -#define HANDLE_V1_BUILTIN_TYPE(t) \ - if (flag.IsOfType()) { \ - return #t; \ - } - - HANDLE_V1_BUILTIN_TYPE(bool); - HANDLE_V1_BUILTIN_TYPE(int32_t); - HANDLE_V1_BUILTIN_TYPE(int64_t); - HANDLE_V1_BUILTIN_TYPE(uint64_t); - HANDLE_V1_BUILTIN_TYPE(double); -#undef HANDLE_V1_BUILTIN_TYPE - - if (flag.IsOfType()) { - return "string"; - } - - return ""; -} - // This class is used to emit an XML element with `tag` and `text`. // It adds opening and closing tags and escapes special characters in the text. // For example: @@ -212,23 +191,20 @@ void FlagHelpHumanReadable(const flags_internal::CommandLineFlag& flag, // Flag help. printer.Write(absl::StrCat("(", flag.Help(), ");"), /*wrap_line=*/true); - // Flag data type (for V1 flags only). - if (!flag.IsAbseilFlag() && !flag.IsRetired()) { - printer.Write(absl::StrCat("type: ", TypenameForHelp(flag), ";")); - } - // The listed default value will be the actual default from the flag // definition in the originating source file, unless the value has // subsequently been modified using SetCommandLineOption() with mode // SET_FLAGS_DEFAULT. std::string dflt_val = flag.DefaultValue(); + std::string curr_val = flag.CurrentValue(); + bool is_modified = curr_val != dflt_val; + if (flag.IsOfType()) { dflt_val = absl::StrCat("\"", dflt_val, "\""); } printer.Write(absl::StrCat("default: ", dflt_val, ";")); - if (flag.IsModified()) { - std::string curr_val = flag.CurrentValue(); + if (is_modified) { if (flag.IsOfType()) { curr_val = absl::StrCat("\"", curr_val, "\""); } diff --git a/absl/strings/internal/str_format/arg.cc b/absl/strings/internal/str_format/arg.cc index 12842276a..a112071c4 100644 --- a/absl/strings/internal/str_format/arg.cc +++ b/absl/strings/internal/str_format/arg.cc @@ -315,7 +315,7 @@ bool ConvertFloatArg(T v, const ConversionSpec conv, FormatSinkImpl *sink) { inline bool ConvertStringArg(string_view v, const ConversionSpec conv, FormatSinkImpl *sink) { - if (conv.conversion_char() != ConversionChar::s) return false; + if (conv.conversion_char() != FormatConversionCharInternal::s) return false; if (conv.is_basic()) { sink->Append(v); return true; @@ -327,22 +327,22 @@ inline bool ConvertStringArg(string_view v, const ConversionSpec conv, } // namespace // ==================== Strings ==================== -ConvertResult FormatConvertImpl(const std::string &v, - const ConversionSpec conv, - FormatSinkImpl *sink) { +StringConvertResult FormatConvertImpl(const std::string &v, + const ConversionSpec conv, + FormatSinkImpl *sink) { return {ConvertStringArg(v, conv, sink)}; } -ConvertResult FormatConvertImpl(string_view v, - const ConversionSpec conv, - FormatSinkImpl *sink) { +StringConvertResult FormatConvertImpl(string_view v, const ConversionSpec conv, + FormatSinkImpl *sink) { return {ConvertStringArg(v, conv, sink)}; } -ConvertResult FormatConvertImpl(const char *v, - const ConversionSpec conv, - FormatSinkImpl *sink) { - if (conv.conversion_char() == ConversionChar::p) +ArgConvertResult +FormatConvertImpl(const char *v, const ConversionSpec conv, + FormatSinkImpl *sink) { + if (conv.conversion_char() == FormatConversionCharInternal::p) return {FormatConvertImpl(VoidPtr(v), conv, sink).value}; size_t len; if (v == nullptr) { @@ -357,9 +357,9 @@ ConvertResult FormatConvertImpl(const char *v, } // ==================== Raw pointers ==================== -ConvertResult FormatConvertImpl(VoidPtr v, const ConversionSpec conv, - FormatSinkImpl *sink) { - if (conv.conversion_char() != ConversionChar::p) return {false}; +ArgConvertResult FormatConvertImpl( + VoidPtr v, const ConversionSpec conv, FormatSinkImpl *sink) { + if (conv.conversion_char() != FormatConversionCharInternal::p) return {false}; if (!v.value) { sink->Append("(nil)"); return {true}; diff --git a/absl/strings/internal/str_format/arg.h b/absl/strings/internal/str_format/arg.h index 1c36e3098..f4ac940ab 100644 --- a/absl/strings/internal/str_format/arg.h +++ b/absl/strings/internal/str_format/arg.h @@ -31,10 +31,11 @@ template struct HasUserDefinedConvert : std::false_type {}; template -struct HasUserDefinedConvert< - T, void_t(), std::declval(), - std::declval()))>> : std::true_type {}; +struct HasUserDefinedConvert(), + std::declval(), + std::declval()))>> + : std::true_type {}; template class StreamedWrapper; @@ -52,25 +53,36 @@ struct VoidPtr { : value(ptr ? reinterpret_cast(ptr) : 0) {} uintptr_t value; }; -ConvertResult FormatConvertImpl(VoidPtr v, ConversionSpec conv, - FormatSinkImpl* sink); + +template +struct ArgConvertResult { + bool value; +}; + +template +constexpr FormatConversionCharSet ExtractCharSet(ArgConvertResult) { + return C; +} + +using StringConvertResult = + ArgConvertResult; +ArgConvertResult FormatConvertImpl( + VoidPtr v, ConversionSpec conv, FormatSinkImpl* sink); // Strings. -ConvertResult FormatConvertImpl(const std::string& v, - ConversionSpec conv, - FormatSinkImpl* sink); -ConvertResult FormatConvertImpl(string_view v, ConversionSpec conv, - FormatSinkImpl* sink); -ConvertResult FormatConvertImpl(const char* v, - ConversionSpec conv, - FormatSinkImpl* sink); -template ::value>::type* = nullptr> -ConvertResult FormatConvertImpl(const AbslCord& value, - ConversionSpec conv, - FormatSinkImpl* sink) { - if (conv.conversion_char() != ConversionChar::s) { +StringConvertResult FormatConvertImpl(const std::string& v, ConversionSpec conv, + FormatSinkImpl* sink); +StringConvertResult FormatConvertImpl(string_view v, ConversionSpec conv, + FormatSinkImpl* sink); +ArgConvertResult +FormatConvertImpl(const char* v, ConversionSpec conv, FormatSinkImpl* sink); +template ::value>::type* = nullptr> +StringConvertResult FormatConvertImpl(const AbslCord& value, + ConversionSpec conv, + FormatSinkImpl* sink) { + if (conv.conversion_char() != FormatConversionCharInternal::s) { return {false}; } @@ -107,9 +119,12 @@ ConvertResult FormatConvertImpl(const AbslCord& value, return {true}; } -using IntegralConvertResult = - ConvertResult; -using FloatingConvertResult = ConvertResult; +using IntegralConvertResult = ArgConvertResult; +using FloatingConvertResult = + ArgConvertResult; // Floats. FloatingConvertResult FormatConvertImpl(float v, ConversionSpec conv, @@ -169,9 +184,9 @@ typename std::enable_if::value && FormatConvertImpl(T v, ConversionSpec conv, FormatSinkImpl* sink); template -ConvertResult FormatConvertImpl(const StreamedWrapper& v, - ConversionSpec conv, - FormatSinkImpl* out) { +StringConvertResult FormatConvertImpl(const StreamedWrapper& v, + ConversionSpec conv, + FormatSinkImpl* out) { std::ostringstream oss; oss << v.v_; if (!oss) return {false}; @@ -182,12 +197,12 @@ ConvertResult FormatConvertImpl(const StreamedWrapper& v, // until after FormatCountCapture is fully defined. struct FormatCountCaptureHelper { template - static ConvertResult ConvertHelper(const FormatCountCapture& v, - ConversionSpec conv, - FormatSinkImpl* sink) { + static ArgConvertResult ConvertHelper( + const FormatCountCapture& v, ConversionSpec conv, FormatSinkImpl* sink) { const absl::enable_if_t& v2 = v; - if (conv.conversion_char() != str_format_internal::ConversionChar::n) { + if (conv.conversion_char() != + str_format_internal::FormatConversionCharInternal::n) { return {false}; } *v2.p_ = static_cast(sink->size()); @@ -196,9 +211,8 @@ struct FormatCountCaptureHelper { }; template -ConvertResult FormatConvertImpl(const FormatCountCapture& v, - ConversionSpec conv, - FormatSinkImpl* sink) { +ArgConvertResult FormatConvertImpl( + const FormatCountCapture& v, ConversionSpec conv, FormatSinkImpl* sink) { return FormatCountCaptureHelper::ConvertHelper(v, conv, sink); } @@ -381,7 +395,8 @@ class FormatArgImpl { template static bool Dispatch(Data arg, ConversionSpec spec, void* out) { // A `none` conv indicates that we want the `int` conversion. - if (ABSL_PREDICT_FALSE(spec.conversion_char() == ConversionChar::kNone)) { + if (ABSL_PREDICT_FALSE(spec.conversion_char() == + FormatConversionCharInternal::kNone)) { return ToInt(arg, static_cast(out), std::is_integral(), std::is_enum()); } diff --git a/absl/strings/internal/str_format/bind.h b/absl/strings/internal/str_format/bind.h index d30fdf507..05105d8d5 100644 --- a/absl/strings/internal/str_format/bind.h +++ b/absl/strings/internal/str_format/bind.h @@ -189,9 +189,8 @@ class StreamedWrapper { private: template - friend ConvertResult FormatConvertImpl(const StreamedWrapper& v, - ConversionSpec conv, - FormatSinkImpl* out); + friend ArgConvertResult FormatConvertImpl( + const StreamedWrapper& v, ConversionSpec conv, FormatSinkImpl* out); const T& v_; }; diff --git a/absl/strings/internal/str_format/checker.h b/absl/strings/internal/str_format/checker.h index 8993a79b9..73ef05ff4 100644 --- a/absl/strings/internal/str_format/checker.h +++ b/absl/strings/internal/str_format/checker.h @@ -25,10 +25,12 @@ constexpr bool AllOf(bool b, T... t) { } template -constexpr Conv ArgumentToConv() { - return decltype(str_format_internal::FormatConvertImpl( - std::declval(), std::declval(), - std::declval()))::kConv; +constexpr FormatConversionCharSet ArgumentToConv() { + return absl::str_format_internal::ExtractCharSet( + decltype(str_format_internal::FormatConvertImpl( + std::declval(), + std::declval(), + std::declval())){}); } #ifdef ABSL_INTERNAL_ENABLE_FORMAT_CHECKER @@ -39,14 +41,14 @@ constexpr bool ContainsChar(const char* chars, char c) { // A constexpr compatible list of Convs. struct ConvList { - const Conv* array; + const FormatConversionCharSet* array; int count; // We do the bound check here to avoid having to do it on the callers. - // Returning an empty Conv has the same effect as short circuiting because it - // will never match any conversion. - constexpr Conv operator[](int i) const { - return i < count ? array[i] : Conv{}; + // Returning an empty FormatConversionCharSet has the same effect as + // short circuiting because it will never match any conversion. + constexpr FormatConversionCharSet operator[](int i) const { + return i < count ? array[i] : FormatConversionCharSet{}; } constexpr ConvList without_front() const { @@ -57,7 +59,7 @@ struct ConvList { template struct ConvListT { // Make sure the array has size > 0. - Conv list[count ? count : 1]; + FormatConversionCharSet list[count ? count : 1]; }; constexpr char GetChar(string_view str, size_t index) { @@ -310,7 +312,7 @@ class FormatParser { ConvList args_; }; -template +template constexpr bool ValidFormatImpl(string_view format) { return FormatParser(format, {ConvListT{{C...}}.list, sizeof...(C)}) diff --git a/absl/strings/internal/str_format/extension.h b/absl/strings/internal/str_format/extension.h index fb31a9db4..f0cffe1e6 100644 --- a/absl/strings/internal/str_format/extension.h +++ b/absl/strings/internal/str_format/extension.h @@ -30,7 +30,10 @@ namespace absl { ABSL_NAMESPACE_BEGIN + namespace str_format_internal { +enum class FormatConversionCharSet : uint64_t; +enum class FormatConversionChar : uint8_t; class FormatRawSinkImpl { public: @@ -149,13 +152,39 @@ struct Flags { X_VAL(g) X_SEP X_VAL(G) X_SEP X_VAL(a) X_SEP X_VAL(A) X_SEP \ /* misc */ \ X_VAL(n) X_SEP X_VAL(p) +// clang-format on -enum class FormatConversionChar : uint8_t { +// This type should not be referenced, it exists only to provide labels +// internally that match the values declared in FormatConversionChar in +// str_format.h. This is meant to allow internal libraries to use the same +// declared interface type as the public interface +// (absl::StrFormatConversionChar) while keeping the definition in a public +// header. +// Internal libraries should use the form +// `FormatConversionCharInternal::c`, `FormatConversionCharInternal::kNone` for +// comparisons. Use in switch statements is not recommended due to a bug in how +// gcc 4.9 -Wswitch handles declared but undefined enums. +struct FormatConversionCharInternal { + FormatConversionCharInternal() = delete; + + private: + // clang-format off + enum class Enum : uint8_t { c, C, s, S, // text d, i, o, u, x, X, // int f, F, e, E, g, G, a, A, // float n, p, // misc kNone + }; + // clang-format on + public: +#define ABSL_INTERNAL_X_VAL(id) \ + static constexpr FormatConversionChar id = \ + static_cast(Enum::id); + ABSL_INTERNAL_CONVERSION_CHARS_EXPAND_(ABSL_INTERNAL_X_VAL, ) +#undef ABSL_INTERNAL_X_VAL + static constexpr FormatConversionChar kNone = + static_cast(Enum::kNone); }; // clang-format on @@ -163,56 +192,56 @@ inline FormatConversionChar FormatConversionCharFromChar(char c) { switch (c) { #define ABSL_INTERNAL_X_VAL(id) \ case #id[0]: \ - return FormatConversionChar::id; + return FormatConversionCharInternal::id; ABSL_INTERNAL_CONVERSION_CHARS_EXPAND_(ABSL_INTERNAL_X_VAL, ) #undef ABSL_INTERNAL_X_VAL } - return FormatConversionChar::kNone; + return FormatConversionCharInternal::kNone; } inline bool FormatConversionCharIsUpper(FormatConversionChar c) { - switch (c) { - case FormatConversionChar::X: - case FormatConversionChar::F: - case FormatConversionChar::E: - case FormatConversionChar::G: - case FormatConversionChar::A: - return true; - default: - return false; + if (c == FormatConversionCharInternal::X || + c == FormatConversionCharInternal::F || + c == FormatConversionCharInternal::E || + c == FormatConversionCharInternal::G || + c == FormatConversionCharInternal::A) { + return true; + } else { + return false; } } inline bool FormatConversionCharIsFloat(FormatConversionChar c) { - switch (c) { - case FormatConversionChar::a: - case FormatConversionChar::e: - case FormatConversionChar::f: - case FormatConversionChar::g: - case FormatConversionChar::A: - case FormatConversionChar::E: - case FormatConversionChar::F: - case FormatConversionChar::G: - return true; - default: - return false; + if (c == FormatConversionCharInternal::a || + c == FormatConversionCharInternal::e || + c == FormatConversionCharInternal::f || + c == FormatConversionCharInternal::g || + c == FormatConversionCharInternal::A || + c == FormatConversionCharInternal::E || + c == FormatConversionCharInternal::F || + c == FormatConversionCharInternal::G) { + return true; + } else { + return false; } } inline char FormatConversionCharToChar(FormatConversionChar c) { - switch (c) { -#define ABSL_INTERNAL_X_VAL(e) \ - case FormatConversionChar::e: \ + if (c == FormatConversionCharInternal::kNone) { + return '\0'; + +#define ABSL_INTERNAL_X_VAL(e) \ + } else if (c == FormatConversionCharInternal::e) { \ return #e[0]; #define ABSL_INTERNAL_X_SEP - ABSL_INTERNAL_CONVERSION_CHARS_EXPAND_(ABSL_INTERNAL_X_VAL, - ABSL_INTERNAL_X_SEP) - case FormatConversionChar::kNone: - return '\0'; + ABSL_INTERNAL_CONVERSION_CHARS_EXPAND_(ABSL_INTERNAL_X_VAL, + ABSL_INTERNAL_X_SEP) + } else { + return '\0'; + } + #undef ABSL_INTERNAL_X_VAL #undef ABSL_INTERNAL_X_SEP - } - return '\0'; } // The associated char. @@ -224,7 +253,7 @@ inline std::ostream& operator<<(std::ostream& os, FormatConversionChar v) { struct FormatConversionSpecImplFriend; -class FormatConversionSpec { +class FormatConversionSpecImpl { public: // Width and precison are not specified, no flags are set. bool is_basic() const { return flags_.basic; } @@ -237,7 +266,7 @@ class FormatConversionSpec { FormatConversionChar conversion_char() const { // Keep this field first in the struct . It generates better code when // accessing it when ConversionSpec is passed by value in registers. - static_assert(offsetof(FormatConversionSpec, conv_) == 0, ""); + static_assert(offsetof(FormatConversionSpecImpl, conv_) == 0, ""); return conv_; } @@ -248,37 +277,62 @@ class FormatConversionSpec { // negative value. int precision() const { return precision_; } + template + T Wrap() { + return T(*this); + } + private: friend struct str_format_internal::FormatConversionSpecImplFriend; - FormatConversionChar conv_ = FormatConversionChar::kNone; + FormatConversionChar conv_ = FormatConversionCharInternal::kNone; Flags flags_; int width_; int precision_; }; struct FormatConversionSpecImplFriend final { - static void SetFlags(Flags f, FormatConversionSpec* conv) { + static void SetFlags(Flags f, FormatConversionSpecImpl* conv) { conv->flags_ = f; } static void SetConversionChar(FormatConversionChar c, - FormatConversionSpec* conv) { + FormatConversionSpecImpl* conv) { conv->conv_ = c; } - static void SetWidth(int w, FormatConversionSpec* conv) { conv->width_ = w; } - static void SetPrecision(int p, FormatConversionSpec* conv) { + static void SetWidth(int w, FormatConversionSpecImpl* conv) { + conv->width_ = w; + } + static void SetPrecision(int p, FormatConversionSpecImpl* conv) { conv->precision_ = p; } - static std::string FlagsToString(const FormatConversionSpec& spec) { + static std::string FlagsToString(const FormatConversionSpecImpl& spec) { return spec.flags_.ToString(); } }; -constexpr uint64_t FormatConversionCharToConvValue(char conv) { +// Type safe OR operator. +// We need this for two reasons: +// 1. operator| on enums makes them decay to integers and the result is an +// integer. We need the result to stay as an enum. +// 2. We use "enum class" which would not work even if we accepted the decay. +constexpr FormatConversionCharSet FormatConversionCharSetUnion( + FormatConversionCharSet a) { + return a; +} + +template +constexpr FormatConversionCharSet FormatConversionCharSetUnion( + FormatConversionCharSet a, CharSet... rest) { + return static_cast( + static_cast(a) | + static_cast(FormatConversionCharSetUnion(rest...))); +} + +constexpr uint64_t FormatConversionCharToConvInt(char conv) { return -#define ABSL_INTERNAL_CHAR_SET_CASE(c) \ - conv == #c[0] \ - ? (uint64_t{1} << (1 + static_cast(FormatConversionChar::c))) \ - : +#define ABSL_INTERNAL_CHAR_SET_CASE(c) \ + conv == #c[0] ? (uint64_t{1} << (1 + static_cast( \ + FormatConversionCharInternal::c))) \ + : ABSL_INTERNAL_CONVERSION_CHARS_EXPAND_(ABSL_INTERNAL_CHAR_SET_CASE, ) #undef ABSL_INTERNAL_CHAR_SET_CASE conv == '*' @@ -286,20 +340,31 @@ constexpr uint64_t FormatConversionCharToConvValue(char conv) { : 0; } -enum class FormatConversionCharSet : uint64_t { -#define ABSL_INTERNAL_CHAR_SET_CASE(c) \ - c = FormatConversionCharToConvValue(#c[0]), +constexpr FormatConversionCharSet FormatConversionCharToConvValue(char conv) { + return static_cast( + FormatConversionCharToConvInt(conv)); +} + +struct FormatConversionCharSetInternal { +#define ABSL_INTERNAL_CHAR_SET_CASE(c) \ + static constexpr FormatConversionCharSet c = \ + FormatConversionCharToConvValue(#c[0]); ABSL_INTERNAL_CONVERSION_CHARS_EXPAND_(ABSL_INTERNAL_CHAR_SET_CASE, ) #undef ABSL_INTERNAL_CHAR_SET_CASE // Used for width/precision '*' specification. - kStar = FormatConversionCharToConvValue('*'), - // Some predefined values: - kIntegral = d | i | u | o | x | X, - kFloating = a | e | f | g | A | E | F | G, - kNumeric = kIntegral | kFloating, - kString = s, - kPointer = p + static constexpr FormatConversionCharSet kStar = + FormatConversionCharToConvValue('*'); + + // Some predefined values (TODO(matthewbr), delete any that are unused). + static constexpr FormatConversionCharSet kIntegral = + FormatConversionCharSetUnion(d, i, u, o, x, X); + static constexpr FormatConversionCharSet kFloating = + FormatConversionCharSetUnion(a, e, f, g, A, E, F, G); + static constexpr FormatConversionCharSet kNumeric = + FormatConversionCharSetUnion(kIntegral, kFloating); + static constexpr FormatConversionCharSet kString = s; + static constexpr FormatConversionCharSet kPointer = p; }; // Type safe OR operator. @@ -309,8 +374,7 @@ enum class FormatConversionCharSet : uint64_t { // 2. We use "enum class" which would not work even if we accepted the decay. constexpr FormatConversionCharSet operator|(FormatConversionCharSet a, FormatConversionCharSet b) { - return FormatConversionCharSet(static_cast(a) | - static_cast(b)); + return FormatConversionCharSetUnion(a, b); } // Overloaded conversion functions to support absl::ParsedFormat. @@ -331,7 +395,8 @@ void ToFormatConversionCharSet(T) = delete; // Checks whether `c` exists in `set`. constexpr bool Contains(FormatConversionCharSet set, char c) { - return (static_cast(set) & FormatConversionCharToConvValue(c)) != 0; + return (static_cast(set) & + static_cast(FormatConversionCharToConvValue(c))) != 0; } // Checks whether all the characters in `c` are contained in `set` @@ -341,19 +406,6 @@ constexpr bool Contains(FormatConversionCharSet set, static_cast(c); } -// Return type of the AbslFormatConvert() functions. -// The FormatConversionCharSet template parameter is used to inform the -// framework of what conversion characters are supported by that -// AbslFormatConvert routine. -template -struct FormatConvertResult { - static constexpr FormatConversionCharSet kConv = C; - bool value; -}; - -template -constexpr FormatConversionCharSet FormatConvertResult::kConv; - // Return capacity - used, clipped to a minimum of 0. inline size_t Excess(size_t used, size_t capacity) { return used < capacity ? capacity - used : 0; @@ -361,10 +413,85 @@ inline size_t Excess(size_t used, size_t capacity) { // Type alias for use during migration. using ConversionChar = FormatConversionChar; -using ConversionSpec = FormatConversionSpec; +using ConversionSpec = FormatConversionSpecImpl; using Conv = FormatConversionCharSet; -template -using ConvertResult = FormatConvertResult; + +class FormatConversionSpec { + public: + // Width and precison are not specified, no flags are set. + bool is_basic() const { return impl_.is_basic(); } + bool has_left_flag() const { return impl_.has_left_flag(); } + bool has_show_pos_flag() const { return impl_.has_show_pos_flag(); } + bool has_sign_col_flag() const { return impl_.has_sign_col_flag(); } + bool has_alt_flag() const { return impl_.has_alt_flag(); } + bool has_zero_flag() const { return impl_.has_zero_flag(); } + + FormatConversionChar conversion_char() const { + return impl_.conversion_char(); + } + + // Returns the specified width. If width is unspecfied, it returns a negative + // value. + int width() const { return impl_.width(); } + // Returns the specified precision. If precision is unspecfied, it returns a + // negative value. + int precision() const { return impl_.precision(); } + + private: + explicit FormatConversionSpec( + str_format_internal::FormatConversionSpecImpl impl) + : impl_(impl) {} + + friend str_format_internal::FormatConversionSpecImpl; + + absl::str_format_internal::FormatConversionSpecImpl impl_; +}; + +// clang-format off +enum class FormatConversionChar : uint8_t { + c, C, s, S, // text + d, i, o, u, x, X, // int + f, F, e, E, g, G, a, A, // float + n, p // misc +}; +// clang-format on + +enum class FormatConversionCharSet : uint64_t { + // text + c = str_format_internal::FormatConversionCharToConvInt('c'), + C = str_format_internal::FormatConversionCharToConvInt('C'), + s = str_format_internal::FormatConversionCharToConvInt('s'), + S = str_format_internal::FormatConversionCharToConvInt('S'), + // integer + d = str_format_internal::FormatConversionCharToConvInt('d'), + i = str_format_internal::FormatConversionCharToConvInt('i'), + o = str_format_internal::FormatConversionCharToConvInt('o'), + u = str_format_internal::FormatConversionCharToConvInt('u'), + x = str_format_internal::FormatConversionCharToConvInt('x'), + X = str_format_internal::FormatConversionCharToConvInt('X'), + // Float + f = str_format_internal::FormatConversionCharToConvInt('f'), + F = str_format_internal::FormatConversionCharToConvInt('F'), + e = str_format_internal::FormatConversionCharToConvInt('e'), + E = str_format_internal::FormatConversionCharToConvInt('E'), + g = str_format_internal::FormatConversionCharToConvInt('g'), + G = str_format_internal::FormatConversionCharToConvInt('G'), + a = str_format_internal::FormatConversionCharToConvInt('a'), + A = str_format_internal::FormatConversionCharToConvInt('A'), + // misc + n = str_format_internal::FormatConversionCharToConvInt('n'), + p = str_format_internal::FormatConversionCharToConvInt('p'), + + // Used for width/precision '*' specification. + kStar = str_format_internal::FormatConversionCharToConvInt('*'), + + // Some predefined values: + kIntegral = d | i | u | o | x | X, + kFloating = a | e | f | g | A | E | F | G, + kNumeric = kIntegral | kFloating, + kString = s, + kPointer = p, +}; } // namespace str_format_internal diff --git a/absl/strings/internal/str_format/extension_test.cc b/absl/strings/internal/str_format/extension_test.cc index 561eaa36b..0a023f9c0 100644 --- a/absl/strings/internal/str_format/extension_test.cc +++ b/absl/strings/internal/str_format/extension_test.cc @@ -80,13 +80,4 @@ TEST(FormatExtensionTest, SinkAppendChars) { EXPECT_EQ(actual, expected); } } - -TEST(FormatExtensionTest, CustomSink) { - my_namespace::UserDefinedType sink; - absl::Format(&sink, "There were %04d little %s.", 3, "pigs"); - EXPECT_EQ("There were 0003 little pigs.", sink.Value()); - absl::Format(&sink, "And %-3llx bad wolf!", 1); - EXPECT_EQ("There were 0003 little pigs.And 1 bad wolf!", sink.Value()); -} - } // namespace diff --git a/absl/strings/internal/str_format/float_conversion.cc b/absl/strings/internal/str_format/float_conversion.cc index d5a1ee40b..d6858cfff 100644 --- a/absl/strings/internal/str_format/float_conversion.cc +++ b/absl/strings/internal/str_format/float_conversion.cc @@ -403,70 +403,62 @@ bool FloatToSink(const Float v, const ConversionSpec &conv, Buffer buffer; - switch (conv.conversion_char()) { - case ConversionChar::f: - case ConversionChar::F: - if (!FloatToBuffer(decomposed, precision, &buffer, - nullptr)) { - return FallbackToSnprintf(v, conv, sink); - } - if (!conv.has_alt_flag() && buffer.back() == '.') buffer.pop_back(); - break; + FormatConversionChar c = conv.conversion_char(); - case ConversionChar::e: - case ConversionChar::E: - if (!FloatToBuffer(decomposed, precision, &buffer, - &exp)) { - return FallbackToSnprintf(v, conv, sink); + if (c == FormatConversionCharInternal::f || + c == FormatConversionCharInternal::F) { + if (!FloatToBuffer(decomposed, precision, &buffer, + nullptr)) { + return FallbackToSnprintf(v, conv, sink); + } + if (!conv.has_alt_flag() && buffer.back() == '.') buffer.pop_back(); + } else if (c == FormatConversionCharInternal::e || + c == FormatConversionCharInternal::E) { + if (!FloatToBuffer(decomposed, precision, &buffer, + &exp)) { + return FallbackToSnprintf(v, conv, sink); + } + if (!conv.has_alt_flag() && buffer.back() == '.') buffer.pop_back(); + PrintExponent( + exp, FormatConversionCharIsUpper(conv.conversion_char()) ? 'E' : 'e', + &buffer); + } else if (c == FormatConversionCharInternal::g || + c == FormatConversionCharInternal::G) { + precision = std::max(0, precision - 1); + if (!FloatToBuffer(decomposed, precision, &buffer, + &exp)) { + return FallbackToSnprintf(v, conv, sink); + } + if (precision + 1 > exp && exp >= -4) { + if (exp < 0) { + // Have 1.23456, needs 0.00123456 + // Move the first digit + buffer.begin[1] = *buffer.begin; + // Add some zeros + for (; exp < -1; ++exp) *buffer.begin-- = '0'; + *buffer.begin-- = '.'; + *buffer.begin = '0'; + } else if (exp > 0) { + // Have 1.23456, needs 1234.56 + // Move the '.' exp positions to the right. + std::rotate(buffer.begin + 1, buffer.begin + 2, buffer.begin + exp + 2); } - if (!conv.has_alt_flag() && buffer.back() == '.') buffer.pop_back(); + exp = 0; + } + if (!conv.has_alt_flag()) { + while (buffer.back() == '0') buffer.pop_back(); + if (buffer.back() == '.') buffer.pop_back(); + } + if (exp) { PrintExponent( exp, FormatConversionCharIsUpper(conv.conversion_char()) ? 'E' : 'e', &buffer); - break; - - case ConversionChar::g: - case ConversionChar::G: - precision = std::max(0, precision - 1); - if (!FloatToBuffer(decomposed, precision, &buffer, - &exp)) { - return FallbackToSnprintf(v, conv, sink); - } - if (precision + 1 > exp && exp >= -4) { - if (exp < 0) { - // Have 1.23456, needs 0.00123456 - // Move the first digit - buffer.begin[1] = *buffer.begin; - // Add some zeros - for (; exp < -1; ++exp) *buffer.begin-- = '0'; - *buffer.begin-- = '.'; - *buffer.begin = '0'; - } else if (exp > 0) { - // Have 1.23456, needs 1234.56 - // Move the '.' exp positions to the right. - std::rotate(buffer.begin + 1, buffer.begin + 2, - buffer.begin + exp + 2); - } - exp = 0; - } - if (!conv.has_alt_flag()) { - while (buffer.back() == '0') buffer.pop_back(); - if (buffer.back() == '.') buffer.pop_back(); - } - if (exp) { - PrintExponent( - exp, - FormatConversionCharIsUpper(conv.conversion_char()) ? 'E' : 'e', - &buffer); - } - break; - - case ConversionChar::a: - case ConversionChar::A: - return FallbackToSnprintf(v, conv, sink); - - default: - return false; + } + } else if (c == FormatConversionCharInternal::a || + c == FormatConversionCharInternal::A) { + return FallbackToSnprintf(v, conv, sink); + } else { + return false; } WriteBufferToSink(sign_char, diff --git a/absl/strings/internal/str_format/parser.cc b/absl/strings/internal/str_format/parser.cc index aab68db94..61132739b 100644 --- a/absl/strings/internal/str_format/parser.cc +++ b/absl/strings/internal/str_format/parser.cc @@ -17,7 +17,7 @@ namespace absl { ABSL_NAMESPACE_BEGIN namespace str_format_internal { -using CC = ConversionChar; +using CC = FormatConversionCharInternal; using LM = LengthMod; ABSL_CONST_INIT const ConvTag kTags[256] = { @@ -296,15 +296,17 @@ struct ParsedFormatBase::ParsedFormatConsumer { char* data_pos; }; -ParsedFormatBase::ParsedFormatBase(string_view format, bool allow_ignored, - std::initializer_list convs) +ParsedFormatBase::ParsedFormatBase( + string_view format, bool allow_ignored, + std::initializer_list convs) : data_(format.empty() ? nullptr : new char[format.size()]) { has_error_ = !ParseFormatString(format, ParsedFormatConsumer(this)) || !MatchesConversions(allow_ignored, convs); } bool ParsedFormatBase::MatchesConversions( - bool allow_ignored, std::initializer_list convs) const { + bool allow_ignored, + std::initializer_list convs) const { std::unordered_set used; auto add_if_valid_conv = [&](int pos, char c) { if (static_cast(pos) > convs.size() || diff --git a/absl/strings/internal/str_format/parser.h b/absl/strings/internal/str_format/parser.h index 7d9665172..fd2dc9704 100644 --- a/absl/strings/internal/str_format/parser.h +++ b/absl/strings/internal/str_format/parser.h @@ -67,7 +67,7 @@ struct UnboundConversion { Flags flags; LengthMod length_mod = LengthMod::none; - ConversionChar conv = FormatConversionChar::kNone; + FormatConversionChar conv = FormatConversionCharInternal::kNone; }; // Consume conversion spec prefix (not including '%') of [p, end) if valid. @@ -186,8 +186,9 @@ constexpr bool EnsureConstexpr(string_view s) { class ParsedFormatBase { public: - explicit ParsedFormatBase(string_view format, bool allow_ignored, - std::initializer_list convs); + explicit ParsedFormatBase( + string_view format, bool allow_ignored, + std::initializer_list convs); ParsedFormatBase(const ParsedFormatBase& other) { *this = other; } @@ -234,8 +235,9 @@ class ParsedFormatBase { private: // Returns whether the conversions match and if !allow_ignored it verifies // that all conversions are used by the format. - bool MatchesConversions(bool allow_ignored, - std::initializer_list convs) const; + bool MatchesConversions( + bool allow_ignored, + std::initializer_list convs) const; struct ParsedFormatConsumer; diff --git a/absl/strings/internal/str_format/parser_test.cc b/absl/strings/internal/str_format/parser_test.cc index 51eb53f5a..26f5bec6b 100644 --- a/absl/strings/internal/str_format/parser_test.cc +++ b/absl/strings/internal/str_format/parser_test.cc @@ -46,13 +46,13 @@ TEST(ConversionCharTest, Names) { }; // clang-format off const Expectation kExpect[] = { -#define X(c) {ConversionChar::c, #c[0]} +#define X(c) {FormatConversionCharInternal::c, #c[0]} X(c), X(C), X(s), X(S), // text X(d), X(i), X(o), X(u), X(x), X(X), // int X(f), X(F), X(e), X(E), X(g), X(G), X(a), X(A), // float X(n), X(p), // misc #undef X - {ConversionChar::kNone, '\0'}, + {FormatConversionCharInternal::kNone, '\0'}, }; // clang-format on for (auto e : kExpect) { @@ -349,7 +349,8 @@ TEST_F(ParsedFormatTest, ValueSemantics) { ParsedFormatBase p2 = p1; // copy construct (empty) EXPECT_EQ(SummarizeParsedFormat(p1), SummarizeParsedFormat(p2)); - p1 = ParsedFormatBase("hello%s", true, {Conv::s}); // move assign + p1 = ParsedFormatBase("hello%s", true, + {FormatConversionCharSetInternal::s}); // move assign EXPECT_EQ("[hello]{s:1$s}", SummarizeParsedFormat(p1)); ParsedFormatBase p3 = p1; // copy construct (nonempty) @@ -377,9 +378,9 @@ TEST_F(ParsedFormatTest, Parsing) { const ExpectParse kExpect[] = { {"", {}, ""}, {"ab", {}, "[ab]"}, - {"a%d", {Conv::d}, "[a]{d:1$d}"}, - {"a%+d", {Conv::d}, "[a]{+d:1$d}"}, - {"a% d", {Conv::d}, "[a]{ d:1$d}"}, + {"a%d", {FormatConversionCharSetInternal::d}, "[a]{d:1$d}"}, + {"a%+d", {FormatConversionCharSetInternal::d}, "[a]{+d:1$d}"}, + {"a% d", {FormatConversionCharSetInternal::d}, "[a]{ d:1$d}"}, {"a%b %d", {}, "[a]!"}, // stop after error }; for (const auto& e : kExpect) { @@ -391,13 +392,13 @@ TEST_F(ParsedFormatTest, Parsing) { TEST_F(ParsedFormatTest, ParsingFlagOrder) { const ExpectParse kExpect[] = { - {"a%+ 0d", {Conv::d}, "[a]{+ 0d:1$d}"}, - {"a%+0 d", {Conv::d}, "[a]{+0 d:1$d}"}, - {"a%0+ d", {Conv::d}, "[a]{0+ d:1$d}"}, - {"a% +0d", {Conv::d}, "[a]{ +0d:1$d}"}, - {"a%0 +d", {Conv::d}, "[a]{0 +d:1$d}"}, - {"a% 0+d", {Conv::d}, "[a]{ 0+d:1$d}"}, - {"a%+ 0+d", {Conv::d}, "[a]{+ 0+d:1$d}"}, + {"a%+ 0d", {FormatConversionCharSetInternal::d}, "[a]{+ 0d:1$d}"}, + {"a%+0 d", {FormatConversionCharSetInternal::d}, "[a]{+0 d:1$d}"}, + {"a%0+ d", {FormatConversionCharSetInternal::d}, "[a]{0+ d:1$d}"}, + {"a% +0d", {FormatConversionCharSetInternal::d}, "[a]{ +0d:1$d}"}, + {"a%0 +d", {FormatConversionCharSetInternal::d}, "[a]{0 +d:1$d}"}, + {"a% 0+d", {FormatConversionCharSetInternal::d}, "[a]{ 0+d:1$d}"}, + {"a%+ 0+d", {FormatConversionCharSetInternal::d}, "[a]{+ 0+d:1$d}"}, }; for (const auto& e : kExpect) { SCOPED_TRACE(e.in); diff --git a/absl/strings/str_format_test.cc b/absl/strings/str_format_test.cc index f0d1f0ad2..160f4c618 100644 --- a/absl/strings/str_format_test.cc +++ b/absl/strings/str_format_test.cc @@ -450,7 +450,7 @@ struct SummarizeConsumer { if (conv.precision.is_from_arg()) { *out += "." + std::to_string(conv.precision.get_from_arg()) + "$*"; } - *out += FormatConversionCharToChar(conv.conv); + *out += str_format_internal::FormatConversionCharToChar(conv.conv); *out += "}"; return true; } diff --git a/absl/synchronization/internal/create_thread_identity.cc b/absl/synchronization/internal/create_thread_identity.cc index fa0070a9f..53a71b342 100644 --- a/absl/synchronization/internal/create_thread_identity.cc +++ b/absl/synchronization/internal/create_thread_identity.cc @@ -32,9 +32,9 @@ namespace synchronization_internal { // ThreadIdentity storage is persistent, we maintain a free-list of previously // released ThreadIdentity objects. -static base_internal::SpinLock freelist_lock( - base_internal::kLinkerInitialized); -static base_internal::ThreadIdentity* thread_identity_freelist; +ABSL_CONST_INIT static base_internal::SpinLock freelist_lock( + absl::kConstInit, base_internal::SCHEDULE_KERNEL_ONLY); +ABSL_CONST_INIT static base_internal::ThreadIdentity* thread_identity_freelist; // A per-thread destructor for reclaiming associated ThreadIdentity objects. // Since we must preserve their storage we cache them for re-use. diff --git a/absl/synchronization/internal/graphcycles.cc b/absl/synchronization/internal/graphcycles.cc index 6a2bcdf68..19f9aab5b 100644 --- a/absl/synchronization/internal/graphcycles.cc +++ b/absl/synchronization/internal/graphcycles.cc @@ -51,9 +51,9 @@ namespace { // Avoid LowLevelAlloc's default arena since it calls malloc hooks in // which people are doing things like acquiring Mutexes. -static absl::base_internal::SpinLock arena_mu( - absl::base_internal::kLinkerInitialized); -static base_internal::LowLevelAlloc::Arena* arena; +ABSL_CONST_INIT static absl::base_internal::SpinLock arena_mu( + absl::kConstInit, base_internal::SCHEDULE_KERNEL_ONLY); +ABSL_CONST_INIT static base_internal::LowLevelAlloc::Arena* arena; static void InitArenaIfNecessary() { arena_mu.Lock(); diff --git a/absl/synchronization/mutex.cc b/absl/synchronization/mutex.cc index 8cda5a1ce..1f8a696e0 100644 --- a/absl/synchronization/mutex.cc +++ b/absl/synchronization/mutex.cc @@ -207,12 +207,12 @@ static void AtomicClearBits(std::atomic* pv, intptr_t bits, //------------------------------------------------------------------ // Data for doing deadlock detection. -static absl::base_internal::SpinLock deadlock_graph_mu( - absl::base_internal::kLinkerInitialized); +ABSL_CONST_INIT static absl::base_internal::SpinLock deadlock_graph_mu( + absl::kConstInit, base_internal::SCHEDULE_KERNEL_ONLY); -// graph used to detect deadlocks. -static GraphCycles *deadlock_graph ABSL_GUARDED_BY(deadlock_graph_mu) - ABSL_PT_GUARDED_BY(deadlock_graph_mu); +// Graph used to detect deadlocks. +ABSL_CONST_INIT static GraphCycles *deadlock_graph + ABSL_GUARDED_BY(deadlock_graph_mu) ABSL_PT_GUARDED_BY(deadlock_graph_mu); //------------------------------------------------------------------ // An event mechanism for debugging mutex use. @@ -273,13 +273,12 @@ static const struct { {0, "SignalAll on "}, }; -static absl::base_internal::SpinLock synch_event_mu( - absl::base_internal::kLinkerInitialized); -// protects synch_event +ABSL_CONST_INIT static absl::base_internal::SpinLock synch_event_mu( + absl::kConstInit, base_internal::SCHEDULE_KERNEL_ONLY); // Hash table size; should be prime > 2. // Can't be too small, as it's used for deadlock detection information. -static const uint32_t kNSynchEvent = 1031; +static constexpr uint32_t kNSynchEvent = 1031; static struct SynchEvent { // this is a trivial hash table for the events // struct is freed when refcount reaches 0 diff --git a/absl/time/clock.cc b/absl/time/clock.cc index 3b895c38e..e5c423c7e 100644 --- a/absl/time/clock.cc +++ b/absl/time/clock.cc @@ -226,9 +226,9 @@ static_assert(((kMinNSBetweenSamples << (kScale + 1)) >> (kScale + 1)) == // A reader-writer lock protecting the static locations below. // See SeqAcquire() and SeqRelease() above. -static absl::base_internal::SpinLock lock( - absl::base_internal::kLinkerInitialized); -static std::atomic seq(0); +ABSL_CONST_INIT static absl::base_internal::SpinLock lock( + absl::kConstInit, base_internal::SCHEDULE_KERNEL_ONLY); +ABSL_CONST_INIT static std::atomic seq(0); // data from a sample of the kernel's time value struct TimeSampleAtomic { diff --git a/absl/time/internal/cctz/src/cctz_benchmark.cc b/absl/time/internal/cctz/src/cctz_benchmark.cc index d30a644e0..a402760d1 100644 --- a/absl/time/internal/cctz/src/cctz_benchmark.cc +++ b/absl/time/internal/cctz/src/cctz_benchmark.cc @@ -280,6 +280,7 @@ const char* const kTimeZoneNames[] = {"Africa/Abidjan", "America/North_Dakota/Beulah", "America/North_Dakota/Center", "America/North_Dakota/New_Salem", + "America/Nuuk", "America/Ojinaga", "America/Panama", "America/Pangnirtung", diff --git a/absl/time/internal/cctz/src/time_zone_lookup_test.cc b/absl/time/internal/cctz/src/time_zone_lookup_test.cc index 35911ce56..0b0c1a3b7 100644 --- a/absl/time/internal/cctz/src/time_zone_lookup_test.cc +++ b/absl/time/internal/cctz/src/time_zone_lookup_test.cc @@ -211,6 +211,7 @@ const char* const kTimeZoneNames[] = {"Africa/Abidjan", "America/North_Dakota/Beulah", "America/North_Dakota/Center", "America/North_Dakota/New_Salem", + "America/Nuuk", "America/Ojinaga", "America/Panama", "America/Pangnirtung",