diff --git a/src/google/protobuf/compiler/cpp/field_generators/map_field.cc b/src/google/protobuf/compiler/cpp/field_generators/map_field.cc index dde6cff070..97bdb68b0c 100644 --- a/src/google/protobuf/compiler/cpp/field_generators/map_field.cc +++ b/src/google/protobuf/compiler/cpp/field_generators/map_field.cc @@ -104,8 +104,6 @@ class MapFieldGenerator : public FieldGeneratorBase { void GenerateCopyAggregateInitializer(io::Printer* printer) const override; void GenerateAggregateInitializer(io::Printer* printer) const override; void GenerateDestructorCode(io::Printer* printer) const override; - void GenerateArenaDestructorCode(io::Printer* printer) const override; - ArenaDtorNeeds NeedsArenaDestructor() const override; private: bool has_required_fields_; @@ -304,11 +302,7 @@ void MapFieldGenerator::GenerateIsInitialized(io::Printer* printer) const { void MapFieldGenerator::GenerateConstexprAggregateInitializer( io::Printer* printer) const { Formatter format(printer, variables_); - if (HasDescriptorMethods(descriptor_->file(), options_)) { - format("/*decltype($field$)*/{::_pbi::ConstantInitialized()}"); - } else { - format("/*decltype($field$)*/{}"); - } + format("/*decltype($field$)*/{}"); } void MapFieldGenerator::GenerateCopyAggregateInitializer( @@ -341,22 +335,6 @@ void MapFieldGenerator::GenerateDestructorCode(io::Printer* printer) const { format("$field$.~MapField$lite$();\n"); } -void MapFieldGenerator::GenerateArenaDestructorCode( - io::Printer* printer) const { - if (NeedsArenaDestructor() == ArenaDtorNeeds::kNone) { - return; - } - - Formatter format(printer, variables_); - // _this is the object being destructed (we are inside a static method here). - format("_this->$field$.ArenaDestruct();\n"); -} - -ArenaDtorNeeds MapFieldGenerator::NeedsArenaDestructor() const { - return HasDescriptorMethods(descriptor_->file(), options_) - ? ArenaDtorNeeds::kRequired - : ArenaDtorNeeds::kNone; -} } // namespace std::unique_ptr MakeMapGenerator( diff --git a/src/google/protobuf/dynamic_message.cc b/src/google/protobuf/dynamic_message.cc index 680fa0b407..985ae703c3 100644 --- a/src/google/protobuf/dynamic_message.cc +++ b/src/google/protobuf/dynamic_message.cc @@ -439,11 +439,6 @@ void DynamicMessage::SharedCtor(bool lock_factory) { new (field_ptr) DynamicMapField( type_info_->factory->GetPrototype(field->message_type()), GetArenaForAllocation()); - if (GetOwningArena() != nullptr) { - // Needs to destroy the mutex member. - static_cast(field_ptr)->OwnMutexDestructor( - *GetOwningArena()); - } } else { new (field_ptr) DynamicMapField( type_info_->factory->GetPrototype(field->message_type())); @@ -454,11 +449,6 @@ void DynamicMessage::SharedCtor(bool lock_factory) { DynamicMapField(type_info_->factory->GetPrototypeNoLock( field->message_type()), GetArenaForAllocation()); - if (GetOwningArena() != nullptr) { - // Needs to destroy the mutex member. - static_cast(field_ptr)->OwnMutexDestructor( - *GetOwningArena()); - } } else { new (field_ptr) DynamicMapField(type_info_->factory->GetPrototypeNoLock( diff --git a/src/google/protobuf/map_field.cc b/src/google/protobuf/map_field.cc index 59c17624cb..63fab8b3bb 100644 --- a/src/google/protobuf/map_field.cc +++ b/src/google/protobuf/map_field.cc @@ -30,6 +30,7 @@ #include "google/protobuf/map_field.h" +#include #include #include "google/protobuf/port.h" @@ -43,82 +44,90 @@ namespace protobuf { namespace internal { using ::google::protobuf::internal::DownCast; +MapFieldBase::~MapFieldBase() { + ABSL_DCHECK_EQ(arena(), nullptr); + delete maybe_payload(); +} + const RepeatedPtrFieldBase& MapFieldBase::GetRepeatedField() const { ConstAccess(); SyncRepeatedFieldWithMap(); - return *reinterpret_cast(repeated_field_); + return reinterpret_cast(payload().repeated_field); } RepeatedPtrFieldBase* MapFieldBase::MutableRepeatedField() { MutableAccess(); SyncRepeatedFieldWithMap(); SetRepeatedDirty(); - return reinterpret_cast(repeated_field_); + return reinterpret_cast(&payload().repeated_field); } -void MapFieldBase::SwapState(MapFieldBase* other) { - // a relaxed swap of the atomic - auto other_state = other->state_.load(std::memory_order_relaxed); - auto this_state = state_.load(std::memory_order_relaxed); - other->state_.store(this_state, std::memory_order_relaxed); - state_.store(other_state, std::memory_order_relaxed); +template +static void SwapRelaxed(std::atomic& a, std::atomic& b) { + auto value_b = b.load(std::memory_order_relaxed); + auto value_a = a.load(std::memory_order_relaxed); + b.store(value_a, std::memory_order_relaxed); + a.store(value_b, std::memory_order_relaxed); } -void SwapRepeatedPtrToNull(RepeatedPtrField** from, - RepeatedPtrField** to, Arena* from_arena, - Arena* to_arena) { - ABSL_DCHECK(*from != nullptr); - ABSL_DCHECK(*to == nullptr); - *to = Arena::CreateMessage >(to_arena); - **to = std::move(**from); - if (from_arena == nullptr) { - delete *from; +MapFieldBase::ReflectionPayload& MapFieldBase::PayloadSlow() const { + auto p = payload_.load(std::memory_order_acquire); + if (!IsPayload(p)) { + auto* arena = ToArena(p); + auto* payload = Arena::Create(arena, arena); + auto new_p = ToTaggedPtr(payload); + if (payload_.compare_exchange_strong(p, new_p, std::memory_order_acq_rel)) { + // We were able to store it. + p = new_p; + } else { + // Someone beat us to it. Throw away the one we made. `p` already contains + // the one we want. + if (arena == nullptr) delete payload; + } } - *from = nullptr; + return *ToPayload(p); } void MapFieldBase::Swap(MapFieldBase* other) { - if (arena_ == other->arena_) { + if (arena() == other->arena()) { InternalSwap(other); return; } - if (repeated_field_ != nullptr || other->repeated_field_ != nullptr) { - if (repeated_field_ == nullptr) { - SwapRepeatedPtrToNull(&other->repeated_field_, &repeated_field_, - other->arena_, arena_); - } else if (other->repeated_field_ == nullptr) { - SwapRepeatedPtrToNull(&repeated_field_, &other->repeated_field_, arena_, - other->arena_); - } else { - repeated_field_->Swap(other->repeated_field_); - } - } - SwapState(other); + auto* p1 = maybe_payload(); + auto* p2 = other->maybe_payload(); + if (p1 == nullptr && p2 == nullptr) return; + + if (p1 == nullptr) p1 = &payload(); + if (p2 == nullptr) p2 = &other->payload(); + p1->repeated_field.Swap(&p2->repeated_field); + SwapRelaxed(p1->state, p2->state); } void MapFieldBase::UnsafeShallowSwap(MapFieldBase* other) { - ABSL_DCHECK_EQ(arena_, other->arena_); + ABSL_DCHECK_EQ(arena(), other->arena()); InternalSwap(other); } void MapFieldBase::InternalSwap(MapFieldBase* other) { - std::swap(arena_, other->arena_); - std::swap(repeated_field_, other->repeated_field_); - SwapState(other); + SwapRelaxed(payload_, other->payload_); } size_t MapFieldBase::SpaceUsedExcludingSelfLong() const { ConstAccess(); - mutex_.Lock(); - size_t size = SpaceUsedExcludingSelfNoLock(); - mutex_.Unlock(); - ConstAccess(); + size_t size = 0; + if (auto* p = maybe_payload()) { + { + absl::MutexLock lock(&p->mutex); + size = SpaceUsedExcludingSelfNoLock(); + } + ConstAccess(); + } return size; } size_t MapFieldBase::SpaceUsedExcludingSelfNoLock() const { - if (repeated_field_ != nullptr) { - return repeated_field_->SpaceUsedExcludingSelfLong(); + if (auto* p = maybe_payload()) { + return p->repeated_field.SpaceUsedExcludingSelfLong(); } else { return 0; } @@ -128,67 +137,42 @@ bool MapFieldBase::IsMapValid() const { ConstAccess(); // "Acquire" insures the operation after SyncRepeatedFieldWithMap won't get // executed before state_ is checked. - int state = state_.load(std::memory_order_acquire); - return state != STATE_MODIFIED_REPEATED; + return state() != STATE_MODIFIED_REPEATED; } bool MapFieldBase::IsRepeatedFieldValid() const { ConstAccess(); - int state = state_.load(std::memory_order_acquire); - return state != STATE_MODIFIED_MAP; + return state() != STATE_MODIFIED_MAP; } void MapFieldBase::SetMapDirty() { MutableAccess(); // These are called by (non-const) mutator functions. So by our API it's the // callers responsibility to have these calls properly ordered. - state_.store(STATE_MODIFIED_MAP, std::memory_order_relaxed); + payload().state.store(STATE_MODIFIED_MAP, std::memory_order_relaxed); } void MapFieldBase::SetRepeatedDirty() { MutableAccess(); // These are called by (non-const) mutator functions. So by our API it's the // callers responsibility to have these calls properly ordered. - state_.store(STATE_MODIFIED_REPEATED, std::memory_order_relaxed); + payload().state.store(STATE_MODIFIED_REPEATED, std::memory_order_relaxed); } void MapFieldBase::SyncRepeatedFieldWithMap() const { ConstAccess(); - // acquire here matches with release below to ensure that we can only see a - // value of CLEAN after all previous changes have been synced. - switch (state_.load(std::memory_order_acquire)) { - case STATE_MODIFIED_MAP: - mutex_.Lock(); + if (state() == STATE_MODIFIED_MAP) { + auto& p = payload(); + { + absl::MutexLock lock(&p.mutex); // Double check state, because another thread may have seen the same // state and done the synchronization before the current thread. - if (state_.load(std::memory_order_relaxed) == STATE_MODIFIED_MAP) { + if (p.state.load(std::memory_order_relaxed) == STATE_MODIFIED_MAP) { SyncRepeatedFieldWithMapNoLock(); - state_.store(CLEAN, std::memory_order_release); - } - mutex_.Unlock(); - ConstAccess(); - break; - case CLEAN: - mutex_.Lock(); - // Double check state - if (state_.load(std::memory_order_relaxed) == CLEAN) { - if (repeated_field_ == nullptr) { - repeated_field_ = - Arena::CreateMessage >(arena_); - } - state_.store(CLEAN, std::memory_order_release); + p.state.store(CLEAN, std::memory_order_release); } - mutex_.Unlock(); - ConstAccess(); - break; - default: - break; - } -} - -void MapFieldBase::SyncRepeatedFieldWithMapNoLock() const { - if (repeated_field_ == nullptr) { - repeated_field_ = Arena::CreateMessage >(arena_); + } + ConstAccess(); } } @@ -196,15 +180,17 @@ void MapFieldBase::SyncMapWithRepeatedField() const { ConstAccess(); // acquire here matches with release below to ensure that we can only see a // value of CLEAN after all previous changes have been synced. - if (state_.load(std::memory_order_acquire) == STATE_MODIFIED_REPEATED) { - mutex_.Lock(); - // Double check state, because another thread may have seen the same state - // and done the synchronization before the current thread. - if (state_.load(std::memory_order_relaxed) == STATE_MODIFIED_REPEATED) { - SyncMapWithRepeatedFieldNoLock(); - state_.store(CLEAN, std::memory_order_release); + if (state() == STATE_MODIFIED_REPEATED) { + auto& p = payload(); + { + absl::MutexLock lock(&p.mutex); + // Double check state, because another thread may have seen the same state + // and done the synchronization before the current thread. + if (p.state.load(std::memory_order_relaxed) == STATE_MODIFIED_REPEATED) { + SyncMapWithRepeatedFieldNoLock(); + p.state.store(CLEAN, std::memory_order_release); + } } - mutex_.Unlock(); ConstAccess(); } } @@ -219,7 +205,7 @@ DynamicMapField::DynamicMapField(const Message* default_entry, Arena* arena) default_entry_(default_entry) {} DynamicMapField::~DynamicMapField() { - ABSL_DCHECK_EQ(arena_, nullptr); + ABSL_DCHECK_EQ(arena(), nullptr); // DynamicMapField owns map values. Need to delete them before clearing the // map. for (auto& kv : map_) { @@ -232,7 +218,7 @@ int DynamicMapField::size() const { return GetMap().size(); } void DynamicMapField::Clear() { Map* map = &const_cast(this)->map_; - if (MapFieldBase::arena_ == nullptr) { + if (arena() == nullptr) { for (Map::iterator iter = map->begin(); iter != map->end(); ++iter) { iter->second.DeleteData(); @@ -241,8 +227,8 @@ void DynamicMapField::Clear() { map->clear(); - if (MapFieldBase::repeated_field_ != nullptr) { - MapFieldBase::repeated_field_->Clear(); + if (auto* p = maybe_payload()) { + p->repeated_field.Clear(); } // Data in map and repeated field are both empty, but we can't set status // CLEAN which will invalidate previous reference to map. @@ -261,11 +247,11 @@ void DynamicMapField::AllocateMapValue(MapValueRef* map_val) { // Allocate memory for the MapValueRef, and initialize to // default value. switch (val_des->cpp_type()) { -#define HANDLE_TYPE(CPPTYPE, TYPE) \ - case FieldDescriptor::CPPTYPE_##CPPTYPE: { \ - TYPE* value = Arena::Create(MapFieldBase::arena_); \ - map_val->SetValue(value); \ - break; \ +#define HANDLE_TYPE(CPPTYPE, TYPE) \ + case FieldDescriptor::CPPTYPE_##CPPTYPE: { \ + auto* value = Arena::Create(arena()); \ + map_val->SetValue(value); \ + break; \ } HANDLE_TYPE(INT32, int32_t); HANDLE_TYPE(INT64, int64_t); @@ -280,7 +266,7 @@ void DynamicMapField::AllocateMapValue(MapValueRef* map_val) { case FieldDescriptor::CPPTYPE_MESSAGE: { const Message& message = default_entry_->GetReflection()->GetMessage(*default_entry_, val_des); - Message* value = message.New(MapFieldBase::arena_); + Message* value = message.New(arena()); map_val->SetValue(value); break; } @@ -326,7 +312,7 @@ bool DynamicMapField::DeleteMapValue(const MapKey& map_key) { } // Set map dirty only if the delete is successful. MapFieldBase::SetMapDirty(); - if (MapFieldBase::arena_ == nullptr) { + if (arena() == nullptr) { iter->second.DeleteData(); } map_.erase(iter); @@ -420,31 +406,22 @@ void DynamicMapField::MergeFrom(const MapFieldBase& other) { } void DynamicMapField::Swap(MapFieldBase* other) { - DynamicMapField* other_field = DownCast(other); - std::swap(this->MapFieldBase::repeated_field_, other_field->repeated_field_); - map_.swap(other_field->map_); - // a relaxed swap of the atomic - auto other_state = other_field->state_.load(std::memory_order_relaxed); - auto this_state = this->MapFieldBase::state_.load(std::memory_order_relaxed); - other_field->state_.store(this_state, std::memory_order_relaxed); - this->MapFieldBase::state_.store(other_state, std::memory_order_relaxed); + MapFieldBase::Swap(other); + map_.swap(DownCast(other)->map_); } void DynamicMapField::SyncRepeatedFieldWithMapNoLock() const { const Reflection* reflection = default_entry_->GetReflection(); const FieldDescriptor* key_des = default_entry_->GetDescriptor()->map_key(); const FieldDescriptor* val_des = default_entry_->GetDescriptor()->map_value(); - if (MapFieldBase::repeated_field_ == nullptr) { - MapFieldBase::repeated_field_ = - Arena::CreateMessage >(MapFieldBase::arena_); - } - MapFieldBase::repeated_field_->Clear(); + auto& rep = payload().repeated_field; + rep.Clear(); for (Map::const_iterator it = map_.begin(); it != map_.end(); ++it) { - Message* new_entry = default_entry_->New(MapFieldBase::arena_); - MapFieldBase::repeated_field_->AddAllocated(new_entry); + Message* new_entry = default_entry_->New(arena()); + rep.AddAllocated(new_entry); const MapKey& map_key = it->first; switch (key_des->cpp_type()) { case FieldDescriptor::CPPTYPE_STRING: @@ -515,38 +492,38 @@ void DynamicMapField::SyncMapWithRepeatedFieldNoLock() const { const Reflection* reflection = default_entry_->GetReflection(); const FieldDescriptor* key_des = default_entry_->GetDescriptor()->map_key(); const FieldDescriptor* val_des = default_entry_->GetDescriptor()->map_value(); + Arena* arena = this->arena(); // DynamicMapField owns map values. Need to delete them before clearing // the map. - if (MapFieldBase::arena_ == nullptr) { + if (arena == nullptr) { for (Map::iterator iter = map->begin(); iter != map->end(); ++iter) { iter->second.DeleteData(); } } map->clear(); - for (RepeatedPtrField::iterator it = - MapFieldBase::repeated_field_->begin(); - it != MapFieldBase::repeated_field_->end(); ++it) { + auto& rep = payload().repeated_field; + for (const Message& elem : rep) { // MapKey type will be set later. MapKey map_key; switch (key_des->cpp_type()) { case FieldDescriptor::CPPTYPE_STRING: - map_key.SetStringValue(reflection->GetString(*it, key_des)); + map_key.SetStringValue(reflection->GetString(elem, key_des)); break; case FieldDescriptor::CPPTYPE_INT64: - map_key.SetInt64Value(reflection->GetInt64(*it, key_des)); + map_key.SetInt64Value(reflection->GetInt64(elem, key_des)); break; case FieldDescriptor::CPPTYPE_INT32: - map_key.SetInt32Value(reflection->GetInt32(*it, key_des)); + map_key.SetInt32Value(reflection->GetInt32(elem, key_des)); break; case FieldDescriptor::CPPTYPE_UINT64: - map_key.SetUInt64Value(reflection->GetUInt64(*it, key_des)); + map_key.SetUInt64Value(reflection->GetUInt64(elem, key_des)); break; case FieldDescriptor::CPPTYPE_UINT32: - map_key.SetUInt32Value(reflection->GetUInt32(*it, key_des)); + map_key.SetUInt32Value(reflection->GetUInt32(elem, key_des)); break; case FieldDescriptor::CPPTYPE_BOOL: - map_key.SetBoolValue(reflection->GetBool(*it, key_des)); + map_key.SetBoolValue(reflection->GetBool(elem, key_des)); break; case FieldDescriptor::CPPTYPE_DOUBLE: case FieldDescriptor::CPPTYPE_FLOAT: @@ -556,7 +533,7 @@ void DynamicMapField::SyncMapWithRepeatedFieldNoLock() const { break; } - if (MapFieldBase::arena_ == nullptr) { + if (arena == nullptr) { // Remove existing map value with same key. Map::iterator iter = map->find(map_key); if (iter != map->end()) { @@ -567,12 +544,12 @@ void DynamicMapField::SyncMapWithRepeatedFieldNoLock() const { MapValueRef& map_val = (*map)[map_key]; map_val.SetType(val_des->cpp_type()); switch (val_des->cpp_type()) { -#define HANDLE_TYPE(CPPTYPE, TYPE, METHOD) \ - case FieldDescriptor::CPPTYPE_##CPPTYPE: { \ - TYPE* value = Arena::Create(MapFieldBase::arena_); \ - *value = reflection->Get##METHOD(*it, val_des); \ - map_val.SetValue(value); \ - break; \ +#define HANDLE_TYPE(CPPTYPE, TYPE, METHOD) \ + case FieldDescriptor::CPPTYPE_##CPPTYPE: { \ + auto* value = Arena::Create(arena); \ + *value = reflection->Get##METHOD(elem, val_des); \ + map_val.SetValue(value); \ + break; \ } HANDLE_TYPE(INT32, int32_t, Int32); HANDLE_TYPE(INT64, int64_t, Int64); @@ -585,8 +562,8 @@ void DynamicMapField::SyncMapWithRepeatedFieldNoLock() const { HANDLE_TYPE(ENUM, int32_t, EnumValue); #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_MESSAGE: { - const Message& message = reflection->GetMessage(*it, val_des); - Message* value = message.New(MapFieldBase::arena_); + const Message& message = reflection->GetMessage(elem, val_des); + Message* value = message.New(arena); value->CopyFrom(message); map_val.SetValue(value); break; @@ -597,8 +574,8 @@ void DynamicMapField::SyncMapWithRepeatedFieldNoLock() const { size_t DynamicMapField::SpaceUsedExcludingSelfNoLock() const { size_t size = 0; - if (MapFieldBase::repeated_field_ != nullptr) { - size += MapFieldBase::repeated_field_->SpaceUsedExcludingSelfLong(); + if (auto* p = maybe_payload()) { + size += p->repeated_field.SpaceUsedExcludingSelfLong(); } size += sizeof(map_); size_t map_size = map_.size(); diff --git a/src/google/protobuf/map_field.h b/src/google/protobuf/map_field.h index 411911448e..c48e7646d2 100644 --- a/src/google/protobuf/map_field.h +++ b/src/google/protobuf/map_field.h @@ -338,31 +338,14 @@ class MapFieldAccessor; // reflection implementation only. Users should never use this directly. class PROTOBUF_EXPORT MapFieldBase : public MapFieldBaseForParse { public: - MapFieldBase() - : arena_(nullptr), repeated_field_(nullptr), state_(STATE_MODIFIED_MAP) {} - - // This constructor is for constant initialized global instances. - // It uses a linker initialized mutex, so it is not compatible with regular - // runtime instances. - // Except in MSVC, where we can't have a constinit mutex. - // NOLINTNEXTLINE(google-explicit-constructor) - constexpr MapFieldBase(ConstantInitialized) - : arena_(nullptr), - repeated_field_(nullptr), - mutex_(absl::kConstInit), - state_(STATE_MODIFIED_MAP) {} - explicit MapFieldBase(Arena* arena) - : arena_(arena), repeated_field_(nullptr), state_(STATE_MODIFIED_MAP) {} + constexpr MapFieldBase() {} + explicit MapFieldBase(Arena* arena) : payload_{ToTaggedPtr(arena)} {} MapFieldBase(const MapFieldBase&) = delete; MapFieldBase& operator=(const MapFieldBase&) = delete; protected: - ~MapFieldBase() { // "protected" stops users from deleting a `MapFieldBase *` - ABSL_DCHECK_EQ(arena_, nullptr); - delete repeated_field_; - } - void ArenaDestruct() { mutex_.~Mutex(); } - void OwnMutexDestructor(Arena& arena) { arena.OwnDestructor(&mutex_); } + // "protected" stops users from deleting a `MapFieldBase *` + ~MapFieldBase(); public: // Returns reference to internal repeated field. Data written using @@ -412,12 +395,12 @@ class PROTOBUF_EXPORT MapFieldBase : public MapFieldBaseForParse { // Synchronizes the content in Map to RepeatedPtrField if there is any change // to Map after last synchronization. void SyncRepeatedFieldWithMap() const; - virtual void SyncRepeatedFieldWithMapNoLock() const; + virtual void SyncRepeatedFieldWithMapNoLock() const = 0; // Synchronizes the content in RepeatedPtrField to Map if there is any change // to RepeatedPtrField after last synchronization. void SyncMapWithRepeatedField() const; - virtual void SyncMapWithRepeatedFieldNoLock() const {} + virtual void SyncMapWithRepeatedFieldNoLock() const = 0; // Tells MapFieldBase that there is new change to Map. void SetMapDirty(); @@ -457,12 +440,39 @@ class PROTOBUF_EXPORT MapFieldBase : public MapFieldBaseForParse { CLEAN = 2, // data in map and repeated field are same }; - Arena* arena_; - mutable RepeatedPtrField* repeated_field_; + struct ReflectionPayload { + explicit ReflectionPayload(Arena* arena) : repeated_field(arena) {} + RepeatedPtrField repeated_field; + + absl::Mutex mutex; // The thread to synchronize map and repeated + // field needs to get lock first; + std::atomic state{STATE_MODIFIED_MAP}; + }; + + Arena* arena() const { + auto p = payload_.load(std::memory_order_acquire); + if (IsPayload(p)) return ToPayload(p)->repeated_field.GetArena(); + return ToArena(p); + } + + // Returns the reflection payload. Returns null if it does not exist yet. + ReflectionPayload* maybe_payload() const { + auto p = payload_.load(std::memory_order_acquire); + return IsPayload(p) ? ToPayload(p) : nullptr; + } + // Returns the reflection payload, and constructs one if does not exist yet. + ReflectionPayload& payload() const { + auto* p = maybe_payload(); + return p != nullptr ? *p : PayloadSlow(); + } + ReflectionPayload& PayloadSlow() const; - mutable absl::Mutex mutex_; // The thread to synchronize map and repeated - // field needs to get lock first; - mutable std::atomic state_; + State state() const { + auto* p = maybe_payload(); + return p != nullptr ? p->state.load(std::memory_order_acquire) + // The default + : STATE_MODIFIED_MAP; + } private: friend class ContendedMapCleanTest; @@ -492,8 +502,32 @@ class PROTOBUF_EXPORT MapFieldBase : public MapFieldBaseForParse { // It implements the ++ operator of MapIterator. virtual void IncreaseIterator(MapIterator* map_iter) const = 0; - // Swaps state_ with another MapFieldBase - void SwapState(MapFieldBase* other); + enum class TaggedPtr : uintptr_t {}; + static constexpr uintptr_t kHasPayloadBit = 1; + + static ReflectionPayload* ToPayload(TaggedPtr p) { + ABSL_DCHECK(IsPayload(p)); + auto* res = reinterpret_cast(static_cast(p) - + kHasPayloadBit); + PROTOBUF_ASSUME(res != nullptr); + return res; + } + static Arena* ToArena(TaggedPtr p) { + ABSL_DCHECK(!IsPayload(p)); + return reinterpret_cast(p); + } + static TaggedPtr ToTaggedPtr(ReflectionPayload* p) { + return static_cast(reinterpret_cast(p) + + kHasPayloadBit); + } + static TaggedPtr ToTaggedPtr(Arena* p) { + return static_cast(reinterpret_cast(p)); + } + static bool IsPayload(TaggedPtr p) { + return static_cast(p) & kHasPayloadBit; + } + + mutable std::atomic payload_{}; }; // This class provides common Map Reflection implementations for generated @@ -501,28 +535,21 @@ class PROTOBUF_EXPORT MapFieldBase : public MapFieldBaseForParse { template class TypeDefinedMapFieldBase : public MapFieldBase { public: - TypeDefinedMapFieldBase() {} + constexpr TypeDefinedMapFieldBase() {} TypeDefinedMapFieldBase(const TypeDefinedMapFieldBase&) = delete; TypeDefinedMapFieldBase& operator=(const TypeDefinedMapFieldBase&) = delete; - // This constructor is for constant initialized global instances. - // It uses a linker initialized mutex, so it is not compatible with regular - // runtime instances. - // NOLINTNEXTLINE(google-explicit-constructor) - constexpr TypeDefinedMapFieldBase(ConstantInitialized tag) - : MapFieldBase(tag) {} explicit TypeDefinedMapFieldBase(Arena* arena) : MapFieldBase(arena) {} TypeDefinedMapFieldBase(ArenaInitialized, Arena* arena) : TypeDefinedMapFieldBase(arena) {} protected: ~TypeDefinedMapFieldBase() {} - using MapFieldBase::ArenaDestruct; public: - void MapBegin(MapIterator* map_iter) const override; - void MapEnd(MapIterator* map_iter) const override; - bool EqualIterator(const MapIterator& a, const MapIterator& b) const override; + void MapBegin(MapIterator* map_iter) const final; + void MapEnd(MapIterator* map_iter) const final; + bool EqualIterator(const MapIterator& a, const MapIterator& b) const final; virtual const Map& GetMap() const = 0; // This overrides the base's method to specialize the signature via @@ -534,11 +561,11 @@ class TypeDefinedMapFieldBase : public MapFieldBase { const MapIterator* map_iter) const; private: - void InitializeIterator(MapIterator* map_iter) const override; - void DeleteIterator(MapIterator* map_iter) const override; + void InitializeIterator(MapIterator* map_iter) const final; + void DeleteIterator(MapIterator* map_iter) const final; void CopyIterator(MapIterator* this_iteratorm, - const MapIterator& that_iterator) const override; - void IncreaseIterator(MapIterator* map_iter) const override; + const MapIterator& that_iterator) const final; + void IncreaseIterator(MapIterator* map_iter) const final; virtual void SetMapIteratorValue(MapIterator* map_iter) const = 0; }; @@ -549,7 +576,7 @@ class TypeDefinedMapFieldBase : public MapFieldBase { template -class MapField : public TypeDefinedMapFieldBase { +class MapField final : public TypeDefinedMapFieldBase { // Provide utilities to parse/serialize key/value. Provide utilities to // manipulate internal stored type. typedef MapTypeHandler KeyTypeHandler; @@ -574,47 +601,40 @@ class MapField : public TypeDefinedMapFieldBase { static constexpr WireFormatLite::FieldType kKeyFieldType = kKeyFieldType_; static constexpr WireFormatLite::FieldType kValueFieldType = kValueFieldType_; - MapField() : impl_() {} + constexpr MapField() : impl_() {} MapField(const MapField&) = delete; MapField& operator=(const MapField&) = delete; - virtual ~MapField() = default; - void ArenaDestruct() { TypeDefinedMapFieldBase::ArenaDestruct(); } - - // This constructor is for constant initialized global instances. - // It uses a linker initialized mutex, so it is not compatible with regular - // runtime instances. - // NOLINTNEXTLINE(google-explicit-constructor) - constexpr MapField(ConstantInitialized tag) - : TypeDefinedMapFieldBase(tag), impl_() {} + ~MapField() = default; + explicit MapField(Arena* arena) : TypeDefinedMapFieldBase(arena), impl_(arena) {} MapField(ArenaInitialized, Arena* arena) : MapField(arena) {} + // TODO(sbenza): Move these up to TypeDefinedMapFieldBase where possible. // Implement MapFieldBase - bool ContainsMapKey(const MapKey& map_key) const override; - bool InsertOrLookupMapValue(const MapKey& map_key, MapValueRef* val) override; - bool LookupMapValue(const MapKey& map_key, - MapValueConstRef* val) const override; + bool ContainsMapKey(const MapKey& map_key) const final; + bool InsertOrLookupMapValue(const MapKey& map_key, MapValueRef* val) final; + bool LookupMapValue(const MapKey& map_key, MapValueConstRef* val) const final; bool LookupMapValue(const MapKey&, MapValueRef*) const = delete; - bool DeleteMapValue(const MapKey& map_key) override; + bool DeleteMapValue(const MapKey& map_key) final; - const Map& GetMap() const override { + const Map& GetMap() const final { MapFieldBase::SyncMapWithRepeatedField(); return impl_.GetMap(); } - Map* MutableMap() override { + Map* MutableMap() final { MapFieldBase::SyncMapWithRepeatedField(); Map* result = impl_.MutableMap(); MapFieldBase::SetMapDirty(); return result; } - int size() const override; - void Clear() override; - void MergeFrom(const MapFieldBase& other) override; - void Swap(MapFieldBase* other) override; - void UnsafeShallowSwap(MapFieldBase* other) override; + int size() const final; + void Clear() final; + void MergeFrom(const MapFieldBase& other) final; + void Swap(MapFieldBase* other) final; + void UnsafeShallowSwap(MapFieldBase* other) final; void InternalSwap(MapField* other); // Used in the implementation of parsing. Caller should take the ownership iff @@ -639,25 +659,20 @@ class MapField : public TypeDefinedMapFieldBase { typedef void DestructorSkippable_; // Implements MapFieldBase - void SyncRepeatedFieldWithMapNoLock() const override; - void SyncMapWithRepeatedFieldNoLock() const override; - size_t SpaceUsedExcludingSelfNoLock() const override; + void SyncRepeatedFieldWithMapNoLock() const final; + void SyncMapWithRepeatedFieldNoLock() const final; + size_t SpaceUsedExcludingSelfNoLock() const final; - void SetMapIteratorValue(MapIterator* map_iter) const override; + void SetMapIteratorValue(MapIterator* map_iter) const final; friend class ::PROTOBUF_NAMESPACE_ID::Arena; friend class MapFieldStateTest; // For testing, it needs raw access to impl_ }; -template -bool AllAreInitialized( - const MapField& field) { - const auto& t = field.GetMap(); - for (typename Map::const_iterator it = t.begin(); it != t.end(); - ++it) { - if (!it->second.IsInitialized()) return false; +template +bool AllAreInitialized(const TypeDefinedMapFieldBase& field) { + for (const auto& p : field.GetMap()) { + if (!p.second.IsInitialized()) return false; } return true; } @@ -670,7 +685,7 @@ struct MapEntryToMapField< typedef MapField MapFieldType; }; -class PROTOBUF_EXPORT DynamicMapField +class PROTOBUF_EXPORT DynamicMapField final : public TypeDefinedMapFieldBase { public: explicit DynamicMapField(const Message* default_entry); @@ -680,21 +695,20 @@ class PROTOBUF_EXPORT DynamicMapField virtual ~DynamicMapField(); // Implement MapFieldBase - bool ContainsMapKey(const MapKey& map_key) const override; - bool InsertOrLookupMapValue(const MapKey& map_key, MapValueRef* val) override; - bool LookupMapValue(const MapKey& map_key, - MapValueConstRef* val) const override; + bool ContainsMapKey(const MapKey& map_key) const final; + bool InsertOrLookupMapValue(const MapKey& map_key, MapValueRef* val) final; + bool LookupMapValue(const MapKey& map_key, MapValueConstRef* val) const final; bool LookupMapValue(const MapKey&, MapValueRef*) const = delete; - bool DeleteMapValue(const MapKey& map_key) override; - void MergeFrom(const MapFieldBase& other) override; - void Swap(MapFieldBase* other) override; - void UnsafeShallowSwap(MapFieldBase* other) override { Swap(other); } + bool DeleteMapValue(const MapKey& map_key) final; + void MergeFrom(const MapFieldBase& other) final; + void Swap(MapFieldBase* other) final; + void UnsafeShallowSwap(MapFieldBase* other) final { Swap(other); } - const Map& GetMap() const override; - Map* MutableMap() override; + const Map& GetMap() const final; + Map* MutableMap() final; - int size() const override; - void Clear() override; + int size() const final; + void Clear() final; private: Map map_; @@ -703,10 +717,10 @@ class PROTOBUF_EXPORT DynamicMapField void AllocateMapValue(MapValueRef* map_val); // Implements MapFieldBase - void SyncRepeatedFieldWithMapNoLock() const override; - void SyncMapWithRepeatedFieldNoLock() const override; - size_t SpaceUsedExcludingSelfNoLock() const override; - void SetMapIteratorValue(MapIterator* map_iter) const override; + void SyncRepeatedFieldWithMapNoLock() const final; + void SyncMapWithRepeatedFieldNoLock() const final; + size_t SpaceUsedExcludingSelfNoLock() const final; + void SetMapIteratorValue(MapIterator* map_iter) const final; }; } // namespace internal diff --git a/src/google/protobuf/map_field_inl.h b/src/google/protobuf/map_field_inl.h index 677ccc9bc4..155b8e0454 100644 --- a/src/google/protobuf/map_field_inl.h +++ b/src/google/protobuf/map_field_inl.h @@ -178,11 +178,8 @@ template void MapField::Clear() { - if (this->MapFieldBase::repeated_field_ != nullptr) { - RepeatedPtrField* repeated_field = - reinterpret_cast*>( - this->MapFieldBase::repeated_field_); - repeated_field->Clear(); + if (auto* p = this->maybe_payload()) { + reinterpret_cast&>(p->repeated_field).Clear(); } impl_.MutableMap()->clear(); @@ -308,15 +305,10 @@ template void MapField::SyncRepeatedFieldWithMapNoLock() const { - if (this->MapFieldBase::repeated_field_ == nullptr) { - this->MapFieldBase::repeated_field_ = - Arena::CreateMessage >( - this->MapFieldBase::arena_); - } const Map& map = impl_.GetMap(); RepeatedPtrField* repeated_field = reinterpret_cast*>( - this->MapFieldBase::repeated_field_); + &this->payload().repeated_field); repeated_field->Clear(); @@ -325,11 +317,11 @@ void MapFieldarena(); const Message* default_entry = Derived::internal_default_instance(); for (typename Map::const_iterator it = map.begin(); it != map.end(); ++it) { - EntryType* new_entry = - DownCast(default_entry->New(this->MapFieldBase::arena_)); + EntryType* new_entry = DownCast(default_entry->New(arena)); repeated_field->AddAllocated(new_entry); (*new_entry->mutable_key()) = it->first; (*new_entry->mutable_value()) = it->second; @@ -344,8 +336,7 @@ void MapField* map = const_cast(this)->impl_.MutableMap(); RepeatedPtrField* repeated_field = reinterpret_cast*>( - this->MapFieldBase::repeated_field_); - ABSL_CHECK(this->MapFieldBase::repeated_field_ != nullptr); + &this->payload().repeated_field); map->clear(); for (typename RepeatedPtrField::iterator it = repeated_field->begin(); @@ -365,8 +356,8 @@ template ::SpaceUsedExcludingSelfNoLock() const { size_t size = 0; - if (this->MapFieldBase::repeated_field_ != nullptr) { - size += this->MapFieldBase::repeated_field_->SpaceUsedExcludingSelfLong(); + if (auto* p = this->maybe_payload()) { + size += p->repeated_field.SpaceUsedExcludingSelfLong(); } size += impl_.GetMap().SpaceUsedExcludingSelfLong(); diff --git a/src/google/protobuf/map_field_test.cc b/src/google/protobuf/map_field_test.cc index 74b7835261..ae6007ffe0 100644 --- a/src/google/protobuf/map_field_test.cc +++ b/src/google/protobuf/map_field_test.cc @@ -54,33 +54,21 @@ namespace internal { using unittest::TestAllTypes; -// ArenaHolder from map_test_util.h works fine for fields other than map -// fields. For arena-owned map fields, the ArenaDestruct() call must be made -// because the destructor will be skipped. -template -struct ArenaDestructor : ArenaHolder { - ArenaDestructor(Arena* arena) - : ArenaHolder(arena), owned_by_arena(arena != nullptr) {} - ~ArenaDestructor() { - if (owned_by_arena) ArenaHolder::get()->ArenaDestruct(); - } - - bool owned_by_arena; -}; - class MapFieldBaseStub : public MapFieldBase { public: - typedef void InternalArenaConstructable_; + using InternalArenaConstructable_ = void; typedef void DestructorSkippable_; MapFieldBaseStub() {} virtual ~MapFieldBaseStub() {} explicit MapFieldBaseStub(Arena* arena) : MapFieldBase(arena) {} void SetMapDirty() { - state_.store(STATE_MODIFIED_MAP, std::memory_order_relaxed); + payload().state.store(STATE_MODIFIED_MAP, std::memory_order_relaxed); } void SetRepeatedDirty() { - state_.store(STATE_MODIFIED_REPEATED, std::memory_order_relaxed); + payload().state.store(STATE_MODIFIED_REPEATED, std::memory_order_relaxed); } + void SyncRepeatedFieldWithMapNoLock() const override {} + void SyncMapWithRepeatedFieldNoLock() const override {} UntypedMapBase* MutableMap() override { return nullptr; } bool ContainsMapKey(const MapKey& map_key) const override { return false; } bool InsertOrLookupMapValue(const MapKey& map_key, @@ -142,7 +130,7 @@ class MapFieldBasePrimitiveTest : public testing::TestWithParam { } std::unique_ptr arena_; - ArenaDestructor map_field_; + ArenaHolder map_field_; MapFieldBase* map_field_base_; Map* map_; const Descriptor* map_descriptor_; @@ -246,7 +234,7 @@ class MapFieldStateTest map_field_base_(map_field_.get()), state_(std::get<0>(GetParam())) { // Build map field - Expect(map_field_.get(), MAP_DIRTY, 0, 0, true); + Expect(map_field_.get(), MAP_DIRTY, 0, 0); switch (state_) { case CLEAN: AddOneStillClean(map_field_.get()); @@ -267,13 +255,13 @@ class MapFieldStateTest Map* map = map_field->MutableMap(); (*map)[0] = 0; map_field_base->GetRepeatedField(); - Expect(map_field, CLEAN, 1, 1, false); + Expect(map_field, CLEAN, 1, 1); } void MakeMapDirty(MapFieldType* map_field) { Map* map = map_field->MutableMap(); (*map)[0] = 0; - Expect(map_field, MAP_DIRTY, 1, 0, true); + Expect(map_field, MAP_DIRTY, 1, 0); } void MakeRepeatedDirty(MapFieldType* map_field) { @@ -284,32 +272,28 @@ class MapFieldStateTest Map* map = map_field->impl_.MutableMap(); map->clear(); - Expect(map_field, REPEATED_DIRTY, 0, 1, false); + Expect(map_field, REPEATED_DIRTY, 0, 1); } void Expect(MapFieldType* map_field, State state, int map_size, - int repeated_size, bool is_repeated_null) { + int repeated_size) { // We use MutableMap on impl_ because we don't want to disturb the syncing Map* map = map_field->impl_.MutableMap(); - RepeatedPtrField* repeated_field = map_field->repeated_field_; switch (state) { case MAP_DIRTY: - EXPECT_FALSE(map_field->state_.load(std::memory_order_relaxed) != - MapFieldType::STATE_MODIFIED_MAP); - EXPECT_TRUE(map_field->state_.load(std::memory_order_relaxed) != + EXPECT_FALSE(map_field->state() != MapFieldType::STATE_MODIFIED_MAP); + EXPECT_TRUE(map_field->state() != MapFieldType::STATE_MODIFIED_REPEATED); break; case REPEATED_DIRTY: - EXPECT_TRUE(map_field->state_.load(std::memory_order_relaxed) != - MapFieldType::STATE_MODIFIED_MAP); - EXPECT_FALSE(map_field->state_.load(std::memory_order_relaxed) != + EXPECT_TRUE(map_field->state() != MapFieldType::STATE_MODIFIED_MAP); + EXPECT_FALSE(map_field->state() != MapFieldType::STATE_MODIFIED_REPEATED); break; case CLEAN: - EXPECT_TRUE(map_field->state_.load(std::memory_order_relaxed) != - MapFieldType::STATE_MODIFIED_MAP); - EXPECT_TRUE(map_field->state_.load(std::memory_order_relaxed) != + EXPECT_TRUE(map_field->state() != MapFieldType::STATE_MODIFIED_MAP); + EXPECT_TRUE(map_field->state() != MapFieldType::STATE_MODIFIED_REPEATED); break; default: @@ -317,19 +301,14 @@ class MapFieldStateTest } EXPECT_EQ(map_size, map->size()); - if (is_repeated_null) { - EXPECT_TRUE(repeated_field == nullptr); - } else { - if (repeated_field == nullptr) { - EXPECT_EQ(repeated_size, 0); - } else { - EXPECT_EQ(repeated_size, repeated_field->size()); - } - } + EXPECT_EQ(repeated_size, + map_field->maybe_payload() == nullptr + ? 0 + : map_field->maybe_payload()->repeated_field.size()); } std::unique_ptr arena_; - ArenaDestructor map_field_; + ArenaHolder map_field_; MapFieldBase* map_field_base_; State state_; }; @@ -342,83 +321,83 @@ INSTANTIATE_TEST_SUITE_P(MapFieldStateTestInstance, MapFieldStateTest, TEST_P(MapFieldStateTest, GetMap) { map_field_->GetMap(); if (state_ != MAP_DIRTY) { - Expect(map_field_.get(), CLEAN, 1, 1, false); + Expect(map_field_.get(), CLEAN, 1, 1); } else { - Expect(map_field_.get(), MAP_DIRTY, 1, 0, true); + Expect(map_field_.get(), MAP_DIRTY, 1, 0); } } TEST_P(MapFieldStateTest, MutableMap) { map_field_->MutableMap(); if (state_ != MAP_DIRTY) { - Expect(map_field_.get(), MAP_DIRTY, 1, 1, false); + Expect(map_field_.get(), MAP_DIRTY, 1, 1); } else { - Expect(map_field_.get(), MAP_DIRTY, 1, 0, true); + Expect(map_field_.get(), MAP_DIRTY, 1, 0); } } TEST_P(MapFieldStateTest, MergeFromClean) { - ArenaDestructor other(arena_.get()); + ArenaHolder other(arena_.get()); AddOneStillClean(other.get()); map_field_->MergeFrom(*other); if (state_ != MAP_DIRTY) { - Expect(map_field_.get(), MAP_DIRTY, 1, 1, false); + Expect(map_field_.get(), MAP_DIRTY, 1, 1); } else { - Expect(map_field_.get(), MAP_DIRTY, 1, 0, true); + Expect(map_field_.get(), MAP_DIRTY, 1, 0); } - Expect(other.get(), CLEAN, 1, 1, false); + Expect(other.get(), CLEAN, 1, 1); } TEST_P(MapFieldStateTest, MergeFromMapDirty) { - ArenaDestructor other(arena_.get()); + ArenaHolder other(arena_.get()); MakeMapDirty(other.get()); map_field_->MergeFrom(*other); if (state_ != MAP_DIRTY) { - Expect(map_field_.get(), MAP_DIRTY, 1, 1, false); + Expect(map_field_.get(), MAP_DIRTY, 1, 1); } else { - Expect(map_field_.get(), MAP_DIRTY, 1, 0, true); + Expect(map_field_.get(), MAP_DIRTY, 1, 0); } - Expect(other.get(), MAP_DIRTY, 1, 0, true); + Expect(other.get(), MAP_DIRTY, 1, 0); } TEST_P(MapFieldStateTest, MergeFromRepeatedDirty) { - ArenaDestructor other(arena_.get()); + ArenaHolder other(arena_.get()); MakeRepeatedDirty(other.get()); map_field_->MergeFrom(*other); if (state_ != MAP_DIRTY) { - Expect(map_field_.get(), MAP_DIRTY, 1, 1, false); + Expect(map_field_.get(), MAP_DIRTY, 1, 1); } else { - Expect(map_field_.get(), MAP_DIRTY, 1, 0, true); + Expect(map_field_.get(), MAP_DIRTY, 1, 0); } - Expect(other.get(), CLEAN, 1, 1, false); + Expect(other.get(), CLEAN, 1, 1); } TEST_P(MapFieldStateTest, SwapClean) { - ArenaDestructor other(arena_.get()); + ArenaHolder other(arena_.get()); AddOneStillClean(other.get()); map_field_->Swap(other.get()); - Expect(map_field_.get(), CLEAN, 1, 1, false); + Expect(map_field_.get(), CLEAN, 1, 1); switch (state_) { case CLEAN: - Expect(other.get(), CLEAN, 1, 1, false); + Expect(other.get(), CLEAN, 1, 1); break; case MAP_DIRTY: - Expect(other.get(), MAP_DIRTY, 1, 0, true); + Expect(other.get(), MAP_DIRTY, 1, 0); break; case REPEATED_DIRTY: - Expect(other.get(), REPEATED_DIRTY, 0, 1, false); + Expect(other.get(), REPEATED_DIRTY, 0, 1); break; default: break; @@ -426,22 +405,22 @@ TEST_P(MapFieldStateTest, SwapClean) { } TEST_P(MapFieldStateTest, SwapMapDirty) { - ArenaDestructor other(arena_.get()); + ArenaHolder other(arena_.get()); MakeMapDirty(other.get()); map_field_->Swap(other.get()); - Expect(map_field_.get(), MAP_DIRTY, 1, 0, true); + Expect(map_field_.get(), MAP_DIRTY, 1, 0); switch (state_) { case CLEAN: - Expect(other.get(), CLEAN, 1, 1, false); + Expect(other.get(), CLEAN, 1, 1); break; case MAP_DIRTY: - Expect(other.get(), MAP_DIRTY, 1, 0, true); + Expect(other.get(), MAP_DIRTY, 1, 0); break; case REPEATED_DIRTY: - Expect(other.get(), REPEATED_DIRTY, 0, 1, false); + Expect(other.get(), REPEATED_DIRTY, 0, 1); break; default: break; @@ -449,22 +428,22 @@ TEST_P(MapFieldStateTest, SwapMapDirty) { } TEST_P(MapFieldStateTest, SwapRepeatedDirty) { - ArenaDestructor other(arena_.get()); + ArenaHolder other(arena_.get()); MakeRepeatedDirty(other.get()); map_field_->Swap(other.get()); - Expect(map_field_.get(), REPEATED_DIRTY, 0, 1, false); + Expect(map_field_.get(), REPEATED_DIRTY, 0, 1); switch (state_) { case CLEAN: - Expect(other.get(), CLEAN, 1, 1, false); + Expect(other.get(), CLEAN, 1, 1); break; case MAP_DIRTY: - Expect(other.get(), MAP_DIRTY, 1, 0, true); + Expect(other.get(), MAP_DIRTY, 1, 0); break; case REPEATED_DIRTY: - Expect(other.get(), REPEATED_DIRTY, 0, 1, false); + Expect(other.get(), REPEATED_DIRTY, 0, 1); break; default: break; @@ -474,7 +453,7 @@ TEST_P(MapFieldStateTest, SwapRepeatedDirty) { TEST_P(MapFieldStateTest, Clear) { map_field_->Clear(); - Expect(map_field_.get(), MAP_DIRTY, 0, 0, false); + Expect(map_field_.get(), MAP_DIRTY, 0, 0); } TEST_P(MapFieldStateTest, SpaceUsedExcludingSelf) { @@ -482,13 +461,13 @@ TEST_P(MapFieldStateTest, SpaceUsedExcludingSelf) { switch (state_) { case CLEAN: - Expect(map_field_.get(), CLEAN, 1, 1, false); + Expect(map_field_.get(), CLEAN, 1, 1); break; case MAP_DIRTY: - Expect(map_field_.get(), MAP_DIRTY, 1, 0, true); + Expect(map_field_.get(), MAP_DIRTY, 1, 0); break; case REPEATED_DIRTY: - Expect(map_field_.get(), REPEATED_DIRTY, 0, 1, false); + Expect(map_field_.get(), REPEATED_DIRTY, 0, 1); break; default: break; @@ -499,9 +478,9 @@ TEST_P(MapFieldStateTest, GetMapField) { map_field_base_->GetRepeatedField(); if (state_ != REPEATED_DIRTY) { - Expect(map_field_.get(), CLEAN, 1, 1, false); + Expect(map_field_.get(), CLEAN, 1, 1); } else { - Expect(map_field_.get(), REPEATED_DIRTY, 0, 1, false); + Expect(map_field_.get(), REPEATED_DIRTY, 0, 1); } } @@ -509,20 +488,16 @@ TEST_P(MapFieldStateTest, MutableMapField) { map_field_base_->MutableRepeatedField(); if (state_ != REPEATED_DIRTY) { - Expect(map_field_.get(), REPEATED_DIRTY, 1, 1, false); + Expect(map_field_.get(), REPEATED_DIRTY, 1, 1); } else { - Expect(map_field_.get(), REPEATED_DIRTY, 0, 1, false); + Expect(map_field_.get(), REPEATED_DIRTY, 0, 1); } } -class MyMapField - : public MapField { - public: - constexpr MyMapField() - : MyMapField::MapField(internal::ConstantInitialized{}) {} -}; +using MyMapField = + MapField; TEST(MapFieldTest, ConstInit) { // This tests that `MapField` and all its base classes can be constant