Skip to content

Commit

Permalink
Untangle GenericTypeHandler cross-references + more IWYU in repeate…
Browse files Browse the repository at this point in the history
…d_ptr_field.*

* `GenericTypeHandler` used to be forward-declared in message_lite.h and unncessarily friended by `MessageLite`, which was its only use in that source. repeated_ptr_field.h relied on that forward declaration being there, and fully defined `GenericTypeHandler` below the first use.
* `StringPieceField` was forward-declared, and now IWYU'ed.

PiperOrigin-RevId: 670689696
  • Loading branch information
protobuf-github-bot authored and copybara-github committed Sep 4, 2024
1 parent f4c04a8 commit 6af1a6c
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 67 deletions.
1 change: 1 addition & 0 deletions rust/cargo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ version = "4.27.3-beta.0"
edition = "2021"
links = "libupb"
license = "BSD-3-Clause"
rust-version = "1.74"

[lib]
path = "src/shared.rs"
Expand Down
5 changes: 0 additions & 5 deletions src/google/protobuf/message_lite.h
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,6 @@ class WireFormatLite;
class WeakFieldMap;
class RustMapHelper;

template <typename Type>
class GenericTypeHandler; // defined in repeated_field.h

// We compute sizes as size_t but cache them as int. This function converts a
// computed size to a cached size. Since we don't proceed with serialization
// if the total size was > INT_MAX, it is not important what this function
Expand Down Expand Up @@ -963,8 +960,6 @@ class PROTOBUF_EXPORT MessageLite {

template <typename Type>
friend class Arena::InternalHelper;
template <typename Type>
friend class internal::GenericTypeHandler;

friend auto internal::GetClassData(const MessageLite& msg);

Expand Down
7 changes: 4 additions & 3 deletions src/google/protobuf/repeated_field.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,13 @@
#include <iterator>
#include <limits>
#include <memory>
#include <string>
#include <type_traits>
#include <utility>

#include "absl/base/attributes.h"
#include "absl/base/dynamic_annotations.h"
#include "absl/base/optimization.h"
#include "absl/log/absl_check.h"
#include "absl/log/absl_log.h"
#include "absl/meta/type_traits.h"
#include "absl/strings/cord.h"
#include "google/protobuf/arena.h"
Expand All @@ -46,7 +44,6 @@
#include "google/protobuf/port.h"
#include "google/protobuf/repeated_ptr_field.h"


// Must be included last.
#include "google/protobuf/port_def.inc"

Expand Down Expand Up @@ -79,7 +76,11 @@ constexpr int RepeatedFieldLowerClampLimit() {
// overflows when multiplied by 2 (which is undefined behavior). Sizes above
// this will clamp to the maximum int value instead of following exponential
// growth when growing a repeated field.
#if defined(__cpp_inline_variables)
inline constexpr int kRepeatedFieldUpperClampLimit =
#else
constexpr int kRepeatedFieldUpperClampLimit =
#endif
(std::numeric_limits<int>::max() / 2) + 1;

template <typename Element>
Expand Down
2 changes: 0 additions & 2 deletions src/google/protobuf/repeated_field_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include <cstdint>
#include <cstdlib>
#include <cstring>
#include <functional>
#include <iterator>
#include <limits>
#include <list>
Expand All @@ -34,7 +33,6 @@
#include <gtest/gtest.h>
#include "absl/log/absl_check.h"
#include "absl/numeric/bits.h"
#include "absl/random/random.h"
#include "absl/strings/cord.h"
#include "absl/strings/str_cat.h"
#include "absl/types/span.h"
Expand Down
2 changes: 1 addition & 1 deletion src/google/protobuf/repeated_ptr_field.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
#include <cstdint>
#include <cstring>
#include <limits>
#include <new>
#include <string>

#include "absl/base/prefetch.h"
#include "absl/log/absl_check.h"
#include "google/protobuf/arena.h"
#include "google/protobuf/message_lite.h"
Expand Down
116 changes: 60 additions & 56 deletions src/google/protobuf/repeated_ptr_field.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
#include <cstdint>
#include <iterator>
#include <limits>
#include <new>
#include <string>
#include <tuple>
#include <type_traits>
#include <utility>

Expand All @@ -40,7 +40,6 @@
#include "google/protobuf/message_lite.h"
#include "google/protobuf/port.h"


// Must be included last.
#include "google/protobuf/port_def.inc"

Expand Down Expand Up @@ -92,42 +91,51 @@ struct IsMovable
// Do not use this struct - it exists for internal use only.
template <typename T>
struct ArenaOffsetHelper {
constexpr static size_t value = offsetof(T, arena_);
static constexpr size_t value = offsetof(T, arena_);
};

// Defined further below.
template <typename Type>
class GenericTypeHandler;

// This is the common base class for RepeatedPtrFields. It deals only in void*
// pointers. Users should not use this interface directly.
//
// The methods of this interface correspond to the methods of RepeatedPtrField,
// but may have a template argument called TypeHandler. Its signature is:
// class TypeHandler {
// public:
// typedef MyType Type;
// static Type* New();
// static Type* NewFromPrototype(const Type* prototype,
// Arena* arena);
// static void Delete(Type*);
// using Type = MyType;
// using Movable = ...;
//
// static Type*(*)(Arena*) GetNewFunc();
// static void GetArena(Type* value);
//
// static Type* New(Arena* arena);
// static Type* New(Arena* arena, Type&& value);
// static Type* NewFromPrototype(const Type* prototype, Arena* arena);
// static void Delete(Type*, Arena* arena);
// static void Clear(Type*);
// static void Merge(const Type& from, Type* to);
//
// // Only needs to be implemented if SpaceUsedExcludingSelf() is called.
// static int SpaceUsedLong(const Type&);
// };
class PROTOBUF_EXPORT RepeatedPtrFieldBase {
template <typename Handler>
using Value = typename Handler::Type;
template <typename TypeHandler>
using Value = typename TypeHandler::Type;

static constexpr int kSSOCapacity = 1;

using ElementFactory = void* (*)(Arena*);

protected:
// We use the same Handler for all Message types to deduplicate generated
// We use the same TypeHandler for all Message types to deduplicate generated
// code.
template <typename Handler>
template <typename TypeHandler>
using CommonHandler = typename std::conditional<
std::is_base_of<MessageLite, Value<Handler>>::value,
internal::GenericTypeHandler<MessageLite>, Handler>::type;
std::is_base_of<MessageLite, Value<TypeHandler>>::value,
GenericTypeHandler<MessageLite>, TypeHandler>::type;

constexpr RepeatedPtrFieldBase()
: tagged_rep_or_elem_(nullptr),
Expand All @@ -145,7 +153,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase {

~RepeatedPtrFieldBase() {
#ifndef NDEBUG
// Try to trigger segfault / asan failure in non-opt builds. If arena_
// Try to trigger segfault / asan failure in non-opt builds if arena_
// lifetime has ended before the destructor.
if (arena_) (void)arena_->SpaceAllocated();
#endif
Expand Down Expand Up @@ -182,12 +190,12 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase {
return cast<TypeHandler>(element_at(index));
}

template <typename Handler>
Value<Handler>* Add() {
if (std::is_same<Value<Handler>, std::string>{}) {
return cast<Handler>(AddString());
template <typename TypeHandler>
Value<TypeHandler>* Add() {
if (std::is_same<Value<TypeHandler>, std::string>{}) {
return cast<TypeHandler>(AddString());
}
return cast<Handler>(AddMessageLite(Handler::GetNewFunc()));
return cast<TypeHandler>(AddMessageLite(TypeHandler::GetNewFunc()));
}

template <
Expand Down Expand Up @@ -318,9 +326,9 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase {
template <typename TypeHandler>
void CopyFrom(const RepeatedPtrFieldBase& other) {
if (&other == this) return;
RepeatedPtrFieldBase::Clear<TypeHandler>();
Clear<TypeHandler>();
if (other.empty()) return;
RepeatedPtrFieldBase::MergeFrom<typename TypeHandler::Type>(other);
MergeFrom<typename TypeHandler::Type>(other);
}

void CloseGap(int start, int num);
Expand Down Expand Up @@ -767,8 +775,8 @@ void RepeatedPtrFieldBase::MergeFrom<std::string>(
const RepeatedPtrFieldBase& from);


template <typename F>
void* RepeatedPtrFieldBase::AddInternal(F factory) {
template <typename Factory>
void* RepeatedPtrFieldBase::AddInternal(Factory factory) {
Arena* const arena = GetArena();
if (tagged_rep_or_elem_ == nullptr) {
ExchangeCurrentSize(1);
Expand Down Expand Up @@ -808,27 +816,30 @@ PROTOBUF_EXPORT void InternalOutOfLineDeleteMessageLite(MessageLite* message);
template <typename GenericType>
class GenericTypeHandler {
public:
typedef GenericType Type;
using Movable = IsMovable<GenericType>;
using Type = GenericType;
using Movable = IsMovable<Type>;

static constexpr auto GetNewFunc() { return Arena::DefaultConstruct<Type>; }
static inline Arena* GetArena(Type* value) {
return Arena::InternalGetArena(value);
}

static inline GenericType* New(Arena* arena) {
return static_cast<GenericType*>(Arena::DefaultConstruct<Type>(arena));
static inline Type* New(Arena* arena) {
return static_cast<Type*>(Arena::DefaultConstruct<Type>(arena));
}
static inline GenericType* New(Arena* arena, GenericType&& value) {
return Arena::Create<GenericType>(arena, std::move(value));
static inline Type* New(Arena* arena, Type&& value) {
return Arena::Create<Type>(arena, std::move(value));
}
static inline GenericType* NewFromPrototype(const GenericType* /*prototype*/,
Arena* arena = nullptr) {
static inline Type* NewFromPrototype(const Type* /*prototype*/,
Arena* arena = nullptr) {
return New(arena);
}
static inline void Delete(GenericType* value, Arena* arena) {
static inline void Delete(Type* value, Arena* arena) {
if (arena != nullptr) return;
#ifdef __cpp_if_constexpr
if constexpr (std::is_base_of<MessageLite, GenericType>::value) {
if constexpr (std::is_base_of<MessageLite, Type>::value) {
// Using virtual destructor to reduce generated code size that would have
// happened otherwise due to inlined `~GenericType`.
// happened otherwise due to inlined `~Type()`.
InternalOutOfLineDeleteMessageLite(value);
} else {
delete value;
Expand All @@ -837,13 +848,9 @@ class GenericTypeHandler {
delete value;
#endif
}
static inline Arena* GetArena(GenericType* value) {
return Arena::InternalGetArena(value);
}

static inline void Clear(GenericType* value) { value->Clear(); }
static void Merge(const GenericType& from, GenericType* to);
static inline size_t SpaceUsedLong(const GenericType& value) {
static inline void Clear(Type* value) { value->Clear(); }
static void Merge(const Type& from, Type* to);
static inline size_t SpaceUsedLong(const Type& value) {
return value.SpaceUsedLong();
}
};
Expand Down Expand Up @@ -887,32 +894,29 @@ PROTOBUF_EXPORT void* NewStringElement(Arena* arena);
template <>
class GenericTypeHandler<std::string> {
public:
typedef std::string Type;
using Type = std::string;
using Movable = IsMovable<Type>;

static constexpr auto GetNewFunc() { return NewStringElement; }
static inline Arena* GetArena(Type*) { return nullptr; }

static PROTOBUF_NOINLINE std::string* New(Arena* arena) {
return Arena::Create<std::string>(arena);
static PROTOBUF_NOINLINE Type* New(Arena* arena) {
return Arena::Create<Type>(arena);
}
static PROTOBUF_NOINLINE std::string* New(Arena* arena, std::string&& value) {
return Arena::Create<std::string>(arena, std::move(value));
static PROTOBUF_NOINLINE Type* New(Arena* arena, Type&& value) {
return Arena::Create<Type>(arena, std::move(value));
}
static inline std::string* NewFromPrototype(const std::string*,
Arena* arena) {
static inline Type* NewFromPrototype(const Type*, Arena* arena) {
return New(arena);
}
static inline Arena* GetArena(std::string*) { return nullptr; }
static inline void Delete(std::string* value, Arena* arena) {
static inline void Delete(Type* value, Arena* arena) {
if (arena == nullptr) {
delete value;
}
}
static inline void Clear(std::string* value) { value->clear(); }
static inline void Merge(const std::string& from, std::string* to) {
*to = from;
}
static size_t SpaceUsedLong(const std::string& value) {
static inline void Clear(Type* value) { value->clear(); }
static inline void Merge(const Type& from, Type* to) { *to = from; }
static size_t SpaceUsedLong(const Type& value) {
return sizeof(value) + StringSpaceUsedExcludingSelfLong(value);
}
};
Expand Down

0 comments on commit 6af1a6c

Please sign in to comment.