From 150b724d76f4a84eae1cc9e5b40a27ad9f581c63 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Tue, 28 Nov 2023 04:35:05 -0800 Subject: [PATCH] Cleanup "mergeable" types. All types supported by RepeatedPtrField have corresponding merge/copy, hence TypeImplementsMergeBehavior is obsolete. PiperOrigin-RevId: 585928795 --- src/google/protobuf/repeated_ptr_field.h | 175 ++++------------------- 1 file changed, 28 insertions(+), 147 deletions(-) diff --git a/src/google/protobuf/repeated_ptr_field.h b/src/google/protobuf/repeated_ptr_field.h index 65adafe5ba..e50e015448 100644 --- a/src/google/protobuf/repeated_ptr_field.h +++ b/src/google/protobuf/repeated_ptr_field.h @@ -81,43 +81,6 @@ inline void memswap(char* PROTOBUF_RESTRICT a, char* PROTOBUF_RESTRICT b) { std::swap_ranges(a, a + N, b); } -// type-traits helper for RepeatedPtrFieldBase: we only want to invoke -// arena-related "copy if on different arena" behavior if the necessary methods -// exist on the contained type. In particular, we rely on MergeFrom() existing -// as a general proxy for the fact that a copy will work, and we also provide a -// specific override for std::string*. -template -struct TypeImplementsMergeBehaviorProbeForMergeFrom { - typedef char HasMerge; - typedef long HasNoMerge; - - // We accept either of: - // - void MergeFrom(const T& other) - // - bool MergeFrom(const T& other) - // - // We mangle these names a bit to avoid compatibility issues in 'unclean' - // include environments that may have, e.g., "#define test ..." (yes, this - // exists). - template - struct CheckType; - template - static HasMerge Check(CheckType*); - template - static HasMerge Check(CheckType*); - template - static HasNoMerge Check(...); - - // Resolves to either std::true_type or std::false_type. - typedef std::integral_constant(0)) == sizeof(HasMerge))> - type; -}; - -template -using TypeImplementsMergeBehavior = - absl::disjunction, - TypeImplementsMergeBehaviorProbeForMergeFrom>; - template struct IsMovable : std::integral_constant::value && @@ -420,9 +383,24 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { template void AddAllocated(Value* value) { - typename TypeImplementsMergeBehavior>::type t; ABSL_DCHECK_NE(value, nullptr); - AddAllocatedInternal(value, t); + Arena* element_arena = TypeHandler::GetArena(value); + Arena* arena = GetArena(); + if (arena != element_arena || AllocatedSizeAtCapacity()) { + AddAllocatedSlowWithCopy(value, element_arena, arena); + return; + } + // Fast path: underlying arena representation (tagged pointer) is equal to + // our arena pointer, and we can add to array without resizing it (at + // least one slot that is not allocated). + void** elems = elements(); + if (current_size_ < allocated_size()) { + // Make space at [current] by moving first allocated element to end of + // allocated list. + elems[allocated_size()] = elems[current_size_]; + } + elems[ExchangeCurrentSize(current_size_ + 1)] = value; + if (!using_sso()) ++rep()->allocated_size; } template @@ -455,8 +433,17 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { template PROTOBUF_NODISCARD Value* ReleaseLast() { - typename TypeImplementsMergeBehavior>::type t; - return ReleaseLastInternal(t); + Value* result = UnsafeArenaReleaseLast(); + // Now perform a copy if we're on an arena. + Arena* arena = GetArena(); + +#ifdef PROTOBUF_FORCE_COPY_IN_RELEASE + auto* new_result = copy(result); + if (arena == nullptr) delete result; +#else // PROTOBUF_FORCE_COPY_IN_RELEASE + auto* new_result = (arena == nullptr) ? result : copy(result); +#endif // !PROTOBUF_FORCE_COPY_IN_RELEASE + return new_result; } // Releases and returns the last element, but does not do out-of-arena copy. @@ -512,48 +499,6 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { } } - // AddAllocated version that implements arena-safe copying behavior. - template - void AddAllocatedInternal(Value* value, std::true_type) { - Arena* element_arena = TypeHandler::GetArena(value); - Arena* arena = GetArena(); - if (arena != element_arena || AllocatedSizeAtCapacity()) { - AddAllocatedSlowWithCopy(value, element_arena, arena); - return; - } - // Fast path: underlying arena representation (tagged pointer) is equal to - // our arena pointer, and we can add to array without resizing it (at - // least one slot that is not allocated). - void** elems = elements(); - if (current_size_ < allocated_size()) { - // Make space at [current] by moving first allocated element to end of - // allocated list. - elems[allocated_size()] = elems[current_size_]; - } - elems[ExchangeCurrentSize(current_size_ + 1)] = value; - if (!using_sso()) ++rep()->allocated_size; - } - - // AddAllocated version that does not implement arena-safe copying behavior. - template - void AddAllocatedInternal(Value* value, std::false_type) { - if (AllocatedSizeAtCapacity()) { - UnsafeArenaAddAllocated(value); - return; - } - // Fast path: underlying arena representation (tagged pointer) is equal to - // our arena pointer, and we can add to array without resizing it (at - // least one slot that is not allocated). - void** elems = elements(); - if (current_size_ < allocated_size()) { - // Make space at [current] by moving first allocated element to end of - // allocated list. - elems[allocated_size()] = elems[current_size_]; - } - elems[ExchangeCurrentSize(current_size_ + 1)] = value; - if (!using_sso()) ++rep()->allocated_size; - } - // Slowpath handles all cases, copying if necessary. template PROTOBUF_NOINLINE void AddAllocatedSlowWithCopy( @@ -576,36 +521,6 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { UnsafeArenaAddAllocated(value); } - template - Value* ReleaseLastInternal(std::true_type) { - // ReleaseLast() for types that implement merge/copy behavior. - // First, release an element. - Value* result = UnsafeArenaReleaseLast(); - // Now perform a copy if we're on an arena. - Arena* arena = GetArena(); - -#ifdef PROTOBUF_FORCE_COPY_IN_RELEASE - auto* new_result = copy(result); - if (arena == nullptr) delete result; -#else // PROTOBUF_FORCE_COPY_IN_RELEASE - auto* new_result = (arena == nullptr) ? result : copy(result); -#endif // !PROTOBUF_FORCE_COPY_IN_RELEASE - return new_result; - } - - template - Value* ReleaseLastInternal(std::false_type) { - // ReleaseLast() for types that *do not* implement merge/copy behavior -- - // this is the same as UnsafeArenaReleaseLast(). Note that we - // ABSL_DCHECK-fail if we're on an arena, since the user really should - // implement the copy operation in this case. - ABSL_DCHECK(GetArena() == nullptr) - << "ReleaseLast() called on a RepeatedPtrField that is on an arena, " - << "with a type that does not implement MergeFrom. This is unsafe; " - << "please implement MergeFrom for your type."; - return UnsafeArenaReleaseLast(); - } - template PROTOBUF_NOINLINE void SwapFallback(RepeatedPtrFieldBase* other) { #ifdef PROTOBUF_FORCE_COPY_IN_SWAP @@ -1323,15 +1238,6 @@ class RepeatedPtrField final : private internal::RepeatedPtrFieldBase { RepeatedPtrField(Arena* arena, RepeatedPtrField&& rhs); - // Implementations for ExtractSubrange(). The copying behavior must be - // included only if the type supports the necessary operations (e.g., - // MergeFrom()), so we must resolve this at compile time. ExtractSubrange() - // uses SFINAE to choose one of the below implementations. - void ExtractSubrangeInternal(int start, int num, Element** elements, - std::true_type); - void ExtractSubrangeInternal(int start, int num, Element** elements, - std::false_type); - void AddAllocatedForParse(Element* p) { return RepeatedPtrFieldBase::AddAllocatedForParse(p); } @@ -1508,16 +1414,6 @@ inline void RepeatedPtrField::DeleteSubrange(int start, int num) { template inline void RepeatedPtrField::ExtractSubrange(int start, int num, Element** elements) { - typename internal::TypeImplementsMergeBehavior< - typename TypeHandler::Type>::type t; - ExtractSubrangeInternal(start, num, elements, t); -} - -// ExtractSubrange() implementation for types that implement merge/copy -// behavior. -template -inline void RepeatedPtrField::ExtractSubrangeInternal( - int start, int num, Element** elements, std::true_type) { ABSL_DCHECK_GE(start, 0); ABSL_DCHECK_GE(num, 0); ABSL_DCHECK_LE(start + num, size()); @@ -1555,21 +1451,6 @@ inline void RepeatedPtrField::ExtractSubrangeInternal( CloseGap(start, num); } -// ExtractSubrange() implementation for types that do not implement merge/copy -// behavior. -template -inline void RepeatedPtrField::ExtractSubrangeInternal( - int start, int num, Element** elements, std::false_type) { - // This case is identical to UnsafeArenaExtractSubrange(). However, since - // ExtractSubrange() must return heap-allocated objects by contract, and we - // cannot fulfill this contract if we are an on arena, we must ABSL_DCHECK() - // that we are not on an arena. - ABSL_DCHECK(GetArena() == nullptr) - << "ExtractSubrange() when arena is non-nullptr is only supported when " - << "the Element type supplies a MergeFrom() operation to make copies."; - UnsafeArenaExtractSubrange(start, num, elements); -} - template inline void RepeatedPtrField::UnsafeArenaExtractSubrange( int start, int num, Element** elements) {