diff options
Diffstat (limited to 'absl')
31 files changed, 1152 insertions, 649 deletions
diff --git a/absl/base/spinlock_test_common.cc b/absl/base/spinlock_test_common.cc index e62b2eaeba7b..84fc4dac0be7 100644 --- a/absl/base/spinlock_test_common.cc +++ b/absl/base/spinlock_test_common.cc @@ -62,7 +62,6 @@ static SpinLock static_cooperative_spinlock( static SpinLock static_noncooperative_spinlock( base_internal::kLinkerInitialized, base_internal::SCHEDULE_KERNEL_ONLY); - // Simple integer hash function based on the public domain lookup2 hash. // http://burtleburtle.net/bob/c/lookup2.c static uint32_t Hash32(uint32_t a, uint32_t c) { @@ -194,6 +193,7 @@ TEST(SpinLock, WaitCyclesEncoding) { TEST(SpinLockWithThreads, StaticSpinLock) { ThreadedTest(&static_spinlock); } + TEST(SpinLockWithThreads, StackSpinLock) { SpinLock spinlock; ThreadedTest(&spinlock); diff --git a/absl/container/BUILD.bazel b/absl/container/BUILD.bazel index 17d725c18ba9..b8cf17fc6072 100644 --- a/absl/container/BUILD.bazel +++ b/absl/container/BUILD.bazel @@ -44,7 +44,10 @@ cc_test( linkopts = ABSL_DEFAULT_LINKOPTS, deps = [ ":compressed_tuple", + ":test_instance_tracker", "//absl/memory", + "//absl/types:any", + "//absl/types:optional", "//absl/utility", "@com_google_googletest//:gtest_main", ], diff --git a/absl/container/CMakeLists.txt b/absl/container/CMakeLists.txt index 6df331e17165..5d7c75dd59cd 100644 --- a/absl/container/CMakeLists.txt +++ b/absl/container/CMakeLists.txt @@ -43,8 +43,11 @@ absl_cc_test( COPTS ${ABSL_TEST_COPTS} DEPS + absl::any absl::compressed_tuple absl::memory + absl::optional + absl::test_instance_tracker absl::utility gmock_main ) diff --git a/absl/container/internal/common.h b/absl/container/internal/common.h index b06e71131801..591d3ea11fa2 100644 --- a/absl/container/internal/common.h +++ b/absl/container/internal/common.h @@ -77,10 +77,18 @@ class node_handle_base { protected: friend struct CommonAccess; - node_handle_base(const allocator_type& a, slot_type* s) : alloc_(a) { + struct transfer_tag_t {}; + node_handle_base(transfer_tag_t, const allocator_type& a, slot_type* s) + : alloc_(a) { PolicyTraits::transfer(alloc(), slot(), s); } + struct move_tag_t {}; + node_handle_base(move_tag_t, const allocator_type& a, slot_type* s) + : alloc_(a) { + PolicyTraits::construct(alloc(), slot(), s); + } + void destroy() { if (!empty()) { PolicyTraits::destroy(alloc(), slot()); @@ -121,7 +129,7 @@ class node_handle : public node_handle_base<PolicyTraits, Alloc> { private: friend struct CommonAccess; - node_handle(const Alloc& a, typename Base::slot_type* s) : Base(a, s) {} + using Base::Base; }; // For maps. @@ -148,7 +156,7 @@ class node_handle<Policy, PolicyTraits, Alloc, private: friend struct CommonAccess; - node_handle(const Alloc& a, typename Base::slot_type* s) : Base(a, s) {} + using Base::Base; }; // Provide access to non-public node-handle functions. @@ -164,8 +172,13 @@ struct CommonAccess { } template <typename T, typename... Args> - static T Make(Args&&... args) { - return T(std::forward<Args>(args)...); + static T Transfer(Args&&... args) { + return T(typename T::transfer_tag_t{}, std::forward<Args>(args)...); + } + + template <typename T, typename... Args> + static T Move(Args&&... args) { + return T(typename T::move_tag_t{}, std::forward<Args>(args)...); } }; diff --git a/absl/container/internal/compressed_tuple.h b/absl/container/internal/compressed_tuple.h index c29ab41eb9fd..7d08e370dec1 100644 --- a/absl/container/internal/compressed_tuple.h +++ b/absl/container/internal/compressed_tuple.h @@ -102,7 +102,9 @@ template <typename T, size_t I, struct Storage { T value; constexpr Storage() = default; - explicit constexpr Storage(T&& v) : value(absl::forward<T>(v)) {} + template <typename V> + explicit constexpr Storage(absl::in_place_t, V&& v) + : value(absl::forward<V>(v)) {} constexpr const T& get() const& { return value; } T& get() & { return value; } constexpr const T&& get() const&& { return absl::move(*this).value; } @@ -112,7 +114,11 @@ struct Storage { template <typename T, size_t I> struct ABSL_INTERNAL_COMPRESSED_TUPLE_DECLSPEC Storage<T, I, true> : T { constexpr Storage() = default; - explicit constexpr Storage(T&& v) : T(absl::forward<T>(v)) {} + + template <typename V> + explicit constexpr Storage(absl::in_place_t, V&& v) + : T(absl::forward<V>(v)) {} + constexpr const T& get() const& { return *this; } T& get() & { return *this; } constexpr const T&& get() const&& { return absl::move(*this); } @@ -132,8 +138,9 @@ struct ABSL_INTERNAL_COMPRESSED_TUPLE_DECLSPEC CompressedTupleImpl< : uses_inheritance, Storage<Ts, std::integral_constant<size_t, I>::value>... { constexpr CompressedTupleImpl() = default; - explicit constexpr CompressedTupleImpl(Ts&&... args) - : Storage<Ts, I>(absl::forward<Ts>(args))... {} + template <typename... Vs> + explicit constexpr CompressedTupleImpl(absl::in_place_t, Vs&&... args) + : Storage<Ts, I>(absl::in_place, absl::forward<Vs>(args))... {} friend CompressedTuple<Ts...>; }; @@ -143,8 +150,9 @@ struct ABSL_INTERNAL_COMPRESSED_TUPLE_DECLSPEC CompressedTupleImpl< // We use the dummy identity function as above... : Storage<Ts, std::integral_constant<size_t, I>::value, false>... { constexpr CompressedTupleImpl() = default; - explicit constexpr CompressedTupleImpl(Ts&&... args) - : Storage<Ts, I, false>(absl::forward<Ts>(args))... {} + template <typename... Vs> + explicit constexpr CompressedTupleImpl(absl::in_place_t, Vs&&... args) + : Storage<Ts, I, false>(absl::in_place, absl::forward<Vs>(args))... {} friend CompressedTuple<Ts...>; }; @@ -159,6 +167,11 @@ constexpr bool ShouldAnyUseBase() { Or({std::integral_constant<bool, ShouldUseBase<Ts>()>()...})){}; } +template <typename T, typename V> +using TupleMoveConstructible = typename std::conditional< + std::is_reference<T>::value, std::is_convertible<V, T>, + std::is_constructible<T, V&&>>::type; + } // namespace internal_compressed_tuple // Helper class to perform the Empty Base Class Optimization. @@ -192,9 +205,29 @@ class ABSL_INTERNAL_COMPRESSED_TUPLE_DECLSPEC CompressedTuple using StorageT = internal_compressed_tuple::Storage<ElemT<I>, I>; public: + // There seems to be a bug in MSVC dealing in which using '=default' here will + // cause the compiler to ignore the body of other constructors. The work- + // around is to explicitly implement the default constructor. +#if defined(_MSC_VER) + constexpr CompressedTuple() : CompressedTuple::CompressedTupleImpl() {} +#else constexpr CompressedTuple() = default; - explicit constexpr CompressedTuple(Ts... base) - : CompressedTuple::CompressedTupleImpl(absl::forward<Ts>(base)...) {} +#endif + explicit constexpr CompressedTuple(const Ts&... base) + : CompressedTuple::CompressedTupleImpl(absl::in_place, base...) {} + + template <typename... Vs, + absl::enable_if_t< + absl::conjunction< + // Ensure we are not hiding default copy/move constructors. + absl::negation<std::is_same<void(CompressedTuple), + void(absl::decay_t<Vs>...)>>, + internal_compressed_tuple::TupleMoveConstructible< + Ts, Vs&&>...>::value, + bool> = true> + explicit constexpr CompressedTuple(Vs&&... base) + : CompressedTuple::CompressedTupleImpl(absl::in_place, + absl::forward<Vs>(base)...) {} template <int I> ElemT<I>& get() & { diff --git a/absl/container/internal/compressed_tuple_test.cc b/absl/container/internal/compressed_tuple_test.cc index 3b0ec4555abf..19af8f10aa6c 100644 --- a/absl/container/internal/compressed_tuple_test.cc +++ b/absl/container/internal/compressed_tuple_test.cc @@ -19,7 +19,10 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "absl/container/internal/test_instance_tracker.h" #include "absl/memory/memory.h" +#include "absl/types/any.h" +#include "absl/types/optional.h" #include "absl/utility/utility.h" // These are declared at global scope purely so that error messages @@ -43,10 +46,14 @@ struct TwoValues { U value2; }; + namespace absl { namespace container_internal { namespace { +using absl::test_internal::CopyableMovableInstance; +using absl::test_internal::InstanceTracker; + TEST(CompressedTupleTest, Sizeof) { EXPECT_EQ(sizeof(int), sizeof(CompressedTuple<int>)); EXPECT_EQ(sizeof(int), sizeof(CompressedTuple<int, Empty<0>>)); @@ -62,6 +69,141 @@ TEST(CompressedTupleTest, Sizeof) { sizeof(CompressedTuple<int, Empty<0>, NotEmpty<double>, Empty<1>>)); } +TEST(CompressedTupleTest, OneMoveOnRValueConstructionTemp) { + InstanceTracker tracker; + CompressedTuple<CopyableMovableInstance> x1(CopyableMovableInstance(1)); + EXPECT_EQ(tracker.instances(), 1); + EXPECT_EQ(tracker.copies(), 0); + EXPECT_LE(tracker.moves(), 1); + EXPECT_EQ(x1.get<0>().value(), 1); +} + +TEST(CompressedTupleTest, OneMoveOnRValueConstructionMove) { + InstanceTracker tracker; + + CopyableMovableInstance i1(1); + CompressedTuple<CopyableMovableInstance> x1(std::move(i1)); + EXPECT_EQ(tracker.instances(), 2); + EXPECT_EQ(tracker.copies(), 0); + EXPECT_LE(tracker.moves(), 1); + EXPECT_EQ(x1.get<0>().value(), 1); +} + +TEST(CompressedTupleTest, OneMoveOnRValueConstructionMixedTypes) { + InstanceTracker tracker; + CopyableMovableInstance i1(1); + CopyableMovableInstance i2(2); + Empty<0> empty; + CompressedTuple<CopyableMovableInstance, CopyableMovableInstance&, Empty<0>> + x1(std::move(i1), i2, empty); + EXPECT_EQ(x1.get<0>().value(), 1); + EXPECT_EQ(x1.get<1>().value(), 2); + EXPECT_EQ(tracker.copies(), 0); + EXPECT_EQ(tracker.moves(), 1); +} + +struct IncompleteType; +CompressedTuple<CopyableMovableInstance, IncompleteType&, Empty<0>> +MakeWithIncomplete(CopyableMovableInstance i1, + IncompleteType& t, // NOLINT + Empty<0> empty) { + return CompressedTuple<CopyableMovableInstance, IncompleteType&, Empty<0>>{ + std::move(i1), t, empty}; +} + +struct IncompleteType {}; +TEST(CompressedTupleTest, OneMoveOnRValueConstructionWithIncompleteType) { + InstanceTracker tracker; + CopyableMovableInstance i1(1); + Empty<0> empty; + struct DerivedType : IncompleteType {int value = 0;}; + DerivedType fd; + fd.value = 7; + + CompressedTuple<CopyableMovableInstance, IncompleteType&, Empty<0>> x1 = + MakeWithIncomplete(std::move(i1), fd, empty); + + EXPECT_EQ(x1.get<0>().value(), 1); + EXPECT_EQ(static_cast<DerivedType&>(x1.get<1>()).value, 7); + + EXPECT_EQ(tracker.copies(), 0); + EXPECT_EQ(tracker.moves(), 2); +} + +TEST(CompressedTupleTest, + OneMoveOnRValueConstructionMixedTypes_BraceInitPoisonPillExpected) { + InstanceTracker tracker; + CopyableMovableInstance i1(1); + CopyableMovableInstance i2(2); + CompressedTuple<CopyableMovableInstance, CopyableMovableInstance&, Empty<0>> + x1(std::move(i1), i2, {}); // NOLINT + EXPECT_EQ(x1.get<0>().value(), 1); + EXPECT_EQ(x1.get<1>().value(), 2); + EXPECT_EQ(tracker.instances(), 3); + // We are forced into the `const Ts&...` constructor (invoking copies) + // because we need it to deduce the type of `{}`. + // std::tuple also has this behavior. + // Note, this test is proof that this is expected behavior, but it is not + // _desired_ behavior. + EXPECT_EQ(tracker.copies(), 1); + EXPECT_EQ(tracker.moves(), 0); +} + +TEST(CompressedTupleTest, OneCopyOnLValueConstruction) { + InstanceTracker tracker; + CopyableMovableInstance i1(1); + + CompressedTuple<CopyableMovableInstance> x1(i1); + EXPECT_EQ(tracker.copies(), 1); + EXPECT_EQ(tracker.moves(), 0); + + tracker.ResetCopiesMovesSwaps(); + + CopyableMovableInstance i2(2); + const CopyableMovableInstance& i2_ref = i2; + CompressedTuple<CopyableMovableInstance> x2(i2_ref); + EXPECT_EQ(tracker.copies(), 1); + EXPECT_EQ(tracker.moves(), 0); +} + +TEST(CompressedTupleTest, OneMoveOnRValueAccess) { + InstanceTracker tracker; + CopyableMovableInstance i1(1); + CompressedTuple<CopyableMovableInstance> x(std::move(i1)); + tracker.ResetCopiesMovesSwaps(); + + CopyableMovableInstance i2 = std::move(x).get<0>(); + EXPECT_EQ(tracker.copies(), 0); + EXPECT_EQ(tracker.moves(), 1); +} + +TEST(CompressedTupleTest, OneCopyOnLValueAccess) { + InstanceTracker tracker; + + CompressedTuple<CopyableMovableInstance> x(CopyableMovableInstance(0)); + EXPECT_EQ(tracker.copies(), 0); + EXPECT_EQ(tracker.moves(), 1); + + CopyableMovableInstance t = x.get<0>(); + EXPECT_EQ(tracker.copies(), 1); + EXPECT_EQ(tracker.moves(), 1); +} + +TEST(CompressedTupleTest, ZeroCopyOnRefAccess) { + InstanceTracker tracker; + + CompressedTuple<CopyableMovableInstance> x(CopyableMovableInstance(0)); + EXPECT_EQ(tracker.copies(), 0); + EXPECT_EQ(tracker.moves(), 1); + + CopyableMovableInstance& t1 = x.get<0>(); + const CopyableMovableInstance& t2 = x.get<0>(); + EXPECT_EQ(tracker.copies(), 0); + EXPECT_EQ(tracker.moves(), 1); + EXPECT_EQ(t1.value(), 0); + EXPECT_EQ(t2.value(), 0); +} + TEST(CompressedTupleTest, Access) { struct S { std::string x; @@ -173,7 +315,40 @@ TEST(CompressedTupleTest, MoveOnlyElements) { EXPECT_EQ(*x1, 5); } +TEST(CompressedTupleTest, MoveConstructionMoveOnlyElements) { + CompressedTuple<std::unique_ptr<std::string>> base( + absl::make_unique<std::string>("str")); + EXPECT_EQ(*base.get<0>(), "str"); + + CompressedTuple<std::unique_ptr<std::string>> copy(std::move(base)); + EXPECT_EQ(*copy.get<0>(), "str"); +} + +TEST(CompressedTupleTest, AnyElements) { + any a(std::string("str")); + CompressedTuple<any, any&> x(any(5), a); + EXPECT_EQ(absl::any_cast<int>(x.get<0>()), 5); + EXPECT_EQ(absl::any_cast<std::string>(x.get<1>()), "str"); + + a = 0.5f; + EXPECT_EQ(absl::any_cast<float>(x.get<1>()), 0.5); + + // Ensure copy construction work in the face of a type with a universal + // implicit constructor; + CompressedTuple<absl::any> c{}, d(c); // NOLINT +} + TEST(CompressedTupleTest, Constexpr) { + struct NonTrivialStruct { + constexpr NonTrivialStruct() = default; + constexpr int value() const { return v; } + int v = 5; + }; + struct TrivialStruct { + TrivialStruct() = default; + constexpr int value() const { return v; } + int v; + }; constexpr CompressedTuple<int, double, CompressedTuple<int>, Empty<0>> x( 7, 1.25, CompressedTuple<int>(5), {}); constexpr int x0 = x.get<0>(); @@ -186,6 +361,32 @@ TEST(CompressedTupleTest, Constexpr) { EXPECT_EQ(x2, 5); EXPECT_EQ(x3, CallType::kConstRef); +#if !defined(__GNUC__) || defined(__clang__) || __GNUC__ > 4 + constexpr CompressedTuple<Empty<0>, TrivialStruct, int> trivial = {}; + constexpr CallType trivial0 = trivial.get<0>().value(); + constexpr int trivial1 = trivial.get<1>().value(); + constexpr int trivial2 = trivial.get<2>(); + + EXPECT_EQ(trivial0, CallType::kConstRef); + EXPECT_EQ(trivial1, 0); + EXPECT_EQ(trivial2, 0); +#endif + + constexpr CompressedTuple<Empty<0>, NonTrivialStruct, absl::optional<int>> + non_trivial = {}; + constexpr CallType non_trivial0 = non_trivial.get<0>().value(); + constexpr int non_trivial1 = non_trivial.get<1>().value(); + constexpr absl::optional<int> non_trivial2 = non_trivial.get<2>(); + + EXPECT_EQ(non_trivial0, CallType::kConstRef); + EXPECT_EQ(non_trivial1, 5); + EXPECT_EQ(non_trivial2, absl::nullopt); + + static constexpr char data[] = "DEF"; + constexpr CompressedTuple<const char*> z(data); + constexpr const char* z1 = z.get<0>(); + EXPECT_EQ(std::string(z1), std::string(data)); + #if defined(__clang__) // An apparent bug in earlier versions of gcc claims these are ambiguous. constexpr int x2m = absl::move(x.get<2>()).get<0>(); diff --git a/absl/container/internal/hashtablez_sampler.cc b/absl/container/internal/hashtablez_sampler.cc index 6667d3ad847a..d03dd82e7fd5 100644 --- a/absl/container/internal/hashtablez_sampler.cc +++ b/absl/container/internal/hashtablez_sampler.cc @@ -32,7 +32,7 @@ constexpr int HashtablezInfo::kMaxStackDepth; namespace { ABSL_CONST_INIT std::atomic<bool> g_hashtablez_enabled{ - false + false }; ABSL_CONST_INIT std::atomic<int32_t> g_hashtablez_sample_parameter{1 << 10}; ABSL_CONST_INIT std::atomic<int32_t> g_hashtablez_max_samples{1 << 20}; diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index c4889cd47506..656e9806d899 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -1182,7 +1182,7 @@ class raw_hash_set { node_type extract(const_iterator position) { auto node = - CommonAccess::Make<node_type>(alloc_ref(), position.inner_.slot_); + CommonAccess::Transfer<node_type>(alloc_ref(), position.inner_.slot_); erase_meta_only(position); return node; } diff --git a/absl/container/internal/test_instance_tracker.h b/absl/container/internal/test_instance_tracker.h index 3d4b2980ea8b..c4731dbe3d1d 100644 --- a/absl/container/internal/test_instance_tracker.h +++ b/absl/container/internal/test_instance_tracker.h @@ -23,7 +23,7 @@ namespace absl { namespace test_internal { -// A type that counts number of occurences of the type, the live occurrences of +// A type that counts number of occurrences of the type, the live occurrences of // the type, as well as the number of copies, moves, swaps, and comparisons that // have occurred on the type. This is used as a base class for the copyable, // copyable+movable, and movable types below that are used in actual tests. Use diff --git a/absl/flags/BUILD.bazel b/absl/flags/BUILD.bazel index 82e6ffcfcef3..2fe61eaa8a8b 100644 --- a/absl/flags/BUILD.bazel +++ b/absl/flags/BUILD.bazel @@ -155,14 +155,12 @@ cc_library( ) cc_library( - name = "usage", + name = "usage_internal", srcs = [ "internal/usage.cc", - "usage.cc", ], hdrs = [ "internal/usage.h", - "usage.h", ], copts = ABSL_DEFAULT_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -180,6 +178,23 @@ cc_library( ) cc_library( + name = "usage", + srcs = [ + "usage.cc", + ], + hdrs = [ + "usage.h", + ], + copts = ABSL_DEFAULT_COPTS, + linkopts = ABSL_DEFAULT_LINKOPTS, + deps = [ + ":usage_internal", + "//absl/strings", + "//absl/synchronization", + ], +) + +cc_library( name = "parse", srcs = ["parse.cc"], hdrs = [ @@ -195,6 +210,7 @@ cc_library( ":internal", ":registry", ":usage", + ":usage_internal", "//absl/strings", "//absl/synchronization", ], @@ -360,6 +376,7 @@ cc_test( ":internal", ":parse", ":usage", + ":usage_internal", "//absl/memory", "//absl/strings", "@com_google_googletest//:gtest", diff --git a/absl/flags/CMakeLists.txt b/absl/flags/CMakeLists.txt index 9c936eccc8e3..fa1d4e177b3e 100644 --- a/absl/flags/CMakeLists.txt +++ b/absl/flags/CMakeLists.txt @@ -141,13 +141,11 @@ absl_cc_library( # Internal-only target, do not depend on directly. absl_cc_library( NAME - flags_usage + flags_usage_internal SRCS "internal/usage.cc" - "usage.cc" HDRS "internal/usage.h" - "usage.h" COPTS ${ABSL_DEFAULT_COPTS} LINKOPTS @@ -163,6 +161,23 @@ absl_cc_library( absl_cc_library( NAME + flags_usage + SRCS + "usage.cc" + HDRS + "usage.h" + COPTS + ${ABSL_DEFAULT_COPTS} + LINKOPTS + ${ABSL_DEFAULT_LINKOPTS} + DEPS + absl::flags_usage_internal + absl::strings + absl::synchronization +) + +absl_cc_library( + NAME flags_parse SRCS "parse.cc" diff --git a/absl/flags/flag.cc b/absl/flags/flag.cc index ba002edd24f1..f16cc75e3843 100644 --- a/absl/flags/flag.cc +++ b/absl/flags/flag.cc @@ -24,24 +24,16 @@ namespace absl { // so in debug builds we always use the slower implementation, which always // validates the type. #ifndef NDEBUG -#define ABSL_FLAGS_ATOMIC_GET(T) \ - T GetFlag(const absl::Flag<T>& flag) { \ - T result; \ - flag.internal.Read(&result, &flags_internal::FlagOps<T>); \ - return result; \ - } +#define ABSL_FLAGS_ATOMIC_GET(T) \ + T GetFlag(const absl::Flag<T>& flag) { return flag.Get(); } #else -#define ABSL_FLAGS_ATOMIC_GET(T) \ - T GetFlag(const absl::Flag<T>& flag) { \ - const int64_t r = flag.internal.atomic.load(std::memory_order_acquire); \ - if (r != flags_internal::CommandLineFlag::kAtomicInit) { \ - T t; \ - memcpy(&t, &r, sizeof(T)); \ - return t; \ - } \ - T result; \ - flag.internal.Read(&result, &flags_internal::FlagOps<T>); \ - return result; \ +#define ABSL_FLAGS_ATOMIC_GET(T) \ + T GetFlag(const absl::Flag<T>& flag) { \ + T result; \ + if (flag.AtomicGet(&result)) { \ + return result; \ + } \ + return flag.Get(); \ } #endif diff --git a/absl/flags/flag.h b/absl/flags/flag.h index 193755e56aa6..c70023aba8c7 100644 --- a/absl/flags/flag.h +++ b/absl/flags/flag.h @@ -91,30 +91,7 @@ T GetFlag(const absl::Flag<T>& flag) { ABSL_FLAGS_INTERNAL_FOR_EACH_LOCK_FREE(ABSL_FLAGS_INTERNAL_LOCK_FREE_VALIDATE) #undef ABSL_FLAGS_INTERNAL_LOCK_FREE_VALIDATE - // Implementation notes: - // - // We are wrapping a union around the value of `T` to serve three purposes: - // - // 1. `U.value` has correct size and alignment for a value of type `T` - // 2. The `U.value` constructor is not invoked since U's constructor does not - // do it explicitly. - // 3. The `U.value` destructor is invoked since U's destructor does it - // explicitly. This makes `U` a kind of RAII wrapper around non default - // constructible value of T, which is destructed when we leave the scope. - // We do need to destroy U.value, which is constructed by - // CommandLineFlag::Read even though we left it in a moved-from state - // after std::move. - // - // All of this serves to avoid requiring `T` being default constructible. - union U { - T value; - U() {} - ~U() { value.~T(); } - }; - U u; - - flag.internal.Read(&u.value, &flags_internal::FlagOps<T>); - return std::move(u.value); + return flag.Get(); } // Overload for `GetFlag()` for types that support lock-free reads. @@ -132,7 +109,7 @@ ABSL_FLAGS_INTERNAL_FOR_EACH_LOCK_FREE(ABSL_FLAGS_INTERNAL_LOCK_FREE_EXPORT) // but especially within performance-critical code. template <typename T> void SetFlag(absl::Flag<T>* flag, const T& v) { - flag->internal.Write(&v, &flags_internal::FlagOps<T>); + flag->Set(v); } // Overload of `SetFlag()` to allow callers to pass in a value that is @@ -141,7 +118,7 @@ void SetFlag(absl::Flag<T>* flag, const T& v) { template <typename T, typename V> void SetFlag(absl::Flag<T>* flag, const V& v) { T value(v); - SetFlag<T>(flag, value); + flag->Set(value); } } // namespace absl @@ -239,17 +216,17 @@ void SetFlag(absl::Flag<T>* flag, const V& v) { // Note: Name of registrar object is not arbitrary. It is used to "grab" // global name for FLAGS_no<flag_name> symbol, thus preventing the possibility // of defining two flags with names foo and nofoo. -#define ABSL_FLAG_IMPL(Type, name, default_value, help) \ - namespace absl {} \ - ABSL_FLAG_IMPL_DECLARE_DEF_VAL_WRAPPER(name, Type, default_value) \ - ABSL_FLAG_IMPL_DECLARE_HELP_WRAPPER(name, help) \ - absl::Flag<Type> FLAGS_##name( \ - ABSL_FLAG_IMPL_FLAGNAME(#name), \ - &AbslFlagsWrapHelp##name, \ - ABSL_FLAG_IMPL_FILENAME(), \ - &absl::flags_internal::FlagMarshallingOps<Type>, \ - &AbslFlagsInitFlag##name); \ - extern bool FLAGS_no##name; \ +#define ABSL_FLAG_IMPL(Type, name, default_value, help) \ + namespace absl {} \ + ABSL_FLAG_IMPL_DECLARE_DEF_VAL_WRAPPER(name, Type, default_value) \ + ABSL_FLAG_IMPL_DECLARE_HELP_WRAPPER(name, help) \ + absl::Flag<Type> FLAGS_##name( \ + ABSL_FLAG_IMPL_FLAGNAME(#name), \ + &AbslFlagsWrapHelp##name, \ + ABSL_FLAG_IMPL_FILENAME(), \ + &absl::flags_internal::FlagMarshallingOps<Type>, \ + &AbslFlagsInitFlag##name); \ + extern bool FLAGS_no##name; \ bool FLAGS_no##name = ABSL_FLAG_IMPL_REGISTRAR(Type, FLAGS_##name) // ABSL_RETIRED_FLAG diff --git a/absl/flags/internal/commandlineflag.cc b/absl/flags/internal/commandlineflag.cc index 744447219c71..98a46695304b 100644 --- a/absl/flags/internal/commandlineflag.cc +++ b/absl/flags/internal/commandlineflag.cc @@ -37,44 +37,6 @@ const char kStrippedFlagHelp[] = "\001\002\003\004 (unknown) \004\003\002\001"; namespace { -void StoreAtomic(CommandLineFlag* flag, const void* data, size_t size) { - int64_t t = 0; - assert(size <= sizeof(int64_t)); - memcpy(&t, data, size); - flag->atomic.store(t, std::memory_order_release); -} - -// 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(CommandLineFlag* flag, absl::Mutex* primary_lock) - EXCLUSIVE_LOCKS_REQUIRED(primary_lock) { - if (!flag->callback) return; - - // The callback lock is guaranteed initialized, because *primary_lock exists. - absl::Mutex* callback_mu = &flag->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. - primary_lock->Unlock(); - - { - absl::MutexLock lock(callback_mu); - - flag->callback(); - } - - primary_lock->Lock(); -} - // Currently we only validate flag values for user-defined flag types. bool ShouldValidateFlagValue(const CommandLineFlag& flag) { #define DONT_VALIDATE(T) \ @@ -89,145 +51,72 @@ bool ShouldValidateFlagValue(const CommandLineFlag& flag) { } // namespace -// Update any copy of the flag value that is stored in an atomic word. -// In addition if flag has a mutation callback this function invokes it. -void UpdateCopy(CommandLineFlag* flag, absl::Mutex* primary_lock) - EXCLUSIVE_LOCKS_REQUIRED(primary_lock) { -#define STORE_ATOMIC(T) \ - else if (flag->IsOfType<T>()) { \ - StoreAtomic(flag, flag->cur, sizeof(T)); \ - } - - if (false) { - } - ABSL_FLAGS_INTERNAL_FOR_EACH_LOCK_FREE(STORE_ATOMIC) -#undef STORE_ATOMIC +absl::Mutex* InitFlag(CommandLineFlag* flag) { + ABSL_CONST_INIT static absl::Mutex init_lock(absl::kConstInit); + absl::Mutex* mu; - InvokeCallback(flag, primary_lock); -} + { + absl::MutexLock lock(&init_lock); -// Ensure that the lazily initialized fields of *flag have been initialized, -// and return &flag->locks->primary_mu. -absl::Mutex* InitFlagIfNecessary(CommandLineFlag* flag) - LOCK_RETURNED(flag->locks->primary_mu) { - absl::Mutex* mu; - if (!flag->inited.load(std::memory_order_acquire)) { - // Need to initialize lazily initialized fields. - ABSL_CONST_INIT static absl::Mutex init_lock(absl::kConstInit); - init_lock.Lock(); if (flag->locks == nullptr) { // Must initialize Mutexes for this flag. flag->locks = new flags_internal::CommandLineFlagLocks; } + mu = &flag->locks->primary_mu; - init_lock.Unlock(); - mu->Lock(); - if (!flag->retired && - flag->def == nullptr) { // Need to initialize def and cur fields. + } + + { + absl::MutexLock lock(mu); + + if (!flag->retired && flag->def == nullptr) { + // Need to initialize def and cur fields. flag->def = (*flag->make_init_value)(); flag->cur = Clone(flag->op, flag->def); - UpdateCopy(flag, mu); + UpdateCopy(flag); + flag->inited.store(true, std::memory_order_release); + flag->InvokeCallback(); } - mu->Unlock(); - flag->inited.store(true, std::memory_order_release); - } else { // All fields initialized; flag->locks is therefore safe to read. - mu = &flag->locks->primary_mu; } + + flag->inited.store(true, std::memory_order_release); return mu; } -// Return true iff flag value was changed via direct-access. -bool ChangedDirectly(CommandLineFlag* flag, const void* a, const void* b) { - if (!flag->IsAbseilFlag()) { - // Need to compare values for direct-access flags. -#define CHANGED_FOR_TYPE(T) \ - if (flag->IsOfType<T>()) { \ - return *reinterpret_cast<const T*>(a) != *reinterpret_cast<const T*>(b); \ +// Ensure that the lazily initialized fields of *flag have been initialized, +// and return &flag->locks->primary_mu. +absl::Mutex* CommandLineFlag::InitFlagIfNecessary() const + LOCK_RETURNED(locks->primary_mu) { + if (!this->inited.load(std::memory_order_acquire)) { + return InitFlag(const_cast<CommandLineFlag*>(this)); } - CHANGED_FOR_TYPE(bool); - CHANGED_FOR_TYPE(int32_t); - CHANGED_FOR_TYPE(int64_t); - CHANGED_FOR_TYPE(uint64_t); - CHANGED_FOR_TYPE(double); - CHANGED_FOR_TYPE(std::string); -#undef CHANGED_FOR_TYPE - } - return false; + // All fields initialized; this->locks is therefore safe to read. + return &this->locks->primary_mu; } -// Direct-access flags can be modified without going through the -// flag API. Detect such changes and updated the modified bit. -void UpdateModifiedBit(CommandLineFlag* flag) { - if (!flag->IsAbseilFlag()) { - absl::MutexLock l(InitFlagIfNecessary(flag)); - if (!flag->modified && ChangedDirectly(flag, flag->cur, flag->def)) { - flag->modified = true; - } +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); } -} -bool Validate(CommandLineFlag*, const void*) { - return true; + delete this->locks; } -std::string HelpText::GetHelpText() const { - if (help_function_) return help_function_(); - if (help_message_) return help_message_; - - return {}; +bool CommandLineFlag::IsModified() const { + absl::MutexLock l(InitFlagIfNecessary()); + return modified; } -const int64_t CommandLineFlag::kAtomicInit; - -void CommandLineFlag::Read(void* dst, - const flags_internal::FlagOpFn dst_op) const { - absl::ReaderMutexLock l( - InitFlagIfNecessary(const_cast<CommandLineFlag*>(this))); - - // `dst_op` is the unmarshaling operation corresponding to the declaration - // visibile at the call site. `op` is the Flag's defined unmarshalling - // operation. They must match for this operation to be well-defined. - if (ABSL_PREDICT_FALSE(dst_op != op)) { - ABSL_INTERNAL_LOG( - ERROR, - absl::StrCat("Flag '", name, - "' is defined as one type and declared as another")); - } - CopyConstruct(op, cur, dst); +void CommandLineFlag::SetModified(bool is_modified) { + absl::MutexLock l(InitFlagIfNecessary()); + modified = is_modified; } -void CommandLineFlag::Write(const void* src, - const flags_internal::FlagOpFn src_op) { - absl::Mutex* mu = InitFlagIfNecessary(this); - absl::MutexLock l(mu); - - // `src_op` is the marshalling operation corresponding to the declaration - // visible at the call site. `op` is the Flag's defined marshalling operation. - // They must match for this operation to be well-defined. - if (ABSL_PREDICT_FALSE(src_op != op)) { - ABSL_INTERNAL_LOG( - ERROR, - absl::StrCat("Flag '", name, - "' is defined as one type and declared as another")); - } - - if (ShouldValidateFlagValue(*this)) { - void* obj = Clone(op, src); - std::string ignored_error; - std::string src_as_str = Unparse(marshalling_op, src); - if (!Parse(marshalling_op, src_as_str, obj, &ignored_error) || - !Validate(this, obj)) { - ABSL_INTERNAL_LOG(ERROR, absl::StrCat("Attempt to set flag '", name, - "' to invalid value ", src_as_str)); - } - Delete(op, obj); - } - - modified = true; - counter++; - Copy(op, src, cur); - - UpdateCopy(this, mu); +bool CommandLineFlag::IsSpecifiedOnCommandLine() const { + absl::MutexLock l(InitFlagIfNecessary()); + return on_command_line; } absl::string_view CommandLineFlag::Typename() const { @@ -259,21 +148,96 @@ std::string CommandLineFlag::Filename() const { } std::string CommandLineFlag::DefaultValue() const { + absl::MutexLock l(InitFlagIfNecessary()); + return Unparse(this->marshalling_op, this->def); } std::string CommandLineFlag::CurrentValue() const { + absl::MutexLock l(InitFlagIfNecessary()); + 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::Mutex* mu = InitFlagIfNecessary(this); - absl::MutexLock l(mu); + absl::MutexLock l(InitFlagIfNecessary()); callback = mutation_callback; - InvokeCallback(this, mu); + 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` @@ -282,8 +246,9 @@ void CommandLineFlag::SetCallback( // parsed value in 'dst' assuming it is a pointer to the flag's value type. In // case if any error is encountered in either step, the error message is stored // in 'err' -static bool TryParseLocked(CommandLineFlag* flag, void* dst, - absl::string_view value, std::string* err) { +bool TryParseLocked(CommandLineFlag* flag, void* dst, absl::string_view value, + std::string* err) + EXCLUSIVE_LOCKS_REQUIRED(flag->locks->primary_mu) { void* tentative_value = Clone(flag->op, flag->def); std::string parse_err; if (!Parse(flag->marshalling_op, value, tentative_value, &parse_err)) { @@ -297,7 +262,7 @@ static bool TryParseLocked(CommandLineFlag* flag, void* dst, return false; } - if (!Validate(flag, tentative_value)) { + if (!flag->InvokeValidator(tentative_value)) { *err = absl::StrCat("Failed validation of new value '", Unparse(flag->marshalling_op, tentative_value), "' for flag '", flag->Name(), "'"); @@ -324,17 +289,23 @@ bool CommandLineFlag::SetFromString(absl::string_view value, ValueSource source, std::string* err) { if (IsRetired()) return false; - UpdateModifiedBit(this); + absl::MutexLock l(InitFlagIfNecessary()); - absl::Mutex* mu = InitFlagIfNecessary(this); - absl::MutexLock l(mu); + // Direct-access flags can be modified without going through the + // flag API. Detect such changes and update the flag->modified bit. + if (!IsAbseilFlag()) { + if (!this->modified && ChangedDirectly(this, this->cur, this->def)) { + this->modified = true; + } + } switch (set_mode) { case SET_FLAGS_VALUE: { // set or modify the flag's value if (!TryParseLocked(this, this->cur, value, err)) return false; this->modified = true; - UpdateCopy(this, mu); + UpdateCopy(this); + InvokeCallback(); if (source == kCommandLine) { this->on_command_line = true; @@ -346,7 +317,8 @@ bool CommandLineFlag::SetFromString(absl::string_view value, if (!this->modified) { if (!TryParseLocked(this, this->cur, value, err)) return false; this->modified = true; - UpdateCopy(this, mu); + UpdateCopy(this); + InvokeCallback(); } else { // TODO(rogeeff): review and fix this semantic. Currently we do not fail // in this case if flag is modified. This is misleading since the flag's @@ -365,7 +337,8 @@ bool CommandLineFlag::SetFromString(absl::string_view value, if (!this->modified) { // Need to set both defvalue *and* current, in this case Copy(this->op, this->def, this->cur); - UpdateCopy(this, mu); + UpdateCopy(this); + InvokeCallback(); } break; } @@ -379,5 +352,143 @@ bool CommandLineFlag::SetFromString(absl::string_view value, return true; } +void CommandLineFlag::StoreAtomic(size_t size) { + int64_t t = 0; + assert(size <= sizeof(int64_t)); + memcpy(&t, this->cur, size); + this->atomic.store(t, std::memory_order_release); +} + +void CommandLineFlag::CheckDefaultValueParsingRoundtrip() const { + std::string v = DefaultValue(); + + absl::MutexLock lock(InitFlagIfNecessary()); + + void* dst = Clone(this->op, this->def); + std::string error; + if (!flags_internal::Parse(this->marshalling_op, v, dst, &error)) { + ABSL_INTERNAL_LOG( + FATAL, + absl::StrCat("Flag ", Name(), " (from ", Filename(), + "): std::string form of default value '", v, + "' could not be parsed; error=", error)); + } + + // We do not compare dst to def since parsing/unparsing may make + // small changes, e.g., precision loss for floating point types. + Delete(this->op, dst); +} + +bool CommandLineFlag::ValidateDefaultValue() const { + absl::MutexLock lock(InitFlagIfNecessary()); + return InvokeValidator(this->def); +} + +bool CommandLineFlag::ValidateInputValue(absl::string_view value) const { + absl::MutexLock l(InitFlagIfNecessary()); // protect default value access + + void* obj = Clone(this->op, this->def); + std::string ignored_error; + const bool result = + flags_internal::Parse(this->marshalling_op, value, obj, &ignored_error) && + InvokeValidator(obj); + Delete(this->op, obj); + return result; +} + +const int64_t CommandLineFlag::kAtomicInit; + +void CommandLineFlag::Read(void* dst, + const flags_internal::FlagOpFn dst_op) const { + absl::ReaderMutexLock l(InitFlagIfNecessary()); + + // `dst_op` is the unmarshaling operation corresponding to the declaration + // visibile at the call site. `op` is the Flag's defined unmarshalling + // operation. They must match for this operation to be well-defined. + if (ABSL_PREDICT_FALSE(dst_op != op)) { + ABSL_INTERNAL_LOG( + ERROR, + absl::StrCat("Flag '", name, + "' is defined as one type and declared as another")); + } + CopyConstruct(op, cur, dst); +} + +void CommandLineFlag::Write(const void* src, + const flags_internal::FlagOpFn src_op) { + absl::MutexLock l(InitFlagIfNecessary()); + + // `src_op` is the marshalling operation corresponding to the declaration + // visible at the call site. `op` is the Flag's defined marshalling operation. + // They must match for this operation to be well-defined. + if (ABSL_PREDICT_FALSE(src_op != op)) { + ABSL_INTERNAL_LOG( + ERROR, + absl::StrCat("Flag '", name, + "' is defined as one type and declared as another")); + } + + if (ShouldValidateFlagValue(*this)) { + void* obj = Clone(op, src); + std::string ignored_error; + std::string src_as_str = Unparse(marshalling_op, src); + if (!Parse(marshalling_op, src_as_str, obj, &ignored_error) || + !InvokeValidator(obj)) { + ABSL_INTERNAL_LOG(ERROR, absl::StrCat("Attempt to set flag '", name, + "' to invalid value ", src_as_str)); + } + Delete(op, obj); + } + + modified = true; + counter++; + Copy(op, src, cur); + + UpdateCopy(this); + InvokeCallback(); +} + +std::string HelpText::GetHelpText() const { + if (help_function_) return help_function_(); + if (help_message_) return help_message_; + + return {}; +} + +// Update any copy of the flag value that is stored in an atomic word. +// In addition if flag has a mutation callback this function invokes it. +void UpdateCopy(CommandLineFlag* flag) { +#define STORE_ATOMIC(T) \ + else if (flag->IsOfType<T>()) { \ + flag->StoreAtomic(sizeof(T)); \ + } + + if (false) { + } + ABSL_FLAGS_INTERNAL_FOR_EACH_LOCK_FREE(STORE_ATOMIC) +#undef STORE_ATOMIC +} + +// Return true iff flag value was changed via direct-access. +bool ChangedDirectly(CommandLineFlag* flag, const void* a, const void* b) { + if (!flag->IsAbseilFlag()) { +// Need to compare values for direct-access flags. +#define CHANGED_FOR_TYPE(T) \ + if (flag->IsOfType<T>()) { \ + return *reinterpret_cast<const T*>(a) != *reinterpret_cast<const T*>(b); \ + } + + CHANGED_FOR_TYPE(bool); + CHANGED_FOR_TYPE(int32_t); + CHANGED_FOR_TYPE(int64_t); + CHANGED_FOR_TYPE(uint64_t); + CHANGED_FOR_TYPE(double); + CHANGED_FOR_TYPE(std::string); +#undef CHANGED_FOR_TYPE + } + + return false; +} + } // namespace flags_internal } // namespace absl diff --git a/absl/flags/internal/commandlineflag.h b/absl/flags/internal/commandlineflag.h index 4815cdc7c9a2..d82443a7760e 100644 --- a/absl/flags/internal/commandlineflag.h +++ b/absl/flags/internal/commandlineflag.h @@ -69,11 +69,15 @@ using HelpGenFunc = std::string (*)(); // based on default value supplied in flag's definition) 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 @@ -217,6 +221,9 @@ struct CommandLineFlag { atomic(kAtomicInit), locks(nullptr) {} + // Revert the init routine. + void Destroy() const; + // Not copyable/assignable. CommandLineFlag(const CommandLineFlag&) = delete; CommandLineFlag& operator=(const CommandLineFlag&) = delete; @@ -224,7 +231,9 @@ struct CommandLineFlag { absl::string_view Name() const { return name; } std::string Help() const { return help.GetHelpText(); } bool IsRetired() const { return this->retired; } - bool IsSpecifiedOnCommandLine() const { return on_command_line; } + 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 @@ -236,6 +245,10 @@ struct CommandLineFlag { std::string DefaultValue() const; std::string CurrentValue() const; + bool HasValidatorFn() const; + bool SetValidatorFn(FlagValidator fn); + bool InvokeValidator(const void* value) const; + // Return true iff flag has type T. template <typename T> inline bool IsOfType() const { @@ -245,7 +258,7 @@ struct CommandLineFlag { // Attempts to retrieve the flag value. Returns value on success, // absl::nullopt otherwise. template <typename T> - absl::optional<T> Get() { + absl::optional<T> Get() const { if (IsRetired() || flags_internal::FlagOps<T> != this->op) return absl::nullopt; @@ -256,6 +269,7 @@ struct CommandLineFlag { } void SetCallback(const flags_internal::FlagCallback mutation_callback); + 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` @@ -269,27 +283,35 @@ struct CommandLineFlag { flags_internal::FlagSettingMode set_mode, flags_internal::ValueSource source, std::string* error); + 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: const char* const name; const HelpText help; const char* const filename; - public: - const FlagOpFn op; // Type-specific handler + 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 + 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. - bool (*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 + 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 // For some types, a copy of the current value is kept in an atomically // accessible field. @@ -302,24 +324,26 @@ struct CommandLineFlag { // TODO(rogeeff): fix it once Mutex has constexpr constructor struct CommandLineFlagLocks* locks; // locks, laziliy allocated. + // 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; + // copy construct new value of flag's type in a memory referenced by dst // based on current flag's value void Read(void* dst, const flags_internal::FlagOpFn dst_op) const; // updates flag's value to *src (locked) void Write(const void* src, const flags_internal::FlagOpFn src_op); - ABSL_DEPRECATED( - "temporary until FlagName call sites are migrated and validator API is " - "changed") - const char* NameAsCString() const { return name; } - - private: friend class FlagRegistry; + friend class FlagPtrMap; + friend class FlagSaverImpl; + friend void FillCommandLineFlagInfo(CommandLineFlag* flag, + CommandLineFlagInfo* result); + friend bool TryParseLocked(CommandLineFlag* flag, void* dst, + absl::string_view value, std::string* err); + friend absl::Mutex* InitFlag(CommandLineFlag* flag); }; -// Ensure that the lazily initialized fields of *flag have been initialized, -// and return &flag->locks->primary_mu. -absl::Mutex* InitFlagIfNecessary(CommandLineFlag* flag); // Update any copy of the flag value that is stored in an atomic word. // In addition if flag has a mutation callback this function invokes it. While // callback is being invoked the primary flag's mutex is unlocked and it is @@ -332,15 +356,9 @@ absl::Mutex* InitFlagIfNecessary(CommandLineFlag* flag); // 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 UpdateCopy(CommandLineFlag* flag, absl::Mutex* primary_lock); +void UpdateCopy(CommandLineFlag* flag); // Return true iff flag value was changed via direct-access. bool ChangedDirectly(CommandLineFlag* flag, const void* a, const void* b); -// Direct-access flags can be modified without going through the -// flag API. Detect such changes and updated the modified bit. -void UpdateModifiedBit(CommandLineFlag* flag); -// Invoke the flag validators for old flags. -// TODO(rogeeff): implement proper validators for Abseil Flags -bool Validate(CommandLineFlag* flag, const void* value); // This macro is the "source of truth" for the list of supported flag types we // expect to perform lock free operations on. Specifically it generates code, diff --git a/absl/flags/internal/commandlineflag_test.cc b/absl/flags/internal/commandlineflag_test.cc index 705f301ce7a0..f0d57adbd260 100644 --- a/absl/flags/internal/commandlineflag_test.cc +++ b/absl/flags/internal/commandlineflag_test.cc @@ -100,39 +100,39 @@ TEST_F(CommandLineFlagTest, TestSetFromStringCurrentValue) { std::string err; auto* flag_01 = flags::FindCommandLineFlag("int_flag"); - EXPECT_FALSE(flag_01->on_command_line); + EXPECT_FALSE(flag_01->IsSpecifiedOnCommandLine()); EXPECT_TRUE(flag_01->SetFromString("11", flags::SET_FLAGS_VALUE, flags::kProgrammaticChange, &err)); EXPECT_EQ(absl::GetFlag(FLAGS_int_flag), 11); - EXPECT_FALSE(flag_01->on_command_line); + EXPECT_FALSE(flag_01->IsSpecifiedOnCommandLine()); EXPECT_TRUE(flag_01->SetFromString("-123", flags::SET_FLAGS_VALUE, flags::kProgrammaticChange, &err)); EXPECT_EQ(absl::GetFlag(FLAGS_int_flag), -123); - EXPECT_FALSE(flag_01->on_command_line); + EXPECT_FALSE(flag_01->IsSpecifiedOnCommandLine()); EXPECT_TRUE(!flag_01->SetFromString("xyz", flags::SET_FLAGS_VALUE, flags::kProgrammaticChange, &err)); EXPECT_EQ(absl::GetFlag(FLAGS_int_flag), -123); EXPECT_EQ(err, "Illegal value 'xyz' specified for flag 'int_flag'"); - EXPECT_FALSE(flag_01->on_command_line); + EXPECT_FALSE(flag_01->IsSpecifiedOnCommandLine()); EXPECT_TRUE(!flag_01->SetFromString("A1", flags::SET_FLAGS_VALUE, flags::kProgrammaticChange, &err)); EXPECT_EQ(absl::GetFlag(FLAGS_int_flag), -123); EXPECT_EQ(err, "Illegal value 'A1' specified for flag 'int_flag'"); - EXPECT_FALSE(flag_01->on_command_line); + EXPECT_FALSE(flag_01->IsSpecifiedOnCommandLine()); EXPECT_TRUE(flag_01->SetFromString("0x10", flags::SET_FLAGS_VALUE, flags::kProgrammaticChange, &err)); EXPECT_EQ(absl::GetFlag(FLAGS_int_flag), 16); - EXPECT_FALSE(flag_01->on_command_line); + EXPECT_FALSE(flag_01->IsSpecifiedOnCommandLine()); EXPECT_TRUE(flag_01->SetFromString("011", flags::SET_FLAGS_VALUE, flags::kCommandLine, &err)); EXPECT_EQ(absl::GetFlag(FLAGS_int_flag), 11); - EXPECT_TRUE(flag_01->on_command_line); + EXPECT_TRUE(flag_01->IsSpecifiedOnCommandLine()); EXPECT_TRUE(!flag_01->SetFromString("", flags::SET_FLAGS_VALUE, flags::kProgrammaticChange, &err)); diff --git a/absl/flags/internal/flag.h b/absl/flags/internal/flag.h index 9b32f46735b6..bc50de5486bc 100644 --- a/absl/flags/internal/flag.h +++ b/absl/flags/internal/flag.h @@ -24,40 +24,58 @@ namespace flags_internal { // This is "unspecified" implementation of absl::Flag<T> type. template <typename T> -class Flag { +class Flag : public flags_internal::CommandLineFlag { public: constexpr Flag(const char* name, const flags_internal::HelpGenFunc help_gen, const char* filename, - const flags_internal::FlagMarshallingOpFn marshalling_op, + const flags_internal::FlagMarshallingOpFn marshalling_op_arg, const flags_internal::InitialValGenFunc initial_value_gen) - : internal(name, flags_internal::HelpText::FromFunctionPointer(help_gen), - filename, &flags_internal::FlagOps<T>, marshalling_op, - initial_value_gen, - /*retired_arg=*/false, /*def_arg=*/nullptr, - /*cur_arg=*/nullptr) {} - - // Not copyable/assignable. - Flag(const Flag<T>&) = delete; - Flag<T>& operator=(const Flag<T>&) = delete; - - absl::string_view Name() const { return internal.Name(); } - std::string Help() const { return internal.Help(); } - std::string Filename() const { return internal.Filename(); } + : flags_internal::CommandLineFlag( + name, flags_internal::HelpText::FromFunctionPointer(help_gen), + filename, &flags_internal::FlagOps<T>, marshalling_op_arg, + initial_value_gen, + /*retired_arg=*/false, /*def_arg=*/nullptr, + /*cur_arg=*/nullptr) {} + + T Get() const { + // Implementation notes: + // + // We are wrapping a union around the value of `T` to serve three purposes: + // + // 1. `U.value` has correct size and alignment for a value of type `T` + // 2. The `U.value` constructor is not invoked since U's constructor does + // not + // do it explicitly. + // 3. The `U.value` destructor is invoked since U's destructor does it + // explicitly. This makes `U` a kind of RAII wrapper around non default + // constructible value of T, which is destructed when we leave the + // scope. We do need to destroy U.value, which is constructed by + // CommandLineFlag::Read even though we left it in a moved-from state + // after std::move. + // + // All of this serves to avoid requiring `T` being default constructible. + union U { + T value; + U() {} + ~U() { value.~T(); } + }; + U u; + + this->Read(&u.value, &flags_internal::FlagOps<T>); + return std::move(u.value); + } - absl::flags_internal::CommandLineFlag internal; + bool AtomicGet(T* v) const { + const int64_t r = this->atomic.load(std::memory_order_acquire); + if (r != flags_internal::CommandLineFlag::kAtomicInit) { + memcpy(v, &r, sizeof(T)); + return true; + } - void SetCallback(const flags_internal::FlagCallback mutation_callback) { - internal.SetCallback(mutation_callback); + return false; } - private: - // TODO(rogeeff): add these validations once UnparseFlag invocation is fixed - // for built-in types and when we cleanup existing code from operating on - // forward declared types. - // auto IsCopyConstructible(const T& v) -> decltype(T(v)); - // auto HasAbslParseFlag(absl::string_view in, T* dst, std::string* err) - // -> decltype(AbslParseFlag(in, dst, err)); - // auto HasAbslUnparseFlag(const T& v) -> decltype(AbslUnparseFlag(v)); + void Set(const T& v) { this->Write(&v, &flags_internal::FlagOps<T>); } }; // This class facilitates Flag object registration and tail expression-based @@ -67,7 +85,7 @@ template <typename T, bool do_register> class FlagRegistrar { public: explicit FlagRegistrar(Flag<T>* flag) : flag_(flag) { - if (do_register) flags_internal::RegisterCommandLineFlag(&flag_->internal); + if (do_register) flags_internal::RegisterCommandLineFlag(flag_); } FlagRegistrar& OnUpdate(flags_internal::FlagCallback cb) && { diff --git a/absl/flags/internal/registry.cc b/absl/flags/internal/registry.cc index e52cbb683b9e..f37871c9d40f 100644 --- a/absl/flags/internal/registry.cc +++ b/absl/flags/internal/registry.cc @@ -34,13 +34,7 @@ namespace flags_internal { namespace { void DestroyFlag(CommandLineFlag* flag) NO_THREAD_SAFETY_ANALYSIS { - // Values are heap allocated for retired and Abseil Flags. - if (flag->IsRetired() || flag->IsAbseilFlag()) { - if (flag->cur) Delete(flag->op, flag->cur); - if (flag->def) Delete(flag->op, flag->def); - } - - delete flag->locks; + flag->Destroy(); // CommandLineFlag handle object is heap allocated for non Abseil Flags. if (!flag->IsAbseilFlag()) { @@ -48,6 +42,8 @@ void DestroyFlag(CommandLineFlag* flag) NO_THREAD_SAFETY_ANALYSIS { } } +} // namespace + // -------------------------------------------------------------------- // FlagRegistry // A FlagRegistry singleton object holds all flag objects indexed @@ -105,8 +101,6 @@ class FlagPtrMap { }; constexpr size_t FlagPtrMap::kNumBuckets; -} // namespace - class FlagRegistry { public: FlagRegistry() = default; @@ -292,10 +286,10 @@ class FlagSaverImpl { saved.op = flag->op; saved.marshalling_op = flag->marshalling_op; { - absl::MutexLock l(InitFlagIfNecessary(flag)); + absl::MutexLock l(flag->InitFlagIfNecessary()); saved.validator = flag->validator; saved.modified = flag->modified; - saved.on_command_line = flag->IsSpecifiedOnCommandLine(); + saved.on_command_line = flag->on_command_line; saved.current = Clone(saved.op, flag->cur); saved.default_value = Clone(saved.op, flag->def); saved.counter = flag->counter; @@ -318,34 +312,34 @@ class FlagSaverImpl { bool restored = false; { - absl::Mutex* mu = InitFlagIfNecessary(flag); - absl::MutexLock l(mu); + absl::MutexLock l(flag->InitFlagIfNecessary()); flag->validator = src.validator; flag->modified = src.modified; flag->on_command_line = src.on_command_line; if (flag->counter != src.counter || ChangedDirectly(flag, src.default_value, flag->def)) { - flag->counter++; + restored = true; Copy(src.op, src.default_value, flag->def); } if (flag->counter != src.counter || ChangedDirectly(flag, src.current, flag->cur)) { restored = true; - flag->counter++; Copy(src.op, src.current, flag->cur); - UpdateCopy(flag, mu); - - // Revalidate the flag because the validator might store state based - // on the flag's value, which just changed due to the restore. - // Failing validation is ignored because it's assumed that the flag - // was valid previously and there's little that can be done about it - // here, anyway. - Validate(flag, flag->cur); + UpdateCopy(flag); + flag->InvokeCallback(); } } - // Log statements must be done when no flag lock is held. if (restored) { + flag->counter++; + + // Revalidate the flag because the validator might store state based + // on the flag's value, which just changed due to the restore. + // Failing validation is ignored because it's assumed that the flag + // was valid previously and there's little that can be done about it + // here, anyway. + flag->ValidateInputValue(flag->CurrentValue()); + ABSL_INTERNAL_LOG( INFO, absl::StrCat("Restore saved value of ", flag->Name(), ": ", Unparse(src.marshalling_op, src.current))); @@ -412,13 +406,17 @@ void FillCommandLineFlagInfo(CommandLineFlag* flag, result->description = flag->Help(); result->filename = flag->Filename(); - UpdateModifiedBit(flag); + if (!flag->IsAbseilFlag()) { + if (!flag->IsModified() && ChangedDirectly(flag, flag->cur, flag->def)) { + flag->modified = true; + } + } - absl::MutexLock l(InitFlagIfNecessary(flag)); result->current_value = flag->CurrentValue(); result->default_value = flag->DefaultValue(); - result->is_default = !flag->modified; - result->has_validator_fn = (flag->validator != nullptr); + result->is_default = !flag->IsModified(); + result->has_validator_fn = flag->HasValidatorFn(); + absl::MutexLock l(flag->InitFlagIfNecessary()); result->flag_ptr = flag->IsAbseilFlag() ? nullptr : flag->cur; } diff --git a/absl/flags/internal/type_erased.cc b/absl/flags/internal/type_erased.cc index cc103983672b..2984291c87c7 100644 --- a/absl/flags/internal/type_erased.cc +++ b/absl/flags/internal/type_erased.cc @@ -32,7 +32,6 @@ bool GetCommandLineOption(absl::string_view name, std::string* value) { return false; } - absl::MutexLock l(InitFlagIfNecessary(flag)); *value = flag->CurrentValue(); return true; } @@ -88,22 +87,9 @@ bool SetCommandLineOptionWithMode(absl::string_view name, bool IsValidFlagValue(absl::string_view name, absl::string_view value) { CommandLineFlag* flag = flags_internal::FindCommandLineFlag(name); - if (flag == nullptr) { - return false; - } - - if (flag->IsRetired()) { - return true; - } - // No need to lock the flag since we are not mutating it. - void* obj = Clone(flag->op, flag->def); - std::string ignored_error; - const bool result = - flags_internal::Parse(flag->marshalling_op, value, obj, &ignored_error) && - Validate(flag, obj); - Delete(flag->op, obj); - return result; + return flag != nullptr && + (flag->IsRetired() || flag->ValidateInputValue(value)); } // -------------------------------------------------------------------- @@ -111,7 +97,6 @@ bool IsValidFlagValue(absl::string_view name, absl::string_view value) { bool SpecifiedOnCommandLine(absl::string_view name) { CommandLineFlag* flag = flags_internal::FindCommandLineFlag(name); if (flag != nullptr && !flag->IsRetired()) { - absl::MutexLock l(InitFlagIfNecessary(flag)); return flag->IsSpecifiedOnCommandLine(); } return false; diff --git a/absl/flags/internal/usage.cc b/absl/flags/internal/usage.cc index b1ff8b83c223..aac02db6c27a 100644 --- a/absl/flags/internal/usage.cc +++ b/absl/flags/internal/usage.cc @@ -21,11 +21,11 @@ #include "absl/flags/flag.h" #include "absl/flags/internal/path_util.h" #include "absl/flags/internal/program_name.h" -#include "absl/flags/usage.h" #include "absl/flags/usage_config.h" #include "absl/strings/ascii.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_split.h" +#include "absl/strings/string_view.h" #include "absl/synchronization/mutex.h" ABSL_FLAG(bool, help, false, @@ -185,7 +185,7 @@ void FlagHelpHumanReadable(const flags_internal::CommandLineFlag& flag, } printer.Write(absl::StrCat("default: ", dflt_val, ";")); - if (flag.modified) { + if (flag.IsModified()) { std::string curr_val = flag.CurrentValue(); if (flag.IsOfType<std::string>()) { curr_val = absl::StrCat("\"", curr_val, "\""); @@ -202,10 +202,10 @@ void FlagHelpHumanReadable(const flags_internal::CommandLineFlag& flag, // STRIP_FLAG_HELP 1' then this flag will not be displayed by '--help' // and its variants. void FlagsHelpImpl(std::ostream& out, flags_internal::FlagKindFilter filter_cb, - HelpFormat format = HelpFormat::kHumanReadable) { + HelpFormat format, absl::string_view program_usage_message) { if (format == HelpFormat::kHumanReadable) { out << flags_internal::ShortProgramInvocationName() << ": " - << absl::ProgramUsageMessage() << "\n\n"; + << program_usage_message << "\n\n"; } else { // XML schema is not a part of our public API for now. out << "<?xml version=\"1.0\"?>\n" @@ -214,7 +214,7 @@ void FlagsHelpImpl(std::ostream& out, flags_internal::FlagKindFilter filter_cb, // The program name and usage. << XMLElement("program", flags_internal::ShortProgramInvocationName()) << '\n' - << XMLElement("usage", absl::ProgramUsageMessage()) << '\n'; + << XMLElement("usage", program_usage_message) << '\n'; } // Map of package name to @@ -228,8 +228,6 @@ void FlagsHelpImpl(std::ostream& out, flags_internal::FlagKindFilter filter_cb, matching_flags; flags_internal::ForEachFlag([&](flags_internal::CommandLineFlag* flag) { - absl::MutexLock l(InitFlagIfNecessary(flag)); - std::string flag_filename = flag->Filename(); // Ignore retired flags. @@ -292,44 +290,51 @@ void FlagHelp(std::ostream& out, const flags_internal::CommandLineFlag& flag, // -------------------------------------------------------------------- // Produces the help messages for all flags matching the filter. // If filter is empty produces help messages for all flags. -void FlagsHelp(std::ostream& out, absl::string_view filter, HelpFormat format) { +void FlagsHelp(std::ostream& out, absl::string_view filter, HelpFormat format, + absl::string_view program_usage_message) { flags_internal::FlagKindFilter filter_cb = [&](absl::string_view filename) { return filter.empty() || filename.find(filter) != absl::string_view::npos; }; - flags_internal::FlagsHelpImpl(out, filter_cb, format); + flags_internal::FlagsHelpImpl(out, filter_cb, format, program_usage_message); } // -------------------------------------------------------------------- // Checks all the 'usage' command line flags to see if any have been set. // If so, handles them appropriately. -int HandleUsageFlags(std::ostream& out) { +int HandleUsageFlags(std::ostream& out, + absl::string_view program_usage_message) { if (absl::GetFlag(FLAGS_helpshort)) { flags_internal::FlagsHelpImpl( out, flags_internal::GetUsageConfig().contains_helpshort_flags, - HelpFormat::kHumanReadable); + HelpFormat::kHumanReadable, program_usage_message); return 1; } if (absl::GetFlag(FLAGS_helpfull)) { // show all options - flags_internal::FlagsHelp(out); + flags_internal::FlagsHelp(out, "", HelpFormat::kHumanReadable, + program_usage_message); return 1; } if (!absl::GetFlag(FLAGS_helpon).empty()) { flags_internal::FlagsHelp( - out, absl::StrCat("/", absl::GetFlag(FLAGS_helpon), ".")); + out, absl::StrCat("/", absl::GetFlag(FLAGS_helpon), "."), + HelpFormat::kHumanReadable, program_usage_message); return 1; } if (!absl::GetFlag(FLAGS_helpmatch).empty()) { - flags_internal::FlagsHelp(out, absl::GetFlag(FLAGS_helpmatch)); + flags_internal::FlagsHelp(out, absl::GetFlag(FLAGS_helpmatch), + HelpFormat::kHumanReadable, + program_usage_message); return 1; } if (absl::GetFlag(FLAGS_help)) { flags_internal::FlagsHelpImpl( - out, flags_internal::GetUsageConfig().contains_help_flags); + out, flags_internal::GetUsageConfig().contains_help_flags, + HelpFormat::kHumanReadable, program_usage_message); out << "\nTry --helpfull to get a list of all flags.\n"; @@ -338,7 +343,8 @@ int HandleUsageFlags(std::ostream& out) { if (absl::GetFlag(FLAGS_helppackage)) { flags_internal::FlagsHelpImpl( - out, flags_internal::GetUsageConfig().contains_helppackage_flags); + out, flags_internal::GetUsageConfig().contains_helppackage_flags, + HelpFormat::kHumanReadable, program_usage_message); out << "\nTry --helpfull to get a list of all flags.\n"; diff --git a/absl/flags/internal/usage.h b/absl/flags/internal/usage.h index 33f3f969bfaf..76075b0829d9 100644 --- a/absl/flags/internal/usage.h +++ b/absl/flags/internal/usage.h @@ -47,8 +47,8 @@ void FlagHelp(std::ostream& out, const flags_internal::CommandLineFlag& flag, // .../path/to/file.<ext> // for any extension 'ext'. If the filter is empty this function produces help // messages for all flags. -void FlagsHelp(std::ostream& out, absl::string_view filter = {}, - HelpFormat format = HelpFormat::kHumanReadable); +void FlagsHelp(std::ostream& out, absl::string_view filter, + HelpFormat format, absl::string_view program_usage_message); // -------------------------------------------------------------------- @@ -60,7 +60,8 @@ void FlagsHelp(std::ostream& out, absl::string_view filter = {}, // -1 - if no usage flags were set on a commmand line. // Non negative return values are expected to be used as an exit code for a // binary. -int HandleUsageFlags(std::ostream& out); +int HandleUsageFlags(std::ostream& out, + absl::string_view program_usage_message); } // namespace flags_internal } // namespace absl diff --git a/absl/flags/internal/usage_test.cc b/absl/flags/internal/usage_test.cc index fa121fc933fc..5e5f7a84cc43 100644 --- a/absl/flags/internal/usage_test.cc +++ b/absl/flags/internal/usage_test.cc @@ -36,6 +36,8 @@ ABSL_FLAG(double, usage_reporting_test_flag_03, 1.03, ABSL_FLAG(int64_t, usage_reporting_test_flag_04, 1000000000000004L, "usage_reporting_test_flag_04 help message"); +static const char kTestUsageMessage[] = "Custom usage message"; + struct UDT { UDT() = default; UDT(const UDT&) = default; @@ -83,7 +85,7 @@ class UsageReportingTest : public testing::Test { using UsageReportingDeathTest = UsageReportingTest; TEST_F(UsageReportingDeathTest, TestSetProgramUsageMessage) { - EXPECT_EQ(absl::ProgramUsageMessage(), "Custom usage message"); + EXPECT_EQ(absl::ProgramUsageMessage(), kTestUsageMessage); #ifndef _WIN32 // TODO(rogeeff): figure out why this does not work on Windows. @@ -175,22 +177,22 @@ TEST_F(UsageReportingTest, TestFlagsHelpHRF) { std::stringstream test_buf_01; flags::FlagsHelp(test_buf_01, "usage_test.cc", - flags::HelpFormat::kHumanReadable); + flags::HelpFormat::kHumanReadable, kTestUsageMessage); EXPECT_EQ(test_buf_01.str(), usage_test_flags_out); std::stringstream test_buf_02; flags::FlagsHelp(test_buf_02, "flags/internal/usage_test.cc", - flags::HelpFormat::kHumanReadable); + flags::HelpFormat::kHumanReadable, kTestUsageMessage); EXPECT_EQ(test_buf_02.str(), usage_test_flags_out); std::stringstream test_buf_03; - flags::FlagsHelp(test_buf_03, "usage_test", - flags::HelpFormat::kHumanReadable); + flags::FlagsHelp(test_buf_03, "usage_test", flags::HelpFormat::kHumanReadable, + kTestUsageMessage); EXPECT_EQ(test_buf_03.str(), usage_test_flags_out); std::stringstream test_buf_04; flags::FlagsHelp(test_buf_04, "flags/invalid_file_name.cc", - flags::HelpFormat::kHumanReadable); + flags::HelpFormat::kHumanReadable, kTestUsageMessage); EXPECT_EQ(test_buf_04.str(), R"(usage_test: Custom usage message @@ -198,7 +200,8 @@ TEST_F(UsageReportingTest, TestFlagsHelpHRF) { )"); std::stringstream test_buf_05; - flags::FlagsHelp(test_buf_05, "", flags::HelpFormat::kHumanReadable); + flags::FlagsHelp(test_buf_05, "", flags::HelpFormat::kHumanReadable, + kTestUsageMessage); std::string test_out = test_buf_05.str(); absl::string_view test_out_str(test_out); EXPECT_TRUE( @@ -217,7 +220,7 @@ TEST_F(UsageReportingTest, TestFlagsHelpHRF) { TEST_F(UsageReportingTest, TestNoUsageFlags) { std::stringstream test_buf; - EXPECT_EQ(flags::HandleUsageFlags(test_buf), -1); + EXPECT_EQ(flags::HandleUsageFlags(test_buf, kTestUsageMessage), -1); } // -------------------------------------------------------------------- @@ -226,7 +229,7 @@ TEST_F(UsageReportingTest, TestUsageFlag_helpshort) { absl::SetFlag(&FLAGS_helpshort, true); std::stringstream test_buf; - EXPECT_EQ(flags::HandleUsageFlags(test_buf), 1); + EXPECT_EQ(flags::HandleUsageFlags(test_buf, kTestUsageMessage), 1); EXPECT_EQ(test_buf.str(), R"(usage_test: Custom usage message @@ -250,7 +253,7 @@ TEST_F(UsageReportingTest, TestUsageFlag_help) { absl::SetFlag(&FLAGS_help, true); std::stringstream test_buf; - EXPECT_EQ(flags::HandleUsageFlags(test_buf), 1); + EXPECT_EQ(flags::HandleUsageFlags(test_buf, kTestUsageMessage), 1); EXPECT_EQ(test_buf.str(), R"(usage_test: Custom usage message @@ -276,7 +279,7 @@ TEST_F(UsageReportingTest, TestUsageFlag_helppackage) { absl::SetFlag(&FLAGS_helppackage, true); std::stringstream test_buf; - EXPECT_EQ(flags::HandleUsageFlags(test_buf), 1); + EXPECT_EQ(flags::HandleUsageFlags(test_buf, kTestUsageMessage), 1); EXPECT_EQ(test_buf.str(), R"(usage_test: Custom usage message @@ -302,10 +305,9 @@ TEST_F(UsageReportingTest, TestUsageFlag_version) { absl::SetFlag(&FLAGS_version, true); std::stringstream test_buf; - EXPECT_EQ(flags::HandleUsageFlags(test_buf), 0); + EXPECT_EQ(flags::HandleUsageFlags(test_buf, kTestUsageMessage), 0); #ifndef NDEBUG - EXPECT_EQ(test_buf.str(), - "usage_test\nDebug build (NDEBUG not #defined)\n"); + EXPECT_EQ(test_buf.str(), "usage_test\nDebug build (NDEBUG not #defined)\n"); #else EXPECT_EQ(test_buf.str(), "usage_test\n"); #endif @@ -317,7 +319,7 @@ TEST_F(UsageReportingTest, TestUsageFlag_only_check_args) { absl::SetFlag(&FLAGS_only_check_args, true); std::stringstream test_buf; - EXPECT_EQ(flags::HandleUsageFlags(test_buf), 0); + EXPECT_EQ(flags::HandleUsageFlags(test_buf, kTestUsageMessage), 0); EXPECT_EQ(test_buf.str(), ""); } @@ -327,7 +329,7 @@ TEST_F(UsageReportingTest, TestUsageFlag_helpon) { absl::SetFlag(&FLAGS_helpon, "bla-bla"); std::stringstream test_buf_01; - EXPECT_EQ(flags::HandleUsageFlags(test_buf_01), 1); + EXPECT_EQ(flags::HandleUsageFlags(test_buf_01, kTestUsageMessage), 1); EXPECT_EQ(test_buf_01.str(), R"(usage_test: Custom usage message @@ -337,7 +339,7 @@ TEST_F(UsageReportingTest, TestUsageFlag_helpon) { absl::SetFlag(&FLAGS_helpon, "usage_test"); std::stringstream test_buf_02; - EXPECT_EQ(flags::HandleUsageFlags(test_buf_02), 1); + EXPECT_EQ(flags::HandleUsageFlags(test_buf_02, kTestUsageMessage), 1); EXPECT_EQ(test_buf_02.str(), R"(usage_test: Custom usage message @@ -362,7 +364,7 @@ TEST_F(UsageReportingTest, TestUsageFlag_helpon) { int main(int argc, char* argv[]) { absl::GetFlag(FLAGS_undefok); // Force linking of parse.cc flags::SetProgramInvocationName("usage_test"); - absl::SetProgramUsageMessage("Custom usage message"); + absl::SetProgramUsageMessage(kTestUsageMessage); ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); diff --git a/absl/flags/parse.cc b/absl/flags/parse.cc index 3caaa1c256cb..e9dd4204e8a8 100644 --- a/absl/flags/parse.cc +++ b/absl/flags/parse.cc @@ -16,6 +16,7 @@ #include "absl/flags/parse.h" #include <stdlib.h> + #include <fstream> #include <iostream> #include <tuple> @@ -28,6 +29,7 @@ #include "absl/flags/internal/program_name.h" #include "absl/flags/internal/registry.h" #include "absl/flags/internal/usage.h" +#include "absl/flags/usage.h" #include "absl/flags/usage_config.h" #include "absl/strings/str_cat.h" #include "absl/strings/strip.h" @@ -280,22 +282,7 @@ void CheckDefaultValuesParsingRoundtrip() { IGNORE_TYPE(std::vector<std::string>) #undef IGNORE_TYPE - absl::MutexLock lock(InitFlagIfNecessary(flag)); - - std::string v = flag->DefaultValue(); - void* dst = Clone(flag->op, flag->def); - std::string error; - if (!flags_internal::Parse(flag->marshalling_op, v, dst, &error)) { - ABSL_INTERNAL_LOG( - FATAL, - absl::StrCat("Flag ", flag->Name(), " (from ", flag->Filename(), - "): std::string form of default value '", v, - "' could not be parsed; error=", error)); - } - - // We do not compare dst to def since parsing/unparsing may make - // small changes, e.g., precision loss for floating point types. - Delete(flag->op, dst); + flag->CheckDefaultValueParsingRoundtrip(); }); #endif } @@ -717,12 +704,14 @@ std::vector<char*> ParseCommandLineImpl(int argc, char* argv[], #endif if (!success) { - flags_internal::HandleUsageFlags(std::cout); + flags_internal::HandleUsageFlags(std::cout, + ProgramUsageMessage()); std::exit(1); } if (usage_flag_act == UsageFlagsAction::kHandleUsage) { - int exit_code = flags_internal::HandleUsageFlags(std::cout); + int exit_code = flags_internal::HandleUsageFlags( + std::cout, ProgramUsageMessage()); if (exit_code != -1) { std::exit(exit_code); diff --git a/absl/hash/hash_test.cc b/absl/hash/hash_test.cc index bab560bd4e64..9a667ba703b4 100644 --- a/absl/hash/hash_test.cc +++ b/absl/hash/hash_test.cc @@ -288,7 +288,6 @@ TEST(HashValueTest, Strings) { // Also check that nested types maintain the same hash. const WrapInTuple t{}; EXPECT_TRUE(absl::VerifyTypeImplementsAbslHashCorrectly(std::make_tuple( - // t(std::string()), t(absl::string_view()), t(std::string("")), t(absl::string_view("")), t(std::string(small)), t(absl::string_view(small)), diff --git a/absl/hash/internal/hash.h b/absl/hash/internal/hash.h index 18665173350e..4ff4a126f0cd 100644 --- a/absl/hash/internal/hash.h +++ b/absl/hash/internal/hash.h @@ -640,7 +640,8 @@ class CityHashState : public HashStateBase<CityHashState> { #endif // ABSL_HAVE_INTRINSIC_INT128 static constexpr uint64_t kMul = - sizeof(size_t) == 4 ? uint64_t{0xcc9e2d51} : uint64_t{0x9ddfea08eb382d69}; + sizeof(size_t) == 4 ? uint64_t{0xcc9e2d51} + : uint64_t{0x9ddfea08eb382d69}; template <typename T> using IntegralFastPath = diff --git a/absl/numeric/int128.h b/absl/numeric/int128.h index 2f5b8ad7a874..10be8eca42e7 100644 --- a/absl/numeric/int128.h +++ b/absl/numeric/int128.h @@ -19,8 +19,8 @@ // // This header file defines 128-bit integer types. // -// Currently, this file defines `uint128`, an unsigned 128-bit integer; a signed -// 128-bit integer is forthcoming. +// Currently, this file defines `uint128`, an unsigned 128-bit integer; +// a signed 128-bit integer is forthcoming. #ifndef ABSL_NUMERIC_INT128_H_ #define ABSL_NUMERIC_INT128_H_ diff --git a/absl/random/internal/fast_uniform_bits.h b/absl/random/internal/fast_uniform_bits.h index 184a2708b2af..e8df92f3d99f 100644 --- a/absl/random/internal/fast_uniform_bits.h +++ b/absl/random/internal/fast_uniform_bits.h @@ -22,11 +22,18 @@ namespace absl { namespace random_internal { +// Returns true if the input value is zero or a power of two. Useful for +// determining if the range of output values in a URBG +template <typename UIntType> +constexpr bool IsPowerOfTwoOrZero(UIntType n) { + return (n == 0) || ((n & (n - 1)) == 0); +} + // Computes the length of the range of values producible by the URBG, or returns // zero if that would encompass the entire range of representable values in // URBG::result_type. template <typename URBG> -constexpr typename URBG::result_type constexpr_range() { +constexpr typename URBG::result_type RangeSize() { using result_type = typename URBG::result_type; return ((URBG::max)() == (std::numeric_limits<result_type>::max)() && (URBG::min)() == std::numeric_limits<result_type>::lowest()) @@ -34,6 +41,42 @@ constexpr typename URBG::result_type constexpr_range() { : (URBG::max)() - (URBG::min)() + result_type{1}; } +template <typename UIntType> +constexpr UIntType LargestPowerOfTwoLessThanOrEqualTo(UIntType n) { + return n < 2 ? n : 2 * LargestPowerOfTwoLessThanOrEqualTo(n / 2); +} + +// Given a URBG generating values in the closed interval [Lo, Hi], returns the +// largest power of two less than or equal to `Hi - Lo + 1`. +template <typename URBG> +constexpr typename URBG::result_type PowerOfTwoSubRangeSize() { + return LargestPowerOfTwoLessThanOrEqualTo(RangeSize<URBG>()); +} + +// Computes the floor of the log. (i.e., std::floor(std::log2(N)); +template <typename UIntType> +constexpr UIntType IntegerLog2(UIntType n) { + return (n <= 1) ? 0 : 1 + IntegerLog2(n / 2); +} + +// Returns the number of bits of randomness returned through +// `PowerOfTwoVariate(urbg)`. +template <typename URBG> +constexpr size_t NumBits() { + return RangeSize<URBG>() == 0 + ? std::numeric_limits<typename URBG::result_type>::digits + : IntegerLog2(PowerOfTwoSubRangeSize<URBG>()); +} + +// Given a shift value `n`, constructs a mask with exactly the low `n` bits set. +// If `n == 0`, all bits are set. +template <typename UIntType> +constexpr UIntType MaskFromShift(UIntType n) { + return ((n % std::numeric_limits<UIntType>::digits) == 0) + ? ~UIntType{0} + : (UIntType{1} << n) - UIntType{1}; +} + // FastUniformBits implements a fast path to acquire uniform independent bits // from a type which conforms to the [rand.req.urbg] concept. // Parameterized by: @@ -45,14 +88,6 @@ constexpr typename URBG::result_type constexpr_range() { // generator that will outlive the std::independent_bits_engine instance. template <typename UIntType = uint64_t> class FastUniformBits { - static_assert(std::is_unsigned<UIntType>::value, - "Class-template FastUniformBits<> must be parameterized using " - "an unsigned type."); - - // `kWidth` is the width, in binary digits, of the output. By default it is - // the number of binary digits in the `result_type`. - static constexpr size_t kWidth = std::numeric_limits<UIntType>::digits; - public: using result_type = UIntType; @@ -65,14 +100,47 @@ class FastUniformBits { result_type operator()(URBG& g); // NOLINT(runtime/references) private: - // Variate() generates a single random variate, always returning a value - // in the closed interval [0 ... FastUniformBitsURBGConstants::kRangeMask] - // (kRangeMask+1 is a power of 2). + static_assert(std::is_unsigned<UIntType>::value, + "Class-template FastUniformBits<> must be parameterized using " + "an unsigned type."); + + // PowerOfTwoVariate() generates a single random variate, always returning a + // value in the half-open interval `[0, PowerOfTwoSubRangeSize<URBG>())`. If + // the URBG already generates values in a power-of-two range, the generator + // itself is used. Otherwise, we use rejection sampling on the largest + // possible power-of-two-sized subrange. + struct PowerOfTwoTag {}; + struct RejectionSamplingTag {}; template <typename URBG> - typename URBG::result_type Variate(URBG& g); // NOLINT(runtime/references) + static typename URBG::result_type PowerOfTwoVariate( + URBG& g) { // NOLINT(runtime/references) + using tag = + typename std::conditional<IsPowerOfTwoOrZero(RangeSize<URBG>()), + PowerOfTwoTag, RejectionSamplingTag>::type; + return PowerOfTwoVariate(g, tag{}); + } + + template <typename URBG> + static typename URBG::result_type PowerOfTwoVariate( + URBG& g, // NOLINT(runtime/references) + PowerOfTwoTag) { + return g() - (URBG::min)(); + } - // generate() generates a random value, dispatched on whether - // the underlying URNG must loop over multiple calls or not. + template <typename URBG> + static typename URBG::result_type PowerOfTwoVariate( + URBG& g, // NOLINT(runtime/references) + RejectionSamplingTag) { + // Use rejection sampling to ensure uniformity across the range. + typename URBG::result_type u; + do { + u = g() - (URBG::min)(); + } while (u >= PowerOfTwoSubRangeSize<URBG>()); + return u; + } + + // Generate() generates a random value, dispatched on whether + // the underlying URBG must loop over multiple calls or not. template <typename URBG> result_type Generate(URBG& g, // NOLINT(runtime/references) std::true_type /* avoid_looping */); @@ -82,196 +150,107 @@ class FastUniformBits { std::false_type /* avoid_looping */); }; -// FastUniformBitsURBGConstants computes the URBG-derived constants used -// by FastUniformBits::Generate and FastUniformBits::Variate. -// Parameterized by the FastUniformBits parameter: -// `URBG`: The underlying UniformRandomNumberGenerator. -// -// The values here indicate the URBG range as well as providing an indicator -// whether the URBG output is a power of 2, and kRangeMask, which allows masking -// the generated output to kRangeBits. +template <typename UIntType> template <typename URBG> -class FastUniformBitsURBGConstants { - // Computes the floor of the log. (i.e., std::floor(std::log2(N)); - static constexpr size_t constexpr_log2(size_t n) { - return (n <= 1) ? 0 : 1 + constexpr_log2(n / 2); - } - - // Computes a mask of n bits for the URBG::result_type. - static constexpr typename URBG::result_type constexpr_mask(size_t n) { - return (typename URBG::result_type(1) << n) - 1; - } - - public: - using result_type = typename URBG::result_type; - - // The range of the URNG, max - min + 1, or zero if that result would cause - // overflow. - static constexpr result_type kRange = constexpr_range<URBG>(); - - static constexpr bool kPowerOfTwo = - (kRange == 0) || ((kRange & (kRange - 1)) == 0); - - // kRangeBits describes the number number of bits suitable to mask off of URNG - // variate, which is: - // kRangeBits = floor(log2(kRange)) - static constexpr size_t kRangeBits = - kRange == 0 ? std::numeric_limits<result_type>::digits - : constexpr_log2(kRange); - - // kRangeMask is the mask used when sampling variates from the URNG when the - // width of the URNG range is not a power of 2. +typename FastUniformBits<UIntType>::result_type +FastUniformBits<UIntType>::operator()(URBG& g) { // NOLINT(runtime/references) + // kRangeMask is the mask used when sampling variates from the URBG when the + // width of the URBG range is not a power of 2. // Y = (2 ^ kRange) - 1 - static constexpr result_type kRangeMask = - kRange == 0 ? (std::numeric_limits<result_type>::max)() - : constexpr_mask(kRangeBits); - - static_assert((URBG::max)() != (URBG::min)(), - "Class-template FastUniformBitsURBGConstants<> " + static_assert((URBG::max)() > (URBG::min)(), "URBG::max and URBG::min may not be equal."); - - static_assert(std::is_unsigned<result_type>::value, - "Class-template FastUniformBitsURBGConstants<> " - "URBG::result_type must be unsigned."); - - static_assert(kRangeMask > 0, - "Class-template FastUniformBitsURBGConstants<> " - "URBG does not generate sufficient random bits."); - - static_assert(kRange == 0 || - kRangeBits < std::numeric_limits<result_type>::digits, - "Class-template FastUniformBitsURBGConstants<> " - "URBG range computation error."); -}; - -// FastUniformBitsLoopingConstants computes the looping constants used -// by FastUniformBits::Generate. These constants indicate how multiple -// URBG::result_type values are combined into an output_value. -// Parameterized by the FastUniformBits parameters: -// `UIntType`: output type. -// `URNG`: The underlying UniformRandomNumberGenerator. -// -// The looping constants describe the sets of loop counters and mask values -// which control how individual variates are combined the final output. The -// algorithm ensures that the number of bits used by any individual call differs -// by at-most one bit from any other call. This is simplified into constants -// which describe two loops, with the second loop parameters providing one extra -// bit per variate. -// -// See [rand.adapt.ibits] for more details on the use of these constants. -template <typename UIntType, typename URBG> -class FastUniformBitsLoopingConstants { - private: - static constexpr size_t kWidth = std::numeric_limits<UIntType>::digits; using urbg_result_type = typename URBG::result_type; - using uint_result_type = UIntType; - - public: - using result_type = - typename std::conditional<(sizeof(urbg_result_type) <= - sizeof(uint_result_type)), - uint_result_type, urbg_result_type>::type; - - private: - // Estimate N as ceil(width / urng width), and W0 as (width / N). - static constexpr size_t kRangeBits = - FastUniformBitsURBGConstants<URBG>::kRangeBits; - - // The range of the URNG, max - min + 1, or zero if that result would cause - // overflow. - static constexpr result_type kRange = constexpr_range<URBG>(); - static constexpr size_t kEstimateN = - kWidth / kRangeBits + (kWidth % kRangeBits != 0); - static constexpr size_t kEstimateW0 = kWidth / kEstimateN; - static constexpr result_type kEstimateY0 = (kRange >> kEstimateW0) - << kEstimateW0; - - public: - // Parameters for the two loops: - // kN0, kN1 are the number of underlying calls required for each loop. - // KW0, kW1 are shift widths for each loop. - // - static constexpr size_t kN1 = (kRange - kEstimateY0) > - (kEstimateY0 / kEstimateN) - ? kEstimateN + 1 - : kEstimateN; - static constexpr size_t kN0 = kN1 - (kWidth % kN1); - static constexpr size_t kW0 = kWidth / kN1; - static constexpr size_t kW1 = kW0 + 1; - - static constexpr result_type kM0 = (result_type(1) << kW0) - 1; - static constexpr result_type kM1 = (result_type(1) << kW1) - 1; - - static_assert( - kW0 <= kRangeBits, - "Class-template FastUniformBitsLoopingConstants::kW0 too large."); - - static_assert( - kW0 > 0, - "Class-template FastUniformBitsLoopingConstants::kW0 too small."); -}; - -template <typename UIntType> -template <typename URBG> -typename FastUniformBits<UIntType>::result_type -FastUniformBits<UIntType>::operator()( - URBG& g) { // NOLINT(runtime/references) - using constants = FastUniformBitsURBGConstants<URBG>; - return Generate( - g, std::integral_constant<bool, constants::kRangeMask >= (max)()>{}); -} - -template <typename UIntType> -template <typename URBG> -typename URBG::result_type FastUniformBits<UIntType>::Variate( - URBG& g) { // NOLINT(runtime/references) - using constants = FastUniformBitsURBGConstants<URBG>; - if (constants::kPowerOfTwo) { - return g() - (URBG::min)(); - } - - // Use rejection sampling to ensure uniformity across the range. - typename URBG::result_type u; - do { - u = g() - (URBG::min)(); - } while (u > constants::kRangeMask); - return u; + constexpr urbg_result_type kRangeMask = + RangeSize<URBG>() == 0 + ? (std::numeric_limits<urbg_result_type>::max)() + : static_cast<urbg_result_type>(PowerOfTwoSubRangeSize<URBG>() - 1); + return Generate(g, std::integral_constant<bool, (kRangeMask >= (max)())>{}); } template <typename UIntType> template <typename URBG> typename FastUniformBits<UIntType>::result_type -FastUniformBits<UIntType>::Generate( - URBG& g, // NOLINT(runtime/references) - std::true_type /* avoid_looping */) { +FastUniformBits<UIntType>::Generate(URBG& g, // NOLINT(runtime/references) + std::true_type /* avoid_looping */) { // The width of the result_type is less than than the width of the random bits - // provided by URNG. Thus, generate a single value and then simply mask off + // provided by URBG. Thus, generate a single value and then simply mask off // the required bits. - return Variate(g) & (max)(); + + return PowerOfTwoVariate(g) & (max)(); } template <typename UIntType> template <typename URBG> typename FastUniformBits<UIntType>::result_type -FastUniformBits<UIntType>::Generate( - URBG& g, // NOLINT(runtime/references) - std::false_type /* avoid_looping */) { - // The width of the result_type is wider than the number of random bits - // provided by URNG. Thus we merge several variates of URNG into the result - // using a shift and mask. The constants type generates the parameters used - // ensure that the bits are distributed across all the invocations of the - // underlying URNG. - using constants = FastUniformBitsLoopingConstants<UIntType, URBG>; +FastUniformBits<UIntType>::Generate(URBG& g, // NOLINT(runtime/references) + std::false_type /* avoid_looping */) { + // See [rand.adapt.ibits] for more details on the constants calculated below. + // + // It is preferable to use roughly the same number of bits from each generator + // call, however this is only possible when the number of bits provided by the + // URBG is a divisor of the number of bits in `result_type`. In all other + // cases, the number of bits used cannot always be the same, but it can be + // guaranteed to be off by at most 1. Thus we run two loops, one with a + // smaller bit-width size (`kSmallWidth`) and one with a larger width size + // (satisfying `kLargeWidth == kSmallWidth + 1`). The loops are run + // `kSmallIters` and `kLargeIters` times respectively such + // that + // + // `kTotalWidth == kSmallIters * kSmallWidth + // + kLargeIters * kLargeWidth` + // + // where `kTotalWidth` is the total number of bits in `result_type`. + // + constexpr size_t kTotalWidth = std::numeric_limits<result_type>::digits; + constexpr size_t kUrbgWidth = NumBits<URBG>(); + constexpr size_t kTotalIters = + kTotalWidth / kUrbgWidth + (kTotalWidth % kUrbgWidth != 0); + constexpr size_t kSmallWidth = kTotalWidth / kTotalIters; + constexpr size_t kLargeWidth = kSmallWidth + 1; + // + // Because `kLargeWidth == kSmallWidth + 1`, it follows that + // + // `kTotalWidth == kTotalIters * kSmallWidth + kLargeIters` + // + // and therefore + // + // `kLargeIters == kTotalWidth % kSmallWidth` + // + // Intuitively, each iteration with the large width accounts for one unit + // of the remainder when `kTotalWidth` is divided by `kSmallWidth`. As + // mentioned above, if the URBG width is a divisor of `kTotalWidth`, then + // there would be no need for any large iterations (i.e., one loop would + // suffice), and indeed, in this case, `kLargeIters` would be zero. + constexpr size_t kLargeIters = kTotalWidth % kSmallWidth; + constexpr size_t kSmallIters = + (kTotalWidth - (kLargeWidth * kLargeIters)) / kSmallWidth; + + static_assert( + kTotalWidth == kSmallIters * kSmallWidth + kLargeIters * kLargeWidth, + "Error in looping constant calculations."); result_type s = 0; - for (size_t n = 0; n < constants::kN0; ++n) { - auto u = Variate(g); - s = (s << constants::kW0) + (u & constants::kM0); + + constexpr size_t kSmallShift = kSmallWidth % kTotalWidth; + constexpr result_type kSmallMask = MaskFromShift(result_type{kSmallShift}); + for (size_t n = 0; n < kSmallIters; ++n) { + s = (s << kSmallShift) + + (static_cast<result_type>(PowerOfTwoVariate(g)) & kSmallMask); } - for (size_t n = constants::kN0; n < constants::kN1; ++n) { - auto u = Variate(g); - s = (s << constants::kW1) + (u & constants::kM1); + + constexpr size_t kLargeShift = kLargeWidth % kTotalWidth; + constexpr result_type kLargeMask = MaskFromShift(result_type{kLargeShift}); + for (size_t n = 0; n < kLargeIters; ++n) { + s = (s << kLargeShift) + + (static_cast<result_type>(PowerOfTwoVariate(g)) & kLargeMask); } + + static_assert( + kLargeShift == kSmallShift + 1 || + (kLargeShift == 0 && + kSmallShift == std::numeric_limits<result_type>::digits - 1), + "Error in looping constant calculations"); + return s; } diff --git a/absl/random/internal/fast_uniform_bits_test.cc b/absl/random/internal/fast_uniform_bits_test.cc index 183779445a52..9f2e82687dd6 100644 --- a/absl/random/internal/fast_uniform_bits_test.cc +++ b/absl/random/internal/fast_uniform_bits_test.cc @@ -18,6 +18,8 @@ #include "gtest/gtest.h" +namespace absl { +namespace random_internal { namespace { template <typename IntType> @@ -29,7 +31,7 @@ TYPED_TEST_SUITE(FastUniformBitsTypedTest, IntTypes); TYPED_TEST(FastUniformBitsTypedTest, BasicTest) { using Limits = std::numeric_limits<TypeParam>; - using FastBits = absl::random_internal::FastUniformBits<TypeParam>; + using FastBits = FastUniformBits<TypeParam>; EXPECT_EQ(0, FastBits::min()); EXPECT_EQ(Limits::max(), FastBits::max()); @@ -45,91 +47,226 @@ TYPED_TEST(FastUniformBitsTypedTest, BasicTest) { } } -class UrngOddbits { - public: - using result_type = uint8_t; - static constexpr result_type min() { return 1; } - static constexpr result_type max() { return 0xfe; } - result_type operator()() { return 2; } -}; +template <typename UIntType, UIntType Lo, UIntType Hi, UIntType Val = Lo> +struct FakeUrbg { + using result_type = UIntType; -class Urng4bits { - public: - using result_type = uint8_t; - static constexpr result_type min() { return 1; } - static constexpr result_type max() { return 0xf + 1; } - result_type operator()() { return 2; } + static constexpr result_type(max)() { return Hi; } + static constexpr result_type(min)() { return Lo; } + result_type operator()() { return Val; } }; -class Urng32bits { - public: - using result_type = uint32_t; - static constexpr result_type min() { return 0; } - static constexpr result_type max() { return 0xffffffff; } - result_type operator()() { return 1; } -}; +using UrngOddbits = FakeUrbg<uint8_t, 1, 0xfe, 0x73>; +using Urng4bits = FakeUrbg<uint8_t, 1, 0x10, 2>; +using Urng31bits = FakeUrbg<uint32_t, 1, 0xfffffffe, 0x60070f03>; +using Urng32bits = FakeUrbg<uint32_t, 0, 0xffffffff, 0x74010f01>; -// Compile-time test to validate the helper classes used by FastUniformBits -TEST(FastUniformBitsTest, FastUniformBitsDetails) { - using absl::random_internal::FastUniformBitsLoopingConstants; - using absl::random_internal::FastUniformBitsURBGConstants; +TEST(FastUniformBitsTest, IsPowerOfTwoOrZero) { + EXPECT_TRUE(IsPowerOfTwoOrZero(uint8_t{0})); + EXPECT_TRUE(IsPowerOfTwoOrZero(uint8_t{1})); + EXPECT_TRUE(IsPowerOfTwoOrZero(uint8_t{2})); + EXPECT_FALSE(IsPowerOfTwoOrZero(uint8_t{3})); + EXPECT_TRUE(IsPowerOfTwoOrZero(uint8_t{16})); + EXPECT_FALSE(IsPowerOfTwoOrZero(uint8_t{17})); + EXPECT_FALSE(IsPowerOfTwoOrZero((std::numeric_limits<uint8_t>::max)())); - // 4-bit URBG - { - using constants = FastUniformBitsURBGConstants<Urng4bits>; - static_assert(constants::kPowerOfTwo == true, - "constants::kPowerOfTwo == false"); - static_assert(constants::kRange == 16, "constants::kRange == false"); - static_assert(constants::kRangeBits == 4, "constants::kRangeBits == false"); - static_assert(constants::kRangeMask == 0x0f, - "constants::kRangeMask == false"); - } + EXPECT_TRUE(IsPowerOfTwoOrZero(uint16_t{0})); + EXPECT_TRUE(IsPowerOfTwoOrZero(uint16_t{1})); + EXPECT_TRUE(IsPowerOfTwoOrZero(uint16_t{2})); + EXPECT_FALSE(IsPowerOfTwoOrZero(uint16_t{3})); + EXPECT_TRUE(IsPowerOfTwoOrZero(uint16_t{16})); + EXPECT_FALSE(IsPowerOfTwoOrZero(uint16_t{17})); + EXPECT_FALSE(IsPowerOfTwoOrZero((std::numeric_limits<uint16_t>::max)())); - // ~7-bit URBG - { - using constants = FastUniformBitsURBGConstants<UrngOddbits>; - static_assert(constants::kPowerOfTwo == false, - "constants::kPowerOfTwo == false"); - static_assert(constants::kRange == 0xfe, "constants::kRange == 0xfe"); - static_assert(constants::kRangeBits == 7, "constants::kRangeBits == 7"); - static_assert(constants::kRangeMask == 0x7f, - "constants::kRangeMask == 0x7f"); - } + EXPECT_TRUE(IsPowerOfTwoOrZero(uint32_t{0})); + EXPECT_TRUE(IsPowerOfTwoOrZero(uint32_t{1})); + EXPECT_TRUE(IsPowerOfTwoOrZero(uint32_t{2})); + EXPECT_FALSE(IsPowerOfTwoOrZero(uint32_t{3})); + EXPECT_TRUE(IsPowerOfTwoOrZero(uint32_t{32})); + EXPECT_FALSE(IsPowerOfTwoOrZero(uint32_t{17})); + EXPECT_FALSE(IsPowerOfTwoOrZero((std::numeric_limits<uint32_t>::max)())); + + EXPECT_TRUE(IsPowerOfTwoOrZero(uint64_t{0})); + EXPECT_TRUE(IsPowerOfTwoOrZero(uint64_t{1})); + EXPECT_TRUE(IsPowerOfTwoOrZero(uint64_t{2})); + EXPECT_FALSE(IsPowerOfTwoOrZero(uint64_t{3})); + EXPECT_TRUE(IsPowerOfTwoOrZero(uint64_t{64})); + EXPECT_FALSE(IsPowerOfTwoOrZero(uint64_t{17})); + EXPECT_FALSE(IsPowerOfTwoOrZero((std::numeric_limits<uint64_t>::max)())); +} + +TEST(FastUniformBitsTest, IntegerLog2) { + EXPECT_EQ(IntegerLog2(uint16_t{0}), 0); + EXPECT_EQ(IntegerLog2(uint16_t{1}), 0); + EXPECT_EQ(IntegerLog2(uint16_t{2}), 1); + EXPECT_EQ(IntegerLog2(uint16_t{3}), 1); + EXPECT_EQ(IntegerLog2(uint16_t{4}), 2); + EXPECT_EQ(IntegerLog2(uint16_t{5}), 2); + EXPECT_EQ(IntegerLog2(std::numeric_limits<uint64_t>::max()), 63); +} + +TEST(FastUniformBitsTest, RangeSize) { + EXPECT_EQ((RangeSize<FakeUrbg<uint8_t, 0, 3>>()), 4); + EXPECT_EQ((RangeSize<FakeUrbg<uint8_t, 2, 2>>()), 1); + EXPECT_EQ((RangeSize<FakeUrbg<uint8_t, 2, 5>>()), 4); + EXPECT_EQ((RangeSize<FakeUrbg<uint8_t, 2, 6>>()), 5); + EXPECT_EQ((RangeSize<FakeUrbg<uint8_t, 2, 10>>()), 9); + EXPECT_EQ( + (RangeSize<FakeUrbg<uint8_t, 0, std::numeric_limits<uint8_t>::max()>>()), + 0); + + EXPECT_EQ((RangeSize<FakeUrbg<uint16_t, 0, 3>>()), 4); + EXPECT_EQ((RangeSize<FakeUrbg<uint16_t, 2, 2>>()), 1); + EXPECT_EQ((RangeSize<FakeUrbg<uint16_t, 2, 5>>()), 4); + EXPECT_EQ((RangeSize<FakeUrbg<uint16_t, 2, 6>>()), 5); + EXPECT_EQ((RangeSize<FakeUrbg<uint16_t, 1000, 1017>>()), 18); + EXPECT_EQ((RangeSize< + FakeUrbg<uint16_t, 0, std::numeric_limits<uint16_t>::max()>>()), + 0); + + EXPECT_EQ((RangeSize<FakeUrbg<uint32_t, 0, 3>>()), 4); + EXPECT_EQ((RangeSize<FakeUrbg<uint32_t, 2, 2>>()), 1); + EXPECT_EQ((RangeSize<FakeUrbg<uint32_t, 2, 5>>()), 4); + EXPECT_EQ((RangeSize<FakeUrbg<uint32_t, 2, 6>>()), 5); + EXPECT_EQ((RangeSize<FakeUrbg<uint32_t, 1000, 1017>>()), 18); + EXPECT_EQ((RangeSize<FakeUrbg<uint32_t, 0, 0xffffffff>>()), 0); + EXPECT_EQ((RangeSize<FakeUrbg<uint32_t, 1, 0xffffffff>>()), 0xffffffff); + EXPECT_EQ((RangeSize<FakeUrbg<uint32_t, 1, 0xfffffffe>>()), 0xfffffffe); + EXPECT_EQ((RangeSize<FakeUrbg<uint32_t, 2, 0xfffffffe>>()), 0xfffffffd); + EXPECT_EQ((RangeSize< + FakeUrbg<uint32_t, 0, std::numeric_limits<uint32_t>::max()>>()), + 0); + + EXPECT_EQ((RangeSize<FakeUrbg<uint64_t, 0, 3>>()), 4); + EXPECT_EQ((RangeSize<FakeUrbg<uint64_t, 2, 2>>()), 1); + EXPECT_EQ((RangeSize<FakeUrbg<uint64_t, 2, 5>>()), 4); + EXPECT_EQ((RangeSize<FakeUrbg<uint64_t, 2, 6>>()), 5); + EXPECT_EQ((RangeSize<FakeUrbg<uint64_t, 1000, 1017>>()), 18); + EXPECT_EQ((RangeSize<FakeUrbg<uint64_t, 0, 0xffffffff>>()), 0x100000000ull); + EXPECT_EQ((RangeSize<FakeUrbg<uint64_t, 1, 0xffffffff>>()), 0xffffffffull); + EXPECT_EQ((RangeSize<FakeUrbg<uint64_t, 1, 0xfffffffe>>()), 0xfffffffeull); + EXPECT_EQ((RangeSize<FakeUrbg<uint64_t, 2, 0xfffffffe>>()), 0xfffffffdull); + EXPECT_EQ((RangeSize<FakeUrbg<uint64_t, 0, 0xffffffffffffffffull>>()), 0ull); + EXPECT_EQ((RangeSize<FakeUrbg<uint64_t, 1, 0xffffffffffffffffull>>()), + 0xffffffffffffffffull); + EXPECT_EQ((RangeSize<FakeUrbg<uint64_t, 1, 0xfffffffffffffffeull>>()), + 0xfffffffffffffffeull); + EXPECT_EQ((RangeSize<FakeUrbg<uint64_t, 2, 0xfffffffffffffffeull>>()), + 0xfffffffffffffffdull); + EXPECT_EQ((RangeSize< + FakeUrbg<uint64_t, 0, std::numeric_limits<uint64_t>::max()>>()), + 0); +} + +TEST(FastUniformBitsTest, PowerOfTwoSubRangeSize) { + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint8_t, 0, 3>>()), 4); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint8_t, 2, 2>>()), 1); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint8_t, 2, 5>>()), 4); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint8_t, 2, 6>>()), 4); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint8_t, 2, 10>>()), 8); + EXPECT_EQ((PowerOfTwoSubRangeSize< + FakeUrbg<uint8_t, 0, std::numeric_limits<uint8_t>::max()>>()), + 0); + + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint16_t, 0, 3>>()), 4); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint16_t, 2, 2>>()), 1); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint16_t, 2, 5>>()), 4); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint16_t, 2, 6>>()), 4); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint16_t, 1000, 1017>>()), 16); + EXPECT_EQ((PowerOfTwoSubRangeSize< + FakeUrbg<uint16_t, 0, std::numeric_limits<uint16_t>::max()>>()), + 0); + + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint32_t, 0, 3>>()), 4); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint32_t, 2, 2>>()), 1); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint32_t, 2, 5>>()), 4); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint32_t, 2, 6>>()), 4); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint32_t, 1000, 1017>>()), 16); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint32_t, 0, 0xffffffff>>()), 0); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint32_t, 1, 0xffffffff>>()), + 0x80000000); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint32_t, 1, 0xfffffffe>>()), + 0x80000000); + EXPECT_EQ((PowerOfTwoSubRangeSize< + FakeUrbg<uint32_t, 0, std::numeric_limits<uint32_t>::max()>>()), + 0); + + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint64_t, 0, 3>>()), 4); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint64_t, 2, 2>>()), 1); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint64_t, 2, 5>>()), 4); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint64_t, 2, 6>>()), 4); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint64_t, 1000, 1017>>()), 16); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint64_t, 0, 0xffffffff>>()), + 0x100000000ull); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint64_t, 1, 0xffffffff>>()), + 0x80000000ull); + EXPECT_EQ((PowerOfTwoSubRangeSize<FakeUrbg<uint64_t, 1, 0xfffffffe>>()), + 0x80000000ull); + EXPECT_EQ( + (PowerOfTwoSubRangeSize<FakeUrbg<uint64_t, 0, 0xffffffffffffffffull>>()), + 0); + EXPECT_EQ( + (PowerOfTwoSubRangeSize<FakeUrbg<uint64_t, 1, 0xffffffffffffffffull>>()), + 0x8000000000000000ull); + EXPECT_EQ( + (PowerOfTwoSubRangeSize<FakeUrbg<uint64_t, 1, 0xfffffffffffffffeull>>()), + 0x8000000000000000ull); + EXPECT_EQ((PowerOfTwoSubRangeSize< + FakeUrbg<uint64_t, 0, std::numeric_limits<uint64_t>::max()>>()), + 0); } TEST(FastUniformBitsTest, Urng4_VariousOutputs) { // Tests that how values are composed; the single-bit deltas should be spread // across each invocation. Urng4bits urng4; + Urng31bits urng31; Urng32bits urng32; // 8-bit types { - absl::random_internal::FastUniformBits<uint8_t> fast8; + FastUniformBits<uint8_t> fast8; EXPECT_EQ(0x11, fast8(urng4)); + EXPECT_EQ(0x2, fast8(urng31)); EXPECT_EQ(0x1, fast8(urng32)); } // 16-bit types { - absl::random_internal::FastUniformBits<uint16_t> fast16; + FastUniformBits<uint16_t> fast16; EXPECT_EQ(0x1111, fast16(urng4)); - EXPECT_EQ(0x1, fast16(urng32)); + EXPECT_EQ(0xf02, fast16(urng31)); + EXPECT_EQ(0xf01, fast16(urng32)); } // 32-bit types { - absl::random_internal::FastUniformBits<uint32_t> fast32; + FastUniformBits<uint32_t> fast32; EXPECT_EQ(0x11111111, fast32(urng4)); - EXPECT_EQ(0x1, fast32(urng32)); + EXPECT_EQ(0x0f020f02, fast32(urng31)); + EXPECT_EQ(0x74010f01, fast32(urng32)); } // 64-bit types { - absl::random_internal::FastUniformBits<uint64_t> fast64; + FastUniformBits<uint64_t> fast64; EXPECT_EQ(0x1111111111111111, fast64(urng4)); - EXPECT_EQ(0x0000000100000001, fast64(urng32)); + EXPECT_EQ(0x387811c3c0870f02, fast64(urng31)); + EXPECT_EQ(0x74010f0174010f01, fast64(urng32)); } } +TEST(FastUniformBitsTest, URBG32bitRegression) { + // Validate with deterministic 32-bit std::minstd_rand + // to ensure that operator() performs as expected. + std::minstd_rand gen(1); + FastUniformBits<uint64_t> fast64; + + EXPECT_EQ(0x05e47095f847c122ull, fast64(gen)); + EXPECT_EQ(0x8f82c1ba30b64d22ull, fast64(gen)); + EXPECT_EQ(0x3b971a3558155039ull, fast64(gen)); +} + } // namespace +} // namespace random_internal +} // namespace absl diff --git a/absl/strings/str_join.h b/absl/strings/str_join.h index 7345b9627bdd..4772f5d17c89 100644 --- a/absl/strings/str_join.h +++ b/absl/strings/str_join.h @@ -254,7 +254,7 @@ std::string StrJoin(const Range& range, absl::string_view separator, template <typename T, typename Formatter> std::string StrJoin(std::initializer_list<T> il, absl::string_view separator, - Formatter&& fmt) { + Formatter&& fmt) { return strings_internal::JoinRange(il, separator, fmt); } @@ -275,7 +275,8 @@ std::string StrJoin(const Range& range, absl::string_view separator) { } template <typename T> -std::string StrJoin(std::initializer_list<T> il, absl::string_view separator) { +std::string StrJoin(std::initializer_list<T> il, + absl::string_view separator) { return strings_internal::JoinRange(il, separator); } diff --git a/absl/synchronization/internal/create_thread_identity.cc b/absl/synchronization/internal/create_thread_identity.cc index 6e93605de061..ce3676a88482 100644 --- a/absl/synchronization/internal/create_thread_identity.cc +++ b/absl/synchronization/internal/create_thread_identity.cc @@ -31,7 +31,8 @@ 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::SpinLock freelist_lock( + base_internal::kLinkerInitialized); static base_internal::ThreadIdentity* thread_identity_freelist; // A per-thread destructor for reclaiming associated ThreadIdentity objects. diff --git a/absl/time/time.h b/absl/time/time.h index 0534780516d0..e236870bd829 100644 --- a/absl/time/time.h +++ b/absl/time/time.h @@ -179,6 +179,7 @@ class Duration { Duration& operator%=(Duration rhs); // Overloads that forward to either the int64_t or double overloads above. + // Integer operands must be representable as int64_t. template <typename T> Duration& operator*=(T r) { int64_t x = r; @@ -221,6 +222,7 @@ inline Duration operator+(Duration lhs, Duration rhs) { return lhs += rhs; } inline Duration operator-(Duration lhs, Duration rhs) { return lhs -= rhs; } // Multiplicative Operators +// Integer operands must be representable as int64_t. template <typename T> Duration operator*(Duration lhs, T rhs) { return lhs *= rhs; @@ -375,7 +377,8 @@ constexpr Duration InfiniteDuration(); // Hours() // // Factory functions for constructing `Duration` values from an integral number -// of the unit indicated by the factory function's name. +// of the unit indicated by the factory function's name. The number must be +// representable as int64_t. // // Note: no "Days()" factory function exists because "a day" is ambiguous. // Civil days are not always 24 hours long, and a 24-hour duration often does |