diff --git a/src/google/protobuf/generated_message_util.cc b/src/google/protobuf/generated_message_util.cc index b9fb77ef8f..4893ca80b9 100644 --- a/src/google/protobuf/generated_message_util.cc +++ b/src/google/protobuf/generated_message_util.cc @@ -368,7 +368,7 @@ void GenericSwap(MessageLite* m1, MessageLite* m2) { MessageLite* GetOwnedMessageInternal(Arena* message_arena, MessageLite* submessage, Arena* submessage_arena) { - ABSL_DCHECK(Arena::InternalGetOwningArena(submessage) == submessage_arena); + ABSL_DCHECK(submessage->GetArena() == submessage_arena); ABSL_DCHECK(message_arena != submessage_arena); ABSL_DCHECK_EQ(submessage_arena, nullptr); if (message_arena != nullptr && submessage_arena == nullptr) { diff --git a/src/google/protobuf/message.cc b/src/google/protobuf/message.cc index 6f04c4c911..b4b1a40c51 100644 --- a/src/google/protobuf/message.cc +++ b/src/google/protobuf/message.cc @@ -414,8 +414,8 @@ template <> PROTOBUF_NOINLINE #endif Arena* - GenericTypeHandler::GetOwningArena(Message* value) { - return value->GetOwningArena(); + GenericTypeHandler::GetArena(Message* value) { + return value->GetArena(); } template void InternalMetadata::DoClear(); diff --git a/src/google/protobuf/proto3_arena_unittest.cc b/src/google/protobuf/proto3_arena_unittest.cc index 519c176d5e..7a46ab6f20 100644 --- a/src/google/protobuf/proto3_arena_unittest.cc +++ b/src/google/protobuf/proto3_arena_unittest.cc @@ -27,19 +27,6 @@ using proto3_arena_unittest::TestAllTypes; namespace google { namespace protobuf { - -namespace internal { - -class Proto3ArenaTestHelper { - public: - template - static Arena* GetOwningArena(const T& msg) { - return msg.GetOwningArena(); - } -}; - -} // namespace internal - namespace { // We selectively set/check a few representative fields rather than all fields // as this test is only expected to cover the basics of arena support. diff --git a/src/google/protobuf/reflection_ops.cc b/src/google/protobuf/reflection_ops.cc index d1153b81d1..03b0d38b69 100644 --- a/src/google/protobuf/reflection_ops.cc +++ b/src/google/protobuf/reflection_ops.cc @@ -415,16 +415,14 @@ void ReflectionOps::FindInitializationErrors(const Message& message, void GenericSwap(Message* lhs, Message* rhs) { #ifndef PROTOBUF_FORCE_COPY_IN_SWAP - ABSL_DCHECK(Arena::InternalGetOwningArena(lhs) != - Arena::InternalGetOwningArena(rhs)); - ABSL_DCHECK(Arena::InternalGetOwningArena(lhs) != nullptr || - Arena::InternalGetOwningArena(rhs) != nullptr); + ABSL_DCHECK(lhs->GetArena() != rhs->GetArena()); + ABSL_DCHECK(lhs->GetArena() != nullptr || rhs->GetArena() != nullptr); #endif // !PROTOBUF_FORCE_COPY_IN_SWAP // At least one of these must have an arena, so make `rhs` point to it. - Arena* arena = Arena::InternalGetOwningArena(rhs); + Arena* arena = rhs->GetArena(); if (arena == nullptr) { std::swap(lhs, rhs); - arena = Arena::InternalGetOwningArena(rhs); + arena = rhs->GetArena(); } // Improve efficiency by placing the temporary on an arena so that messages diff --git a/src/google/protobuf/repeated_field.h b/src/google/protobuf/repeated_field.h index ed3233b1e7..7e3f8d2e36 100644 --- a/src/google/protobuf/repeated_field.h +++ b/src/google/protobuf/repeated_field.h @@ -501,7 +501,7 @@ RepeatedField::~RepeatedField() { #ifndef NDEBUG // Try to trigger segfault / asan failure in non-opt builds if arena_ // lifetime has ended before the destructor. - auto arena = GetOwningArena(); + auto arena = GetArena(); if (arena) (void)arena->SpaceAllocated(); #endif if (total_size_ > 0) { @@ -526,7 +526,7 @@ inline RepeatedField::RepeatedField(RepeatedField&& other) noexcept // We don't just call Swap(&other) here because it would perform 3 copies if // other is on an arena. This field can't be on an arena because arena // construction always uses the Arena* accepting constructor. - if (other.GetOwningArena()) { + if (other.GetArena()) { CopyFrom(other); } else { InternalSwap(&other); @@ -540,9 +540,9 @@ inline RepeatedField& RepeatedField::operator=( // We don't just call Swap(&other) here because it would perform 3 copies if // the two fields are on different arenas. if (this != &other) { - if (GetOwningArena() != other.GetOwningArena() + if (GetArena() != other.GetArena() #ifdef PROTOBUF_FORCE_COPY_IN_MOVE - || GetOwningArena() == nullptr + || GetArena() == nullptr #endif // !PROTOBUF_FORCE_COPY_IN_MOVE ) { CopyFrom(other); @@ -835,14 +835,13 @@ template void RepeatedField::Swap(RepeatedField* other) { if (this == other) return; #ifdef PROTOBUF_FORCE_COPY_IN_SWAP - if (GetOwningArena() != nullptr && - GetOwningArena() == other->GetOwningArena()) { + if (GetArena() != nullptr && GetArena() == other->GetArena()) { #else // PROTOBUF_FORCE_COPY_IN_SWAP - if (GetOwningArena() == other->GetOwningArena()) { + if (GetArena() == other->GetArena()) { #endif // !PROTOBUF_FORCE_COPY_IN_SWAP InternalSwap(other); } else { - RepeatedField temp(other->GetOwningArena()); + RepeatedField temp(other->GetArena()); temp.MergeFrom(*this); CopyFrom(*other); other->UnsafeArenaSwap(&temp); @@ -852,7 +851,7 @@ void RepeatedField::Swap(RepeatedField* other) { template void RepeatedField::UnsafeArenaSwap(RepeatedField* other) { if (this == other) return; - ABSL_DCHECK_EQ(GetOwningArena(), other->GetOwningArena()); + ABSL_DCHECK_EQ(GetArena(), other->GetArena()); InternalSwap(other); } @@ -941,7 +940,7 @@ PROTOBUF_NOINLINE void RepeatedField::GrowNoAnnotate(int current_size, int new_size) { ABSL_DCHECK_GT(new_size, total_size_); Rep* new_rep; - Arena* arena = GetOwningArena(); + Arena* arena = GetArena(); new_size = internal::CalculateReserveSize( total_size_, new_size); diff --git a/src/google/protobuf/repeated_ptr_field.cc b/src/google/protobuf/repeated_ptr_field.cc index 6e53d56861..6e419c6494 100644 --- a/src/google/protobuf/repeated_ptr_field.cc +++ b/src/google/protobuf/repeated_ptr_field.cc @@ -35,7 +35,7 @@ void** RepeatedPtrFieldBase::InternalExtend(int extend_amount) { ABSL_DCHECK(extend_amount > 0); constexpr size_t ptr_size = sizeof(rep()->elements[0]); int new_capacity = total_size_ + extend_amount; - Arena* arena = GetOwningArena(); + Arena* arena = GetArena(); new_capacity = internal::CalculateReserveSize( total_size_, new_capacity); ABSL_CHECK_LE( @@ -234,7 +234,7 @@ void RepeatedPtrFieldBase::MergeFromConcreteMessage( dst += recycled; src += recycled; } - Arena* arena = GetOwningArena(); + Arena* arena = GetArena(); for (; src < end; ++src, ++dst) { *dst = copy_fn(arena, **src); } @@ -257,7 +257,7 @@ void RepeatedPtrFieldBase::MergeFrom( dst += recycled; src += recycled; } - Arena* arena = GetOwningArena(); + Arena* arena = GetArena(); for (; src < end; ++src, ++dst) { *dst = (*src)->New(arena); (*dst)->CheckTypeAndMergeFrom(**src); diff --git a/src/google/protobuf/repeated_ptr_field.h b/src/google/protobuf/repeated_ptr_field.h index 85a20dad27..d9d8d8fb4c 100644 --- a/src/google/protobuf/repeated_ptr_field.h +++ b/src/google/protobuf/repeated_ptr_field.h @@ -398,10 +398,9 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { template PROTOBUF_NDEBUG_INLINE void Swap(RepeatedPtrFieldBase* other) { #ifdef PROTOBUF_FORCE_COPY_IN_SWAP - if (GetOwningArena() != nullptr && - GetOwningArena() == other->GetOwningArena()) + if (GetArena() != nullptr && GetArena() == other->GetArena()) #else // PROTOBUF_FORCE_COPY_IN_SWAP - if (GetOwningArena() == other->GetOwningArena()) + if (GetArena() == other->GetArena()) #endif // !PROTOBUF_FORCE_COPY_IN_SWAP { InternalSwap(other); @@ -507,10 +506,9 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { template void AddCleared(Value* value) { - ABSL_DCHECK(GetOwningArena() == nullptr) - << "AddCleared() can only be used on a " - "RepeatedPtrField not on an arena."; - ABSL_DCHECK(TypeHandler::GetOwningArena(value) == nullptr) + ABSL_DCHECK(GetArena() == nullptr) << "AddCleared() can only be used on a " + "RepeatedPtrField not on an arena."; + ABSL_DCHECK(TypeHandler::GetArena(value) == nullptr) << "AddCleared() can only accept values not on an arena."; MaybeExtend(); if (using_sso()) { @@ -522,7 +520,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { template PROTOBUF_NODISCARD Value* ReleaseCleared() { - ABSL_DCHECK(GetOwningArena() == nullptr) + ABSL_DCHECK(GetArena() == nullptr) << "ReleaseCleared() can only be used on a RepeatedPtrField not on " << "an arena."; ABSL_DCHECK(tagged_rep_or_elem_ != nullptr); @@ -539,8 +537,8 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { // AddAllocated version that implements arena-safe copying behavior. template void AddAllocatedInternal(Value* value, std::true_type) { - Arena* element_arena = TypeHandler::GetOwningArena(value); - Arena* arena = GetOwningArena(); + Arena* element_arena = TypeHandler::GetArena(value); + Arena* arena = GetArena(); if (arena == element_arena && allocated_size() < total_size_) { // Fast path: underlying arena representation (tagged pointer) is equal to // our arena pointer, and we can add to array without resizing it (at @@ -606,7 +604,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { // First, release an element. Value* result = UnsafeArenaReleaseLast(); // Now perform a copy if we're on an arena. - Arena* arena = GetOwningArena(); + Arena* arena = GetArena(); #ifdef PROTOBUF_FORCE_COPY_IN_RELEASE auto* new_result = copy(result); @@ -623,7 +621,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { // 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(GetOwningArena() == nullptr) + 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."; @@ -633,16 +631,15 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { template PROTOBUF_NOINLINE void SwapFallback(RepeatedPtrFieldBase* other) { #ifdef PROTOBUF_FORCE_COPY_IN_SWAP - ABSL_DCHECK(GetOwningArena() == nullptr || - other->GetOwningArena() != GetOwningArena()); + ABSL_DCHECK(GetArena() == nullptr || other->GetArena() != GetArena()); #else // PROTOBUF_FORCE_COPY_IN_SWAP - ABSL_DCHECK(other->GetOwningArena() != GetOwningArena()); + ABSL_DCHECK(other->GetArena() != GetArena()); #endif // !PROTOBUF_FORCE_COPY_IN_SWAP // Copy semantics in this case. We try to improve efficiency by placing the // temporary on |other|'s arena so that messages are copied twice rather // than three times. - RepeatedPtrFieldBase temp(other->GetOwningArena()); + RepeatedPtrFieldBase temp(other->GetArena()); if (!this->empty()) { temp.MergeFrom(*this); } @@ -652,7 +649,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { } // Gets the Arena on which this RepeatedPtrField stores its elements. - inline Arena* GetArena() const { return GetOwningArena(); } + inline Arena* GetArena() const { return arena_; } inline Arena* GetOwningArena() const { return arena_; } @@ -898,8 +895,8 @@ class GenericTypeHandler { delete value; #endif } - static inline Arena* GetOwningArena(GenericType* value) { - return Arena::InternalGetOwningArena(value); + static inline Arena* GetArena(GenericType* value) { + return Arena::InternalGetArena(value); } static inline void Clear(GenericType* value) { value->Clear(); } @@ -921,9 +918,8 @@ inline MessageLite* GenericTypeHandler::NewFromPrototype( return NewFromPrototypeHelper(prototype, arena); } template <> -inline Arena* GenericTypeHandler::GetOwningArena( - MessageLite* value) { - return value->GetOwningArena(); +inline Arena* GenericTypeHandler::GetArena(MessageLite* value) { + return value->GetArena(); } template @@ -950,8 +946,7 @@ template <> PROTOBUF_EXPORT Message* GenericTypeHandler::NewFromPrototype( const Message* prototype, Arena* arena); template <> -PROTOBUF_EXPORT Arena* GenericTypeHandler::GetOwningArena( - Message* value); +PROTOBUF_EXPORT Arena* GenericTypeHandler::GetArena(Message* value); class StringTypeHandler { public: @@ -968,7 +963,7 @@ class StringTypeHandler { Arena* arena) { return New(arena); } - static inline Arena* GetOwningArena(std::string*) { return nullptr; } + static inline Arena* GetArena(std::string*) { return nullptr; } static inline void Delete(std::string* value, Arena* arena) { if (arena == nullptr) { delete value; @@ -1408,7 +1403,7 @@ inline RepeatedPtrField::RepeatedPtrField( // We don't just call Swap(&other) here because it would perform 3 copies if // other is on an arena. This field can't be on an arena because arena // construction always uses the Arena* accepting constructor. - if (other.GetOwningArena()) { + if (other.GetArena()) { CopyFrom(other); } else { InternalSwap(&other); @@ -1422,9 +1417,9 @@ inline RepeatedPtrField& RepeatedPtrField::operator=( // We don't just call Swap(&other) here because it would perform 3 copies if // the two fields are on different arenas. if (this != &other) { - if (GetOwningArena() != other.GetOwningArena() + if (GetArena() != other.GetArena() #ifdef PROTOBUF_FORCE_COPY_IN_MOVE - || GetOwningArena() == nullptr + || GetArena() == nullptr #endif // !PROTOBUF_FORCE_COPY_IN_MOVE ) { CopyFrom(other); @@ -1506,7 +1501,7 @@ inline void RepeatedPtrField::DeleteSubrange(int start, int num) { ABSL_DCHECK_GE(num, 0); ABSL_DCHECK_LE(start + num, size()); void** subrange = raw_mutable_data() + start; - Arena* arena = GetOwningArena(); + Arena* arena = GetArena(); for (int i = 0; i < num; ++i) { using H = CommonHandler; H::Delete(static_cast(subrange[i]), arena); @@ -1537,7 +1532,7 @@ inline void RepeatedPtrField::ExtractSubrangeInternal( << "Releasing elements without transferring ownership is an unsafe " "operation. Use UnsafeArenaExtractSubrange."; if (elements != nullptr) { - Arena* arena = GetOwningArena(); + Arena* arena = GetArena(); auto* extracted = data() + start; #ifdef PROTOBUF_FORCE_COPY_IN_RELEASE // Always copy. @@ -1573,7 +1568,7 @@ inline void RepeatedPtrField::ExtractSubrangeInternal( // 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(GetOwningArena() == nullptr) + 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); @@ -1658,7 +1653,7 @@ template inline void RepeatedPtrField::UnsafeArenaSwap( RepeatedPtrField* other) { if (this == other) return; - ABSL_DCHECK_EQ(GetOwningArena(), other->GetOwningArena()); + ABSL_DCHECK_EQ(GetArena(), other->GetArena()); RepeatedPtrFieldBase::InternalSwap(other); } @@ -1681,7 +1676,7 @@ inline Arena* RepeatedPtrField::GetArena() const { template inline Arena* RepeatedPtrField::GetOwningArena() const { - return RepeatedPtrFieldBase::GetOwningArena(); + return RepeatedPtrFieldBase::GetArena(); } template