Skip to content

Commit 5823855

Browse files
ClaytonKnittelcopybara-github
authored andcommitted
Support repeated fields with hasbits in message generator.
PiperOrigin-RevId: 792007698
1 parent db04666 commit 5823855

File tree

10 files changed

+568
-278
lines changed

10 files changed

+568
-278
lines changed

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -316,9 +316,13 @@ void HasBitVars(const FieldDescriptor* field, const Options& opts,
316316
: "_impl_._has_bits_";
317317

318318
auto has_bits_array = absl::StrFormat("%s[%d]", has_bits, index);
319-
auto has = absl::StrFormat("CheckHasBit(%s, %s)", has_bits_array, mask);
320-
auto set = absl::StrFormat("SetHasBit(%s, %s);", has_bits_array, mask);
321-
auto clr = absl::StrFormat("ClearHasBit(%s, %s);", has_bits_array, mask);
319+
auto for_repeated = field->is_repeated() ? "ForRepeated" : "";
320+
auto has = absl::StrFormat("CheckHasBit%s(%s, %s)", for_repeated,
321+
has_bits_array, mask);
322+
auto set = absl::StrFormat("SetHasBit%s(%s, %s);", for_repeated,
323+
has_bits_array, mask);
324+
auto clr = absl::StrFormat("ClearHasBit%s(%s, %s);", for_repeated,
325+
has_bits_array, mask);
322326

323327
vars.emplace_back("has_bits_array", has_bits_array);
324328
vars.emplace_back("has_mask", mask);

src/google/protobuf/compiler/cpp/message.cc

Lines changed: 61 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -121,18 +121,21 @@ void DebugAssertUniformLikelyPresence(
121121
// to use.
122122
std::string GenerateConditionMaybeWithProbability(
123123
uint32_t mask, std::optional<float> probability, bool use_cached_has_bits,
124-
std::optional<int> has_array_index, bool is_batch = false) {
124+
std::optional<int> has_array_index, bool is_batch, bool is_repeated) {
125+
ABSL_DCHECK(!is_batch || !is_repeated);
125126
std::string condition;
126127
if (use_cached_has_bits) {
127-
condition = absl::StrFormat("%sCheckHasBit(cached_has_bits, 0x%08xU)",
128-
(is_batch ? "Batch" : ""), mask);
128+
condition = absl::StrFormat("%sCheckHasBit%s(cached_has_bits, 0x%08xU)",
129+
(is_batch ? "Batch" : ""),
130+
(is_repeated ? "ForRepeated" : ""), mask);
129131
} else {
130132
// We only use has_array_index when use_cached_has_bits is false, make sure
131133
// we pas a valid index when we need it.
132134
ABSL_DCHECK(has_array_index.has_value());
133-
condition =
134-
absl::StrFormat("%sCheckHasBit(this_._impl_._has_bits_[%d], 0x%08xU)",
135-
(is_batch ? "Batch" : ""), *has_array_index, mask);
135+
condition = absl::StrFormat(
136+
"%sCheckHasBit%s(this_._impl_._has_bits_[%d], 0x%08xU)",
137+
(is_batch ? "Batch" : ""), (is_repeated ? "ForRepeated" : ""),
138+
*has_array_index, mask);
136139
}
137140
if (probability.has_value()) {
138141
return absl::StrFormat("PROTOBUF_EXPECT_TRUE_WITH_PROBABILITY(%s, %.3f)",
@@ -142,12 +145,13 @@ std::string GenerateConditionMaybeWithProbability(
142145
}
143146

144147
std::string GenerateConditionMaybeWithProbabilityForField(
145-
int has_bit_index, const FieldDescriptor* field, const Options& options) {
148+
int has_bit_index, const FieldDescriptor* field, const Options& options,
149+
bool is_repeated) {
146150
auto prob = GetPresenceProbability(field, options);
147-
return GenerateConditionMaybeWithProbability(
148-
1u << (has_bit_index % 32), prob,
149-
/*use_cached_has_bits*/ true,
150-
/*has_array_index*/ std::nullopt);
151+
return GenerateConditionMaybeWithProbability(1u << (has_bit_index % 32), prob,
152+
/*use_cached_has_bits*/ true,
153+
/*has_array_index*/ std::nullopt,
154+
/*is_batch=*/false, is_repeated);
151155
}
152156

153157
std::string GenerateConditionMaybeWithProbabilityForGroup(
@@ -157,7 +161,8 @@ std::string GenerateConditionMaybeWithProbabilityForGroup(
157161
return GenerateConditionMaybeWithProbability(mask, prob,
158162
/*use_cached_has_bits*/ true,
159163
/*has_array_index*/ std::nullopt,
160-
/*is_batch*/ true);
164+
/*is_batch*/ true,
165+
/*is_repeated*/ false);
161166
}
162167

163168
void PrintPresenceCheck(const FieldDescriptor* field,
@@ -172,8 +177,9 @@ void PrintPresenceCheck(const FieldDescriptor* field,
172177
cached_has_bits = $has_bits$[$index$];
173178
)cc");
174179
}
175-
p->Emit({{"condition", GenerateConditionMaybeWithProbabilityForField(
176-
has_bit_index, field, options)}},
180+
p->Emit({{"condition",
181+
GenerateConditionMaybeWithProbabilityForField(
182+
has_bit_index, field, options, field->is_repeated())}},
177183
R"cc(
178184
if ($condition$) {
179185
)cc");
@@ -686,6 +692,14 @@ MessageGenerator::MessageGenerator(
686692
index_in_file_messages_);
687693
}
688694

695+
bool MessageGenerator::ShouldGenerateEnclosingIf(
696+
const FieldDescriptor& field) const {
697+
return HasBitIndex(&field) != kNoHasbit &&
698+
(field.is_repeated() ||
699+
field.cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE ||
700+
field.cpp_type() == FieldDescriptor::CPPTYPE_STRING);
701+
}
702+
689703
size_t MessageGenerator::HasBitsSize() const {
690704
return (max_has_bit_index_ + 31) / 32;
691705
}
@@ -1236,9 +1250,13 @@ void MessageGenerator::GenerateFieldClear(const FieldDescriptor* field,
12361250
field_generators_.get(field).GenerateClearingCode(p);
12371251
if (HasHasbit(field, options_)) {
12381252
auto v = p->WithVars(HasBitVars(field));
1239-
p->Emit(R"cc(
1240-
ClearHasBit($has_bits$[$has_array_index$], $has_mask$);
1241-
)cc");
1253+
p->Emit({{"clear_has_bit", field->is_repeated()
1254+
? "ClearHasBitForRepeated"
1255+
: "ClearHasBit"}},
1256+
R"cc(
1257+
$clear_has_bit$($has_bits$[$has_array_index$],
1258+
$has_mask$);
1259+
)cc");
12421260
}
12431261
}
12441262
}}},
@@ -1345,8 +1363,9 @@ void MessageGenerator::EmitCheckAndUpdateByteSizeForField(
13451363
}
13461364

13471365
int has_bit_index = has_bit_indices_[field->index()];
1348-
p->Emit({{"condition", GenerateConditionMaybeWithProbabilityForField(
1349-
has_bit_index, field, options_)},
1366+
p->Emit({{"condition",
1367+
GenerateConditionMaybeWithProbabilityForField(
1368+
has_bit_index, field, options_, field->is_repeated())},
13501369
{"check_nondefault_and_emit_body",
13511370
[&] {
13521371
// Note that it's possible that the field has explicit presence.
@@ -3265,8 +3284,9 @@ void MessageGenerator::GenerateCopyInitFields(io::Printer* p) const {
32653284
p->Emit("from.$field$ != nullptr");
32663285
} else {
32673286
int has_bit_index = has_bit_indices_[field->index()];
3268-
p->Emit({{"condition", GenerateConditionMaybeWithProbabilityForField(
3269-
has_bit_index, field, options_)}},
3287+
p->Emit({{"condition",
3288+
GenerateConditionMaybeWithProbabilityForField(
3289+
has_bit_index, field, options_, /*is_repeated=*/false)}},
32703290
"$condition$");
32713291
}
32723292
};
@@ -3630,7 +3650,6 @@ void MessageGenerator::GenerateClear(io::Printer* p) {
36303650
// (memset) per chunk, and if present it will be at the beginning.
36313651
bool same =
36323652
HasByteIndex(a) == HasByteIndex(b) &&
3633-
a->is_repeated() == b->is_repeated() &&
36343653
IsLikelyPresent(a, options_) == IsLikelyPresent(b, options_) &&
36353654
ShouldSplit(a, options_) == ShouldSplit(b, options_) &&
36363655
(CanClearByZeroing(a) == CanClearByZeroing(b) ||
@@ -3727,10 +3746,7 @@ void MessageGenerator::GenerateClear(io::Printer* p) {
37273746
// clear strings and messages if they were set.
37283747
//
37293748
// TODO: Let the CppFieldGenerator decide this somehow.
3730-
bool have_enclosing_if =
3731-
HasBitIndex(field) != kNoHasbit &&
3732-
(field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE ||
3733-
field->cpp_type() == FieldDescriptor::CPPTYPE_STRING);
3749+
const bool have_enclosing_if = ShouldGenerateEnclosingIf(*field);
37343750

37353751
if (have_enclosing_if) {
37363752
PrintPresenceCheck(field, has_bit_indices_, p, &cached_has_word_index,
@@ -4348,10 +4364,7 @@ void MessageGenerator::GenerateClassSpecificMergeImpl(io::Printer* p) {
43484364
for (const auto* field : fields) {
43494365
const auto& generator = field_generators_.get(field);
43504366

4351-
if (field->is_repeated()) {
4352-
generator.GenerateMergingCode(p);
4353-
} else if (!field->is_required() && !field->is_repeated() &&
4354-
!HasHasbit(field, options_)) {
4367+
if (!field->is_required() && !HasHasbit(field, options_)) {
43554368
// Merge semantics without true field presence: primitive fields are
43564369
// merged only if non-zero (numeric) or non-empty (string).
43574370
MayEmitMutableIfNonDefaultCheck(
@@ -4363,8 +4376,12 @@ void MessageGenerator::GenerateClassSpecificMergeImpl(io::Printer* p) {
43634376
// Check hasbit, not using cached bits.
43644377
auto v = p->WithVars(HasBitVars(field));
43654378
p->Emit(
4366-
{{"merge_field", [&] { generator.GenerateMergingCode(p); }}}, R"cc(
4367-
if (CheckHasBit(from.$has_bits$[$has_array_index$], $has_mask$)) {
4379+
{{"check_has_bit",
4380+
field->is_repeated() ? "CheckHasBitForRepeated" : "CheckHasBit"},
4381+
{"merge_field", [&] { generator.GenerateMergingCode(p); }}},
4382+
R"cc(
4383+
if ($check_has_bit$(from.$has_bits$[$has_array_index$],
4384+
$has_mask$)) {
43684385
$merge_field$;
43694386
}
43704387
)cc");
@@ -4374,8 +4391,9 @@ void MessageGenerator::GenerateClassSpecificMergeImpl(io::Printer* p) {
43744391
int has_bit_index = has_bit_indices_[field->index()];
43754392

43764393
p->Emit(
4377-
{{"condition", GenerateConditionMaybeWithProbabilityForField(
4378-
has_bit_index, field, options_)},
4394+
{{"condition",
4395+
GenerateConditionMaybeWithProbabilityForField(
4396+
has_bit_index, field, options_, field->is_repeated())},
43794397
{"merge_field",
43804398
[&] {
43814399
if (GetFieldHasbitMode(field, options_) ==
@@ -4719,10 +4737,12 @@ void MessageGenerator::GenerateSerializeOneField(io::Printer* p,
47194737
std::move(emit_body),
47204738
/*with_enclosing_braces_always=*/false);
47214739
}},
4722-
{"cond", GenerateConditionMaybeWithProbability(
4723-
1u << (has_bit_index % 32),
4724-
GetPresenceProbability(field, options_),
4725-
use_cached_has_bits, has_word_index)},
4740+
{"cond",
4741+
GenerateConditionMaybeWithProbability(
4742+
1u << (has_bit_index % 32),
4743+
GetPresenceProbability(field, options_), use_cached_has_bits,
4744+
has_word_index, /*is_batch=*/false,
4745+
/*is_repeated=*/field->is_repeated())},
47264746
},
47274747
R"cc(
47284748
if ($cond$) {
@@ -5245,7 +5265,6 @@ void MessageGenerator::GenerateByteSize(io::Printer* p) {
52455265
std::vector<FieldChunk> chunks =
52465266
CollectFields(rest, options_, [&](const auto* a, const auto* b) {
52475267
return a->is_required() == b->is_required() &&
5248-
a->is_repeated() == b->is_repeated() &&
52495268
HasByteIndex(a) == HasByteIndex(b) &&
52505269
IsLikelyPresent(a, options_) == IsLikelyPresent(b, options_) &&
52515270
ShouldSplit(a, options_) == ShouldSplit(b, options_);
@@ -5560,8 +5579,9 @@ void MessageGenerator::EmitCheckAndSerializeField(const FieldDescriptor* field,
55605579
}
55615580

55625581
int has_bit_index = has_bit_indices_[field->index()];
5563-
p->Emit({{"condition", GenerateConditionMaybeWithProbabilityForField(
5564-
has_bit_index, field, options_)},
5582+
p->Emit({{"condition",
5583+
GenerateConditionMaybeWithProbabilityForField(
5584+
has_bit_index, field, options_, field->is_repeated())},
55655585
{"check_nondefault_and_emit_body",
55665586
[&] {
55675587
// Note that it's possible that the field has explicit presence.

src/google/protobuf/compiler/cpp/message.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,8 @@ class MessageGenerator {
197197
// at construction.
198198
ArenaDtorNeeds NeedsArenaDestructor() const;
199199

200+
bool ShouldGenerateEnclosingIf(const FieldDescriptor& field) const;
201+
200202
size_t HasBitsSize() const;
201203
size_t InlinedStringDonatedSize() const;
202204
absl::flat_hash_map<absl::string_view, std::string> HasBitVars(

src/google/protobuf/compiler/java/java_features.pb.h

Lines changed: 10 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/google/protobuf/compiler/plugin.pb.cc

Lines changed: 17 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)