From b69c7d880caddfc25bf348dbcfe9d45fdd8bc6e6 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Fri, 21 Feb 2020 12:51:36 -0800 Subject: Export of internal Abseil changes -- 00f5301405423005d9129935c05f20155536cc1a by CJ Johnson : Removes usage of std::aligned_storage from Abseil implementation details PiperOrigin-RevId: 296492301 -- fc11d15f91764612fba080669d2381dc181df52b by Abseil Team : Fix absl::bind_front documentation. PiperOrigin-RevId: 296482945 -- 0164c595c129c46bf21ae74eba5399a1da5f140b by Gennadiy Rozental : Automated g4 rollback of changelist 296320700. PiperOrigin-RevId: 296439968 -- 1eb295700758ca0894d872b2de7c675b4ad679af by Abseil Team : Removes duplicate comments. PiperOrigin-RevId: 296433214 -- c30c01caae02d2fa4ef783d988de6bebb9757c39 by Derek Mauro : Merge GitHub #621: Add RISCV support to GetProgramCounter() Fixes #621 PiperOrigin-RevId: 296351174 -- 95d4498167596fd7543e025bdfe9a8da9e2ca3c8 by Abseil Team : Automated g4 rollback of changelist 296320700. PiperOrigin-RevId: 296348701 -- b193f0543e0cec54dddb2ed51f45dc489c8d06d5 by Gennadiy Rozental : Change TryParse interface to return managed value. In addition introduce companion StoreValue routine which consumes pointer to source value and stores the value inside of FlagImpl. In a follow up CL we will change StoreValue implementation to behave differently depending on "value storage kind". We also rename default_src_ to default_value_. PiperOrigin-RevId: 296320700 -- 57e942b485d12912a0a8d0d0b35fa2a62847020f by Derek Mauro : Merge GitHub #622 * Add missing #ifdef conditionals for ABSL_HAVE_VDSO_SUPPORT PiperOrigin-RevId: 296272830 GitOrigin-RevId: 00f5301405423005d9129935c05f20155536cc1a Change-Id: I1b05eeaf1280f95fb0a2c5f3654995a87c792893 --- absl/base/exception_safety_testing_test.cc | 18 +++--- absl/base/internal/low_level_alloc.cc | 13 ++-- absl/base/options.h | 8 --- absl/container/fixed_array_test.cc | 7 +-- absl/container/internal/common.h | 7 +-- absl/container/internal/raw_hash_set.h | 6 +- absl/debugging/internal/stacktrace_x86-inl.inc | 4 +- absl/flags/internal/flag.cc | 81 +++++++++++-------------- absl/flags/internal/flag.h | 25 ++++---- absl/flags/internal/type_erased_test.cc | 7 +++ absl/functional/bind_front.h | 26 ++++++-- absl/memory/memory_exception_safety_test.cc | 3 - absl/strings/cord.cc | 8 +-- absl/synchronization/internal/mutex_nonprod.inc | 4 +- absl/synchronization/internal/waiter.cc | 34 +++++------ absl/synchronization/internal/waiter.h | 13 ++-- 16 files changed, 130 insertions(+), 134 deletions(-) diff --git a/absl/base/exception_safety_testing_test.cc b/absl/base/exception_safety_testing_test.cc index 575b535..a59be29 100644 --- a/absl/base/exception_safety_testing_test.cc +++ b/absl/base/exception_safety_testing_test.cc @@ -328,17 +328,15 @@ TEST(ThrowingValueTest, NonThrowingDelete) { UnsetCountdown(); } -using Storage = - absl::aligned_storage_t), alignof(ThrowingValue<>)>; - TEST(ThrowingValueTest, NonThrowingPlacementDelete) { constexpr int kArrayLen = 2; // We intentionally create extra space to store the tag allocated by placement // new[]. constexpr int kStorageLen = 4; - Storage buf; - Storage array_buf[kStorageLen]; + alignas(ThrowingValue<>) unsigned char buf[sizeof(ThrowingValue<>)]; + alignas(ThrowingValue<>) unsigned char + array_buf[sizeof(ThrowingValue<>[kStorageLen])]; auto* placed = new (&buf) ThrowingValue<>(1); auto placed_array = new (&array_buf) ThrowingValue<>[kArrayLen]; @@ -902,12 +900,12 @@ TEST(ConstructorTrackerTest, CreatedAfter) { } TEST(ConstructorTrackerTest, NotDestroyedAfter) { - absl::aligned_storage_t storage; + alignas(Tracked) unsigned char storage[sizeof(Tracked)]; EXPECT_NONFATAL_FAILURE( { exceptions_internal::ConstructorTracker ct( exceptions_internal::countdown); - new (&storage) Tracked; + new (&storage) Tracked(); }, "not destroyed"); } @@ -924,11 +922,11 @@ TEST(ConstructorTrackerTest, DestroyedTwice) { TEST(ConstructorTrackerTest, ConstructedTwice) { exceptions_internal::ConstructorTracker ct(exceptions_internal::countdown); - absl::aligned_storage_t storage; + alignas(Tracked) unsigned char storage[sizeof(Tracked)]; EXPECT_NONFATAL_FAILURE( { - new (&storage) Tracked; - new (&storage) Tracked; + new (&storage) Tracked(); + new (&storage) Tracked(); reinterpret_cast(&storage)->~Tracked(); }, "re-constructed"); diff --git a/absl/base/internal/low_level_alloc.cc b/absl/base/internal/low_level_alloc.cc index 225abc2..1bf9443 100644 --- a/absl/base/internal/low_level_alloc.cc +++ b/absl/base/internal/low_level_alloc.cc @@ -220,16 +220,17 @@ struct LowLevelAlloc::Arena { }; namespace { -using ArenaStorage = std::aligned_storage::type; - // Static storage space for the lazily-constructed, default global arena // instances. We require this space because the whole point of LowLevelAlloc // is to avoid relying on malloc/new. -ArenaStorage default_arena_storage; -ArenaStorage unhooked_arena_storage; +alignas(LowLevelAlloc::Arena) unsigned char default_arena_storage[sizeof( + LowLevelAlloc::Arena)]; +alignas(LowLevelAlloc::Arena) unsigned char unhooked_arena_storage[sizeof( + LowLevelAlloc::Arena)]; #ifndef ABSL_LOW_LEVEL_ALLOC_ASYNC_SIGNAL_SAFE_MISSING -ArenaStorage unhooked_async_sig_safe_arena_storage; +alignas( + LowLevelAlloc::Arena) unsigned char unhooked_async_sig_safe_arena_storage + [sizeof(LowLevelAlloc::Arena)]; #endif // We must use LowLevelCallOnce here to construct the global arenas, rather than diff --git a/absl/base/options.h b/absl/base/options.h index 6868c77..234137c 100644 --- a/absl/base/options.h +++ b/absl/base/options.h @@ -123,14 +123,6 @@ // compiler flags passed by the end user. For more info, see // https://abseil.io/about/design/dropin-types. -// A value of 2 means to detect the C++ version being used to compile Abseil, -// and use an alias only if a working std::optional is available. This option -// should not be used when your program is not built from source -- for example, -// if you are distributing Abseil in a binary package manager -- since in mode -// 2, absl::optional will name a different template class, with a different -// mangled name and binary layout, depending on the compiler flags passed by the -// end user. -// // User code should not inspect this macro. To check in the preprocessor if // absl::optional is a typedef of std::optional, use the feature macro // ABSL_USES_STD_OPTIONAL. diff --git a/absl/container/fixed_array_test.cc b/absl/container/fixed_array_test.cc index 2b1cf47..c960fe5 100644 --- a/absl/container/fixed_array_test.cc +++ b/absl/container/fixed_array_test.cc @@ -604,19 +604,16 @@ TEST(FixedArrayTest, Fill) { empty.fill(fill_val); } -// TODO(johnsoncj): Investigate InlinedStorage default initialization in GCC 4.x #ifndef __GNUC__ TEST(FixedArrayTest, DefaultCtorDoesNotValueInit) { using T = char; constexpr auto capacity = 10; using FixedArrType = absl::FixedArray; - using FixedArrBuffType = - absl::aligned_storage_t; constexpr auto scrubbed_bits = 0x95; constexpr auto length = capacity / 2; - FixedArrBuffType buff; - std::memset(std::addressof(buff), scrubbed_bits, sizeof(FixedArrBuffType)); + alignas(FixedArrType) unsigned char buff[sizeof(FixedArrType)]; + std::memset(std::addressof(buff), scrubbed_bits, sizeof(FixedArrType)); FixedArrType* arr = ::new (static_cast(std::addressof(buff))) FixedArrType(length); diff --git a/absl/container/internal/common.h b/absl/container/internal/common.h index 853a5b2..5037d80 100644 --- a/absl/container/internal/common.h +++ b/absl/container/internal/common.h @@ -56,7 +56,7 @@ class node_handle_base { public: using allocator_type = Alloc; - constexpr node_handle_base() {} + constexpr node_handle_base() = default; node_handle_base(node_handle_base&& other) noexcept { *this = std::move(other); } @@ -109,9 +109,8 @@ class node_handle_base { allocator_type* alloc() { return std::addressof(*alloc_); } private: - absl::optional alloc_; - mutable absl::aligned_storage_t - slot_space_; + absl::optional alloc_ = {}; + alignas(slot_type) mutable unsigned char slot_space_[sizeof(slot_type)] = {}; }; // For sets. diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index 0d3d604..ca7be8d 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -1067,8 +1067,7 @@ class raw_hash_set { template ::value, int>::type = 0> std::pair emplace(Args&&... args) { - typename std::aligned_storage::type - raw; + alignas(slot_type) unsigned char raw[sizeof(slot_type)]; slot_type* slot = reinterpret_cast(&raw); PolicyTraits::construct(&alloc_ref(), slot, std::forward(args)...); @@ -1556,8 +1555,7 @@ class raw_hash_set { // mark target as FULL // repeat procedure for current slot with moved from element (target) ConvertDeletedToEmptyAndFullToDeleted(ctrl_, capacity_); - typename std::aligned_storage::type - raw; + alignas(slot_type) unsigned char raw[sizeof(slot_type)]; size_t total_probe_length = 0; slot_type* slot = reinterpret_cast(&raw); for (size_t i = 0; i != capacity_; ++i) { diff --git a/absl/debugging/internal/stacktrace_x86-inl.inc b/absl/debugging/internal/stacktrace_x86-inl.inc index f87cafb..bc320ff 100644 --- a/absl/debugging/internal/stacktrace_x86-inl.inc +++ b/absl/debugging/internal/stacktrace_x86-inl.inc @@ -202,9 +202,9 @@ static void **NextStackFrame(void **old_fp, const void *uc) { } else { num_push_instructions = 0; } -#else +#else // ABSL_HAVE_VDSO_SUPPORT num_push_instructions = 0; -#endif +#endif // ABSL_HAVE_VDSO_SUPPORT } if (num_push_instructions != 0 && kernel_rt_sigreturn_address != nullptr && old_fp[1] == kernel_rt_sigreturn_address) { diff --git a/absl/flags/internal/flag.cc b/absl/flags/internal/flag.cc index 83ec8df..5a921e2 100644 --- a/absl/flags/internal/flag.cc +++ b/absl/flags/internal/flag.cc @@ -121,13 +121,21 @@ void FlagImpl::AssertValidType(FlagStaticTypeId type_id) const { std::unique_ptr FlagImpl::MakeInitValue() const { void* res = nullptr; if (DefaultKind() == FlagDefaultKind::kDynamicValue) { - res = flags_internal::Clone(op_, default_src_.dynamic_value); + res = flags_internal::Clone(op_, default_value_.dynamic_value); } else { - res = (*default_src_.gen_func)(); + res = (*default_value_.gen_func)(); } return {res, DynValueDeleter{op_}}; } +void FlagImpl::StoreValue(const void* src) { + flags_internal::Copy(op_, src, value_.dynamic); + StoreAtomic(); + modified_ = true; + ++counter_; + InvokeCallback(); +} + absl::string_view FlagImpl::Name() const { return name_; } std::string FlagImpl::Filename() const { @@ -220,23 +228,19 @@ bool FlagImpl::RestoreState(const void* value, bool modified, // argument. If parsing successful, this function replaces the dst with newly // parsed value. In case if any error is encountered in either step, the error // message is stored in 'err' -bool FlagImpl::TryParse(void** dst, absl::string_view value, - std::string* err) const { - auto tentative_value = MakeInitValue(); +std::unique_ptr FlagImpl::TryParse( + absl::string_view value, std::string* err) const { + std::unique_ptr tentative_value = MakeInitValue(); std::string parse_err; if (!flags_internal::Parse(op_, value, tentative_value.get(), &parse_err)) { absl::string_view err_sep = parse_err.empty() ? "" : "; "; *err = absl::StrCat("Illegal value '", value, "' specified for flag '", Name(), "'", err_sep, parse_err); - return false; + return nullptr; } - void* old_val = *dst; - *dst = tentative_value.release(); - tentative_value.reset(old_val); - - return true; + return tentative_value; } void FlagImpl::Read(void* dst) const { @@ -266,22 +270,17 @@ void FlagImpl::Write(const void* src) { absl::MutexLock l(DataGuard()); if (ShouldValidateFlagValue(flags_internal::StaticTypeId(op_))) { - void* obj = flags_internal::Clone(op_, src); + std::unique_ptr obj{flags_internal::Clone(op_, src), + DynValueDeleter{op_}}; std::string ignored_error; std::string src_as_str = flags_internal::Unparse(op_, src); - if (!flags_internal::Parse(op_, src_as_str, obj, &ignored_error)) { + if (!flags_internal::Parse(op_, src_as_str, obj.get(), &ignored_error)) { ABSL_INTERNAL_LOG(ERROR, absl::StrCat("Attempt to set flag '", Name(), "' to invalid value ", src_as_str)); } - flags_internal::Delete(op_, obj); } - modified_ = true; - counter_++; - flags_internal::Copy(op_, src, value_.dynamic); - - StoreAtomic(); - InvokeCallback(); + StoreValue(src); } // Sets the value of the flag based on specified string `value`. If the flag @@ -299,11 +298,10 @@ bool FlagImpl::SetFromString(absl::string_view value, FlagSettingMode set_mode, switch (set_mode) { case SET_FLAGS_VALUE: { // set or modify the flag's value - if (!TryParse(&value_.dynamic, value, err)) return false; - modified_ = true; - counter_++; - StoreAtomic(); - InvokeCallback(); + auto tentative_value = TryParse(value, err); + if (!tentative_value) return false; + + StoreValue(tentative_value.get()); if (source == kCommandLine) { on_command_line_ = true; @@ -312,13 +310,7 @@ bool FlagImpl::SetFromString(absl::string_view value, FlagSettingMode set_mode, } case SET_FLAG_IF_DEFAULT: { // set the flag's value, but only if it hasn't been set by someone else - if (!modified_) { - if (!TryParse(&value_.dynamic, value, err)) return false; - modified_ = true; - counter_++; - StoreAtomic(); - InvokeCallback(); - } else { + if (modified_) { // TODO(rogeeff): review and fix this semantic. Currently we do not fail // in this case if flag is modified. This is misleading since the flag's // value is not updated even though we return true. @@ -327,28 +319,29 @@ bool FlagImpl::SetFromString(absl::string_view value, FlagSettingMode set_mode, // return false; return true; } + auto tentative_value = TryParse(value, err); + if (!tentative_value) return false; + + StoreValue(tentative_value.get()); break; } case SET_FLAGS_DEFAULT: { + auto tentative_value = TryParse(value, err); + if (!tentative_value) return false; + if (DefaultKind() == FlagDefaultKind::kDynamicValue) { - if (!TryParse(&default_src_.dynamic_value, value, err)) { - return false; - } + void* old_value = default_value_.dynamic_value; + default_value_.dynamic_value = tentative_value.release(); + tentative_value.reset(old_value); } else { - void* new_default_val = nullptr; - if (!TryParse(&new_default_val, value, err)) { - return false; - } - - default_src_.dynamic_value = new_default_val; + default_value_.dynamic_value = tentative_value.release(); def_kind_ = static_cast(FlagDefaultKind::kDynamicValue); } if (!modified_) { // Need to set both default value *and* current, in this case - flags_internal::Copy(op_, default_src_.dynamic_value, value_.dynamic); - StoreAtomic(); - InvokeCallback(); + StoreValue(default_value_.dynamic_value); + modified_ = false; } break; } diff --git a/absl/flags/internal/flag.h b/absl/flags/internal/flag.h index ec67348..c1beda8 100644 --- a/absl/flags/internal/flag.h +++ b/absl/flags/internal/flag.h @@ -363,7 +363,7 @@ struct DynValueDeleter { if (op != nullptr) Delete(op, ptr); } - const FlagOpFn op; + FlagOpFn op; }; class FlagImpl { @@ -380,7 +380,7 @@ class FlagImpl { on_command_line_(false), counter_(0), callback_(nullptr), - default_src_(default_value_gen), + default_value_(default_value_gen), data_guard_{} {} // Constant access methods @@ -392,10 +392,6 @@ class FlagImpl { std::string DefaultValue() const ABSL_LOCKS_EXCLUDED(*DataGuard()); std::string CurrentValue() const ABSL_LOCKS_EXCLUDED(*DataGuard()); void Read(void* dst) const ABSL_LOCKS_EXCLUDED(*DataGuard()); - // Attempts to parse supplied `value` std::string. If parsing is successful, then - // it replaces `dst` with the new value. - bool TryParse(void** dst, absl::string_view value, std::string* err) const - ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard()); template ::value, int>::type = 0> @@ -466,8 +462,15 @@ class FlagImpl { // Returns heap allocated value of type T initialized with default value. std::unique_ptr MakeInitValue() const ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard()); - // Lazy initialization of the Flag's data. + // Flag initialization called via absl::call_once. void Init(); + // Attempts to parse supplied `value` std::string. If parsing is successful, + // returns new value. Otherwsie returns nullptr. + std::unique_ptr TryParse(absl::string_view value, + std::string* err) const + ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard()); + // Stores the flag value based on the pointer to the source. + void StoreValue(const void* src) ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard()); FlagHelpKind HelpSourceKind() const { return static_cast(help_source_kind_); @@ -511,7 +514,7 @@ class FlagImpl { // Mutable flag's state (guarded by `data_guard_`). - // If def_kind_ == kDynamicValue, default_src_ holds a dynamically allocated + // If def_kind_ == kDynamicValue, default_value_ holds a dynamically allocated // value. uint8_t def_kind_ : 1 ABSL_GUARDED_BY(*DataGuard()); // Has this flag's value been modified? @@ -527,7 +530,7 @@ class FlagImpl { // value specified in ABSL_FLAG or pointer to the dynamically set default // value via SetCommandLineOptionWithMode. def_kind_ is used to distinguish // these two cases. - FlagDefaultSrc default_src_ ABSL_GUARDED_BY(*DataGuard()); + FlagDefaultSrc default_value_ ABSL_GUARDED_BY(*DataGuard()); // Current Flag Value FlagValue value_; @@ -542,8 +545,8 @@ class FlagImpl { }; /////////////////////////////////////////////////////////////////////////////// -// The "unspecified" implementation of Flag object parameterized by the -// flag's value type. +// The Flag object parameterized by the flag's value type. This class implements +// flag reflection handle interface. template class Flag final : public flags_internal::CommandLineFlag { diff --git a/absl/flags/internal/type_erased_test.cc b/absl/flags/internal/type_erased_test.cc index 033e00e..4ce5981 100644 --- a/absl/flags/internal/type_erased_test.cc +++ b/absl/flags/internal/type_erased_test.cc @@ -119,6 +119,13 @@ TEST_F(TypeErasedTest, TestSetCommandLineOptionWithMode_SET_FLAGS_DEFAULT) { EXPECT_TRUE(flags::SetCommandLineOptionWithMode("int_flag", "101", flags::SET_FLAGS_DEFAULT)); + // Set it again to ensure that resetting logic is covered. + EXPECT_TRUE(flags::SetCommandLineOptionWithMode("int_flag", "102", + flags::SET_FLAGS_DEFAULT)); + + EXPECT_TRUE(flags::SetCommandLineOptionWithMode("int_flag", "103", + flags::SET_FLAGS_DEFAULT)); + EXPECT_TRUE(flags::SetCommandLineOptionWithMode("string_flag", "asdfgh", flags::SET_FLAGS_DEFAULT)); EXPECT_EQ(absl::GetFlag(FLAGS_string_flag), "asdfgh"); diff --git a/absl/functional/bind_front.h b/absl/functional/bind_front.h index 8448d7b..5b47970 100644 --- a/absl/functional/bind_front.h +++ b/absl/functional/bind_front.h @@ -38,9 +38,7 @@ ABSL_NAMESPACE_BEGIN // bind_front() // -// Binds the first N arguments of an invocable object and stores them by value, -// except types of `std::reference_wrapper` which are 'unwound' and stored by -// reference. +// Binds the first N arguments of an invocable object and stores them by value. // // Like `std::bind()`, `absl::bind_front()` is implicitly convertible to // `std::function`. In particular, it may be used as a simpler replacement for @@ -140,7 +138,9 @@ ABSL_NAMESPACE_BEGIN // // Example: Storing bound arguments by reference. // -// void Print(const string& a, const string& b) { LOG(INFO) << a << b; } +// void Print(const std::string& a, const std::string& b) { +// std::cerr << a << b; +// } // // std::string hi = "Hello, "; // std::vector names = {"Chuk", "Gek"}; @@ -152,6 +152,24 @@ ABSL_NAMESPACE_BEGIN // // dangling references. // foo->DoInFuture(absl::bind_front(Print, std::ref(hi), "Guest")); // BAD! // auto f = absl::bind_front(Print, std::ref(hi), "Guest"); // BAD! +// +// Example: Storing reference-like types. +// +// void Print(absl::string_view a, const std::string& b) { +// std::cerr << a << b; +// } +// +// std::string hi = "Hello, "; +// // Copies "hi". +// absl::bind_front(Print, hi)("Chuk"); +// +// // Compile error: std::reference_wrapper is not implicitly +// // convertible to string_view. +// // absl::bind_front(Print, std::cref(hi))("Chuk"); +// +// // Doesn't copy "hi". +// absl::bind_front(Print, absl::string_view(hi))("Chuk"); +// template constexpr functional_internal::bind_front_t bind_front( F&& func, BoundArgs&&... args) { diff --git a/absl/memory/memory_exception_safety_test.cc b/absl/memory/memory_exception_safety_test.cc index c0910dc..1df7261 100644 --- a/absl/memory/memory_exception_safety_test.cc +++ b/absl/memory/memory_exception_safety_test.cc @@ -27,9 +27,6 @@ namespace { constexpr int kLength = 50; using Thrower = testing::ThrowingValue; -using ThrowerStorage = - absl::aligned_storage_t; -using ThrowerList = std::array; TEST(MakeUnique, CheckForLeaks) { constexpr int kValue = 321; diff --git a/absl/strings/cord.cc b/absl/strings/cord.cc index cc0cc9d..d9503ae 100644 --- a/absl/strings/cord.cc +++ b/absl/strings/cord.cc @@ -63,16 +63,16 @@ namespace { // Type used with std::allocator for allocating and deallocating // `CordRepExternal`. std::allocator is used because it opaquely handles the // different new / delete overloads available on a given platform. -using ExternalAllocType = - absl::aligned_storage_t; +struct alignas(absl::cord_internal::ExternalRepAlignment()) ExternalAllocType { + unsigned char value[absl::cord_internal::ExternalRepAlignment()]; +}; // Returns the number of objects to pass in to std::allocator // allocate() and deallocate() to create enough room for `CordRepExternal` with // `releaser_size` bytes on the end. constexpr size_t GetExternalAllocNumObjects(size_t releaser_size) { // Be sure to round up since `releaser_size` could be smaller than - // sizeof(ExternalAllocType)`. + // `sizeof(ExternalAllocType)`. return (sizeof(CordRepExternal) + releaser_size + sizeof(ExternalAllocType) - 1) / sizeof(ExternalAllocType); diff --git a/absl/synchronization/internal/mutex_nonprod.inc b/absl/synchronization/internal/mutex_nonprod.inc index 85dae0f..a1502e7 100644 --- a/absl/synchronization/internal/mutex_nonprod.inc +++ b/absl/synchronization/internal/mutex_nonprod.inc @@ -252,8 +252,8 @@ class SynchronizationStorage { absl::once_flag once_; - // An aligned space for T. - typename std::aligned_storage::type space_; + // An aligned space for the T. + alignas(T) unsigned char space_[sizeof(T)]; }; } // namespace synchronization_internal diff --git a/absl/synchronization/internal/waiter.cc b/absl/synchronization/internal/waiter.cc index ddd6081..2949f5a 100644 --- a/absl/synchronization/internal/waiter.cc +++ b/absl/synchronization/internal/waiter.cc @@ -368,31 +368,29 @@ class Waiter::WinHelper { return reinterpret_cast(&w->cv_storage_); } - static_assert(sizeof(SRWLOCK) == sizeof(Waiter::SRWLockStorage), - "SRWLockStorage does not have the same size as SRWLOCK"); + static_assert(sizeof(SRWLOCK) == sizeof(void *), + "`mu_storage_` does not have the same size as SRWLOCK"); + static_assert(alignof(SRWLOCK) == alignof(void *), + "`mu_storage_` does not have the same alignment as SRWLOCK"); + + static_assert(sizeof(CONDITION_VARIABLE) == sizeof(void *), + "`ABSL_CONDITION_VARIABLE_STORAGE` does not have the same size " + "as `CONDITION_VARIABLE`"); static_assert( - alignof(SRWLOCK) == alignof(Waiter::SRWLockStorage), - "SRWLockStorage does not have the same alignment as SRWLOCK"); - - static_assert(sizeof(CONDITION_VARIABLE) == - sizeof(Waiter::ConditionVariableStorage), - "ABSL_CONDITION_VARIABLE_STORAGE does not have the same size " - "as CONDITION_VARIABLE"); - static_assert(alignof(CONDITION_VARIABLE) == - alignof(Waiter::ConditionVariableStorage), - "ConditionVariableStorage does not have the same " - "alignment as CONDITION_VARIABLE"); + alignof(CONDITION_VARIABLE) == alignof(void *), + "`cv_storage_` does not have the same alignment as `CONDITION_VARIABLE`"); // The SRWLOCK and CONDITION_VARIABLE types must be trivially constructible // and destructible because we never call their constructors or destructors. static_assert(std::is_trivially_constructible::value, - "The SRWLOCK type must be trivially constructible"); - static_assert(std::is_trivially_constructible::value, - "The CONDITION_VARIABLE type must be trivially constructible"); + "The `SRWLOCK` type must be trivially constructible"); + static_assert( + std::is_trivially_constructible::value, + "The `CONDITION_VARIABLE` type must be trivially constructible"); static_assert(std::is_trivially_destructible::value, - "The SRWLOCK type must be trivially destructible"); + "The `SRWLOCK` type must be trivially destructible"); static_assert(std::is_trivially_destructible::value, - "The CONDITION_VARIABLE type must be trivially destructible"); + "The `CONDITION_VARIABLE` type must be trivially destructible"); }; class LockHolder { diff --git a/absl/synchronization/internal/waiter.h b/absl/synchronization/internal/waiter.h index 5af5c28..a6e6d4c 100644 --- a/absl/synchronization/internal/waiter.h +++ b/absl/synchronization/internal/waiter.h @@ -133,13 +133,6 @@ class Waiter { std::atomic wakeups_; #elif ABSL_WAITER_MODE == ABSL_WAITER_MODE_WIN32 - // We can't include Windows.h in our headers, so we use aligned storage - // buffers to define the storage of SRWLOCK and CONDITION_VARIABLE. - using SRWLockStorage = - typename std::aligned_storage::type; - using ConditionVariableStorage = - typename std::aligned_storage::type; - // WinHelper - Used to define utilities for accessing the lock and // condition variable storage once the types are complete. class WinHelper; @@ -147,8 +140,10 @@ class Waiter { // REQUIRES: WinHelper::GetLock(this) must be held. void InternalCondVarPoke(); - SRWLockStorage mu_storage_; - ConditionVariableStorage cv_storage_; + // We can't include Windows.h in our headers, so we use aligned charachter + // buffers to define the storage of SRWLOCK and CONDITION_VARIABLE. + alignas(void*) unsigned char mu_storage_[sizeof(void*)]; + alignas(void*) unsigned char cv_storage_[sizeof(void*)]; int waiter_count_; int wakeup_count_; -- cgit v1.2.3