Skip to content

Commit cbc4f45

Browse files
ClaytonKnittelcopybara-github
ClaytonKnittel
authored and
copybara-github
committed
Align generated code with new RepeatedPtrField paradigm.
All changes to generated code are protected by `PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD`. With this change, code will now compile with `PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD` enabled. This change is a no-op and does not enable the change. All generated code is annotated with both the current behavior and the new behavior, and defining PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS will turn the feature on (with no change to codegen needed). PiperOrigin-RevId: 807862312
1 parent a739f21 commit cbc4f45

28 files changed

+2504
-390
lines changed

‎src/google/protobuf/arena.h‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -446,10 +446,19 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8)
446446
Arena* PROTOBUF_NULLABLE arena,
447447
Args&&... args) {
448448
if constexpr (internal::IsRepeatedPtrFieldType<T>::value) {
449-
using ArenaRepT = typename internal::RepeatedPtrFieldArenaRep<T>::Type;
450-
auto* arena_repr =
451-
new (ptr) ArenaRepT(arena, static_cast<Args&&>(args)...);
452-
return arena_repr;
449+
if (ABSL_PREDICT_FALSE(arena == nullptr)) {
450+
return new (ptr) T(static_cast<Args&&>(args)...);
451+
} else {
452+
using ArenaRepT =
453+
typename internal::RepeatedPtrFieldArenaRep<T>::Type;
454+
auto* arena_repr =
455+
new (ptr) ArenaRepT(arena, static_cast<Args&&>(args)...);
456+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD
457+
return &arena_repr->field();
458+
#else
459+
return arena_repr;
460+
#endif
461+
}
453462
} else {
454463
return new (ptr) T(arena, static_cast<Args&&>(args)...);
455464
}
@@ -559,7 +568,11 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8)
559568
using ArenaRepT = typename internal::RepeatedPtrFieldArenaRep<T>::Type;
560569
auto* arena_repr =
561570
arena->DoCreateMessage<ArenaRepT>(static_cast<Args&&>(args)...);
571+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD
572+
return &arena_repr->field();
573+
#else
562574
return arena_repr;
575+
#endif
563576
}
564577
}
565578

‎src/google/protobuf/arena_unittest.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,12 @@ TEST(ArenaTest, RepeatedPtrFieldMoveCtorOnArena) {
493493
TestUtil::ExpectAllFieldsSet(moved->Get(0));
494494

495495
// The only extra allocation with moves is sizeof(RepeatedPtrField).
496+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD
497+
EXPECT_EQ(usage_by_move,
498+
sizeof(internal::RepeatedPtrFieldWithArena<TestAllTypes>));
499+
#else
496500
EXPECT_EQ(usage_by_move, sizeof(internal::RepeatedPtrFieldBase));
501+
#endif
497502
EXPECT_LT(usage_by_move + sizeof(TestAllTypes), usage_original);
498503

499504
// Status after move is unspecified and must not be assumed. It's merely

‎src/google/protobuf/compiler/cpp/field.cc‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,20 @@ namespace google {
3939
namespace protobuf {
4040
namespace compiler {
4141
namespace cpp {
42+
43+
namespace {
4244
using ::google::protobuf::internal::WireFormat;
4345
using Sub = ::google::protobuf::io::Printer::Sub;
4446

47+
void InternalMetadataOffsetFormatString(io::Printer* p) {
48+
p->Emit(R"cc(
49+
$pbi$::InternalMetadataOffset::Build<
50+
$classtype$, offsetof($classtype$, _impl_.$name$_)>()
51+
)cc");
52+
}
53+
54+
} // namespace
55+
4556
std::vector<Sub> FieldVars(const FieldDescriptor* field, const Options& opts) {
4657
bool split = ShouldSplit(field, opts);
4758
std::vector<Sub> vars = {
@@ -156,7 +167,19 @@ void FieldGeneratorBase::GenerateMemberConstexprConstructor(
156167
io::Printer* p) const {
157168
ABSL_CHECK(!field_->is_extension());
158169
if (field_->is_repeated()) {
159-
p->Emit("$name$_{}");
170+
if (IsRepeatedPtrField(field_)) {
171+
p->Emit({{"internal_metadata_offset",
172+
[p] { InternalMetadataOffsetFormatString(p); }}},
173+
R"cc(
174+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD
175+
$name$_{visibility, $internal_metadata_offset$}
176+
#else // !PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD
177+
$name$_ {}
178+
#endif
179+
)cc");
180+
} else {
181+
p->Emit("$name$_{}");
182+
}
160183
} else {
161184
p->Emit({{"default", DefaultValue(options_, field_)}},
162185
"$name$_{$default$}");
@@ -170,6 +193,16 @@ void FieldGeneratorBase::GenerateMemberConstructor(io::Printer* p) const {
170193
} else if (field_->is_repeated()) {
171194
if (ShouldSplit(field_, options_)) {
172195
p->Emit("$name$_{}"); // RawPtr<Repeated>
196+
} else if (IsRepeatedPtrField(field_)) {
197+
p->Emit({{"internal_metadata_offset",
198+
[p] { InternalMetadataOffsetFormatString(p); }}},
199+
R"cc(
200+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD
201+
$name$_{visibility, $internal_metadata_offset$}
202+
#else
203+
$name$_ { visibility, arena }
204+
#endif
205+
)cc");
173206
} else {
174207
p->Emit("$name$_{visibility, arena}");
175208
}
@@ -182,7 +215,19 @@ void FieldGeneratorBase::GenerateMemberConstructor(io::Printer* p) const {
182215
void FieldGeneratorBase::GenerateMemberCopyConstructor(io::Printer* p) const {
183216
ABSL_CHECK(!field_->is_extension());
184217
if (field_->is_repeated()) {
185-
p->Emit("$name$_{visibility, arena, from.$name$_}");
218+
if (IsRepeatedPtrField(field_)) {
219+
p->Emit({{"internal_metadata_offset",
220+
[p] { InternalMetadataOffsetFormatString(p); }}},
221+
R"cc(
222+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD
223+
$name$_{visibility, ($internal_metadata_offset$), from.$name$_}
224+
#else
225+
$name$_ { visibility, arena, from.$name$_ }
226+
#endif
227+
)cc");
228+
} else {
229+
p->Emit("$name$_{visibility, arena, from.$name$_}");
230+
}
186231
} else {
187232
p->Emit("$name$_{from.$name$_}");
188233
}

‎src/google/protobuf/compiler/cpp/field_generators/message_field.cc‎

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -740,6 +740,8 @@ class RepeatedMessage : public FieldGeneratorBase {
740740
void GenerateIsInitialized(io::Printer* p) const override;
741741
bool NeedsIsInitialized() const override;
742742

743+
bool RequiresArena(GeneratorFunction function) const override;
744+
743745
private:
744746
const Options* opts_;
745747
bool has_required_;
@@ -748,7 +750,11 @@ class RepeatedMessage : public FieldGeneratorBase {
748750
void RepeatedMessage::GeneratePrivateMembers(io::Printer* p) const {
749751
if (should_split()) {
750752
p->Emit(R"cc(
753+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD
754+
$pbi$::RawPtr<$pbi$::$Weak$RepeatedPtrFieldWithArena<$Submsg$>> $name$_;
755+
#else
751756
$pbi$::RawPtr<$pb$::$Weak$RepeatedPtrField<$Submsg$>> $name$_;
757+
#endif
752758
)cc");
753759
} else {
754760
p->Emit("$pb$::$Weak$RepeatedPtrField< $Submsg$ > $name$_;\n");
@@ -831,7 +837,8 @@ void RepeatedMessage::GenerateInlineAccessorDefinitions(io::Printer* p) const {
831837
ABSL_ATTRIBUTE_LIFETIME_BOUND {
832838
$WeakDescriptorSelfPin$;
833839
$TsanDetectConcurrentMutation$;
834-
$Submsg$* _add = _internal_mutable_$name_internal$()->Add();
840+
$Submsg$* _add = _internal_mutable_$name_internal$()->AddWithArena(
841+
$pb$::MessageLite::internal_visibility(), GetArena());
835842
$set_hasbit$;
836843
$annotate_add_mutable$;
837844
// @@protoc_insertion_point(field_add:$pkg.Msg.field$)
@@ -854,17 +861,31 @@ void RepeatedMessage::GenerateInlineAccessorDefinitions(io::Printer* p) const {
854861
inline const $pb$::$Weak$RepeatedPtrField<$Submsg$>&
855862
$Msg$::_internal$_weak$_$name_internal$() const {
856863
$TsanDetectConcurrentRead$;
864+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD
865+
return $field_$->field();
866+
#else
857867
return *$field_$;
868+
#endif
858869
}
859870
inline $pb$::$Weak$RepeatedPtrField<$Submsg$>* $nonnull$
860871
$Msg$::_internal_mutable$_weak$_$name_internal$() {
861872
$TsanDetectConcurrentRead$;
862873
$PrepareSplitMessageForWrite$;
863874
if ($field_$.IsDefault()) {
875+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD
876+
$field_$.Set($superclass$::DefaultConstruct<
877+
$pbi$::$Weak$RepeatedPtrFieldWithArena<$Submsg$>>(
878+
GetArena()));
879+
#else
864880
$field_$.Set($superclass$::DefaultConstruct<
865881
$pb$::$Weak$RepeatedPtrField<$Submsg$>>(GetArena()));
882+
#endif
866883
}
884+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD
885+
return &$field_$->field();
886+
#else
867887
return $field_$.Get();
888+
#endif
868889
}
869890
)cc");
870891
} else {
@@ -908,7 +929,8 @@ void RepeatedMessage::GenerateMergingCode(io::Printer* p) const {
908929
// `if (!from.empty()) { body(); }` for both split and non-split cases.
909930
auto body = [&] {
910931
p->Emit(R"cc(
911-
_this->_internal_mutable$_weak$_$name$()->MergeFrom(
932+
_this->_internal_mutable$_weak$_$name$()->MergeFromWithArena(
933+
$pb$::MessageLite::internal_visibility(), arena,
912934
from._internal$_weak$_$name$());
913935
)cc");
914936
};
@@ -940,7 +962,9 @@ void RepeatedMessage::GenerateCopyConstructorCode(io::Printer* p) const {
940962
if (should_split()) {
941963
p->Emit(R"cc(
942964
if (!from._internal$_weak$_$name$().empty()) {
943-
_internal_mutable$_weak$_$name$()->MergeFrom(from._internal$_weak$_$name$());
965+
_internal_mutable$_weak$_$name$()->MergeFromWithArena(
966+
$pb$::MessageLite::internal_visibility(), arena,
967+
from._internal$_weak$_$name$());
944968
}
945969
)cc");
946970
}
@@ -1047,6 +1071,15 @@ void RepeatedMessage::GenerateIsInitialized(io::Printer* p) const {
10471071
}
10481072

10491073
bool RepeatedMessage::NeedsIsInitialized() const { return has_required_; }
1074+
1075+
bool RepeatedMessage::RequiresArena(GeneratorFunction func) const {
1076+
switch (func) {
1077+
case GeneratorFunction::kMergeFrom:
1078+
return true;
1079+
}
1080+
return false;
1081+
}
1082+
10501083
} // namespace
10511084

10521085
std::unique_ptr<FieldGeneratorBase> MakeSinguarMessageGenerator(

‎src/google/protobuf/compiler/cpp/field_generators/string_field.cc‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -753,12 +753,22 @@ class RepeatedString : public FieldGeneratorBase {
753753
}
754754
}
755755

756+
bool RequiresArena(GeneratorFunction function) const override {
757+
switch (function) {
758+
case GeneratorFunction::kMergeFrom:
759+
return true;
760+
}
761+
return false;
762+
}
763+
756764
void GenerateMergingCode(io::Printer* p) const override {
757765
// TODO: experiment with simplifying this to be
758766
// `if (!from.empty()) { body(); }` for both split and non-split cases.
759767
auto body = [&] {
760768
p->Emit(R"cc(
761-
_this->_internal_mutable_$name$()->MergeFrom(from._internal_$name$());
769+
_this->_internal_mutable_$name$()->MergeFromWithArena(
770+
$pb$::MessageLite::internal_visibility(), arena,
771+
from._internal_$name$());
762772
)cc");
763773
};
764774
if (!should_split()) {
@@ -793,7 +803,9 @@ class RepeatedString : public FieldGeneratorBase {
793803
if (should_split()) {
794804
p->Emit(R"cc(
795805
if (!from._internal_$name$().empty()) {
796-
_internal_mutable_$name$()->MergeFrom(from._internal_$name$());
806+
_internal_mutable_$name$()->MergeFromWithArena(
807+
$pb$::MessageLite::internal_visibility(), arena,
808+
from._internal_$name$());
797809
}
798810
)cc");
799811
}
@@ -869,7 +881,8 @@ void RepeatedString::GenerateInlineAccessorDefinitions(io::Printer* p) const {
869881
ABSL_ATTRIBUTE_LIFETIME_BOUND {
870882
$WeakDescriptorSelfPin$;
871883
$TsanDetectConcurrentMutation$;
872-
::std::string* _s = _internal_mutable_$name_internal$()->Add();
884+
::std::string* _s = _internal_mutable_$name_internal$()->AddWithArena(
885+
$pb$::MessageLite::internal_visibility(), GetArena());
873886
$set_hasbit$;
874887
$annotate_add_mutable$;
875888
// @@protoc_insertion_point(field_add_mutable:$pkg.Msg.field$)
@@ -907,9 +920,10 @@ void RepeatedString::GenerateInlineAccessorDefinitions(io::Printer* p) const {
907920
inline void $Msg$::add_$name$(Arg_&& value, Args_... args) {
908921
$WeakDescriptorSelfPin$;
909922
$TsanDetectConcurrentMutation$;
910-
$pbi$::AddToRepeatedPtrField(*_internal_mutable_$name_internal$(),
911-
::std::forward<Arg_>(value),
912-
args... $bytes_tag$);
923+
$pbi$::AddToRepeatedPtrField(
924+
$pb$::MessageLite::internal_visibility(), GetArena(),
925+
*_internal_mutable_$name_internal$(), ::std::forward<Arg_>(value),
926+
args... $bytes_tag$);
913927
$set_hasbit$;
914928
$annotate_add$;
915929
// @@protoc_insertion_point(field_add:$pkg.Msg.field$)

‎src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -630,12 +630,22 @@ class RepeatedStringView : public FieldGeneratorBase {
630630
}
631631
}
632632

633+
bool RequiresArena(GeneratorFunction function) const override {
634+
switch (function) {
635+
case GeneratorFunction::kMergeFrom:
636+
return true;
637+
}
638+
return false;
639+
}
640+
633641
void GenerateMergingCode(io::Printer* p) const override {
634642
// TODO: experiment with simplifying this to be
635643
// `if (!from.empty()) { body(); }` for both split and non-split cases.
636644
auto body = [&] {
637645
p->Emit(R"cc(
638-
_this->_internal_mutable_$name$()->MergeFrom(from._internal_$name$());
646+
_this->_internal_mutable_$name$()->MergeFromWithArena(
647+
$pb$::MessageLite::internal_visibility(), arena,
648+
from._internal_$name$());
639649
)cc");
640650
};
641651
if (!should_split()) {
@@ -670,7 +680,9 @@ class RepeatedStringView : public FieldGeneratorBase {
670680
if (should_split()) {
671681
p->Emit(R"cc(
672682
if (!from._internal_$name$().empty()) {
673-
_internal_mutable_$name$()->MergeFrom(from._internal_$name$());
683+
_internal_mutable_$name$()->MergeFromWithArena(
684+
$pb$::MessageLite::internal_visibility(), arena,
685+
from._internal_$name$());
674686
}
675687
)cc");
676688
}
@@ -757,7 +769,9 @@ void RepeatedStringView::GenerateInlineAccessorDefinitions(
757769
inline void $Msg$::add_$name$(Arg_&& value) {
758770
$WeakDescriptorSelfPin$;
759771
$TsanDetectConcurrentMutation$;
760-
$pbi$::AddToRepeatedPtrField(*_internal_mutable_$name_internal$(),
772+
$pbi$::AddToRepeatedPtrField($pb$::MessageLite::internal_visibility(),
773+
GetArena(),
774+
*_internal_mutable_$name_internal$(),
761775
::std::forward<Arg_>(value) $bytes_tag$);
762776
$set_hasbit$;
763777
$annotate_add$;

0 commit comments

Comments
 (0)