Chiebot-Mirror
/
protobuf
mirror of https://github.com/protocolbuffers/protobuf.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Reduced nesting in GenerateByteSize: slight readability improvements in generated code.

Browse Source 
GenerateByteSize itself remains deeply nested, but by factoring out one part of
the loop, at least we make the part that generates UpdateByteSize a bit more
readable.

Making the callsite of MayEmitIfNonDefaultCheck less nested actually resulted
in slight readability improvements also in the generated code, namely of the
form:

@@ -10563,8 +10559,7 @@ PROTOBUF_NOINLINE void OneStringEdition:
            {
             // string data = 1;
-            cached_has_bits =
-                this_._impl_._has_bits_[0];
+            cached_has_bits = this_._impl_._has_bits_[0];
             if (cached_has_bits & 0x00000001u) {
	       total_size += 1 + ::proto2::internal::WireFormatLite::StringSize(
                                               this_._internal_data());

These readability improvements should be kept IMO -- they make the generated
protobuf C++ code slightly easier to read.

PiperOrigin-RevId: 655180880
pull/17537/head
Tony Liao 7 months ago committed by Copybara-Service
parent a0387c1923
commit 162a740670
5 changed files with 59 additions and 61 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 4
      editions/golden/compare_cpp_codegen_failure.txt
  2. 2
      editions/golden/compare_cpp_codegen_failure.xml
  3. 107
      src/google/protobuf/compiler/cpp/message.cc
  4. 4
      src/google/protobuf/compiler/cpp/message.h
  5. 3
      src/google/protobuf/descriptor.pb.cc

4
editions/golden/compare_cpp_codegen_failure.txt
Unescape View File

@ -30,9 +30,9 @@
{
- // optional int32 int32_field = 1;
+ // int32 int32_field = 1;
cached_has_bits =
this_._impl_._has_bits_[0];
cached_has_bits = this_._impl_._has_bits_[0];
if (cached_has_bits & 0x00000001u) {
total_size += ::_pbi::WireFormatLite::Int32SizePlusOne(
[ FAILED ] third_party/protobuf/editions/golden/simple_proto3.pb.cc
[ RUN ] third_party/protobuf/editions/golden/simple_proto3.pb.h
@@ @@

2
editions/golden/compare_cpp_codegen_failure.xml
Unescape View File

@ -2,7 +2,7 @@
<testsuites tests="1" name="AllTests">
<testsuite name="EditionsCodegenTests">
<testcase name="third_party/protobuf/editions/golden/simple_proto3.pb.cc" status="run" result="completed" classname="DiffTest">
<failure message="Value of: third_party/protobuf/editions/golden/simple_proto3.pb.cc&#x0A;Expected: &#x0A;// Generated by the protocol buffer compiler. DO NOT EDIT!&#x0A;// NO CHECKED-IN PROTOBUF GENCODE&#x0A;// source: third_party/protobuf/editions/golden/simple_proto3.proto&#x0A;// Protobuf C++ Version: 0.20240718.0&#x0A;&#x0A;#include &quot;third_party/protobuf/editions/golden/simple_proto3.pb.h&quot;&#x0A;&#x0A;#include &lt;algorithm&gt;&#x0A;#include &lt;type_traits&gt;&#x0A;#include &quot;third_party/protobuf/io/coded_stream.h&quot;&#x0A;#include &quot;third_party/protobuf/generated_message_tctable_impl.h&quot;&#x0A;#include &quot;third_party/protobuf/extension_set.h&quot;&#x0A;#include &quot;third_party/protobuf/wire_format_lite.h&quot;&#x0A;#include &quot;third_party/protobuf/io/zero_copy_stream_impl_lite.h&quot;&#x0A;// @@protoc_insertion_point(includes)&#x0A;&#x0A;// Must be included last.&#x0A;, with the difference:&#x0A;@@ @@&#x0A; ::_pbi::TcParser::GetTable&lt;::protobuf_editions_test::golden::SimpleProto3&gt;(), // to_prefetch&#x0A; #endif // PROTOBUF_PREFETCH_PARSE_TABLE&#x0A; }, {{&#x0A;- // optional int32 int32_field = 1;&#x0A;+ // int32 int32_field = 1;&#x0A; {::_pbi::TcParser::FastV32S1,&#x0A; {8, 0, 0, PROTOBUF_FIELD_OFFSET(SimpleProto3, _impl_.int32_field_)}},&#x0A; }}, {{&#x0A; 65535, 65535&#x0A; }}, {{&#x0A;- // optional int32 int32_field = 1;&#x0A;+ // int32 int32_field = 1;&#x0A; {PROTOBUF_FIELD_OFFSET(SimpleProto3, _impl_.int32_field_), _Internal::kHasBitsOffset + 0, 0,&#x0A; (0 | ::_fl::kFcOptional | ::_fl::kInt32)},&#x0A; }},&#x0A;@@ @@&#x0A; (void)cached_has_bits;&#x0A; &#x0A; cached_has_bits = this_._impl_._has_bits_[0];&#x0A;- // optional int32 int32_field = 1;&#x0A;+ // int32 int32_field = 1;&#x0A; if (cached_has_bits &amp; 0x00000001u) {&#x0A; target = ::proto2::internal::WireFormatLite::&#x0A; WriteInt32ToArrayWithField&lt;1&gt;(&#x0A;@@ @@&#x0A; (void)cached_has_bits;&#x0A; &#x0A; {&#x0A;- // optional int32 int32_field = 1;&#x0A;+ // int32 int32_field = 1;&#x0A; cached_has_bits =&#x0A; this_._impl_._has_bits_[0];&#x0A; if (cached_has_bits &amp; 0x00000001u) {" type=""></failure>
<failure message="Value of: third_party/protobuf/editions/golden/simple_proto3.pb.cc&#x0A;Expected: &#x0A;// Generated by the protocol buffer compiler. DO NOT EDIT!&#x0A;// NO CHECKED-IN PROTOBUF GENCODE&#x0A;// source: third_party/protobuf/editions/golden/simple_proto3.proto&#x0A;// Protobuf C++ Version: 0.20240718.0&#x0A;&#x0A;#include &quot;third_party/protobuf/editions/golden/simple_proto3.pb.h&quot;&#x0A;&#x0A;#include &lt;algorithm&gt;&#x0A;#include &lt;type_traits&gt;&#x0A;#include &quot;third_party/protobuf/io/coded_stream.h&quot;&#x0A;#include &quot;third_party/protobuf/generated_message_tctable_impl.h&quot;&#x0A;#include &quot;third_party/protobuf/extension_set.h&quot;&#x0A;#include &quot;third_party/protobuf/wire_format_lite.h&quot;&#x0A;#include &quot;third_party/protobuf/io/zero_copy_stream_impl_lite.h&quot;&#x0A;// @@protoc_insertion_point(includes)&#x0A;&#x0A;// Must be included last.&#x0A;, with the difference:&#x0A;@@ @@&#x0A; ::_pbi::TcParser::GetTable&lt;::protobuf_editions_test::golden::SimpleProto3&gt;(), // to_prefetch&#x0A; #endif // PROTOBUF_PREFETCH_PARSE_TABLE&#x0A; }, {{&#x0A;- // optional int32 int32_field = 1;&#x0A;+ // int32 int32_field = 1;&#x0A; {::_pbi::TcParser::FastV32S1,&#x0A; {8, 0, 0, PROTOBUF_FIELD_OFFSET(SimpleProto3, _impl_.int32_field_)}},&#x0A; }}, {{&#x0A; 65535, 65535&#x0A; }}, {{&#x0A;- // optional int32 int32_field = 1;&#x0A;+ // int32 int32_field = 1;&#x0A; {PROTOBUF_FIELD_OFFSET(SimpleProto3, _impl_.int32_field_), _Internal::kHasBitsOffset + 0, 0,&#x0A; (0 | ::_fl::kFcOptional | ::_fl::kInt32)},&#x0A; }},&#x0A;@@ @@&#x0A; (void)cached_has_bits;&#x0A; &#x0A; cached_has_bits = this_._impl_._has_bits_[0];&#x0A;- // optional int32 int32_field = 1;&#x0A;+ // int32 int32_field = 1;&#x0A; if (cached_has_bits &amp; 0x00000001u) {&#x0A; target = ::proto2::internal::WireFormatLite::&#x0A; WriteInt32ToArrayWithField&lt;1&gt;(&#x0A;@@ @@&#x0A; (void)cached_has_bits;&#x0A; &#x0A; {&#x0A;- // optional int32 int32_field = 1;&#x0A;+ // int32 int32_field = 1;&#x0A; cached_has_bits = this_._impl_._has_bits_[0];&#x0A; if (cached_has_bits &amp; 0x00000001u) {&#x0A; total_size += ::_pbi::WireFormatLite::Int32SizePlusOne(" type=""></failure>
</testcase>
<testcase name="third_party/protobuf/editions/golden/simple_proto3.pb.h" status="run" result="completed" classname="DiffTest">
<failure message="Value of: third_party/protobuf/editions/golden/simple_proto3.pb.h&#x0A;Expected: &#x0A;// Generated by the protocol buffer compiler. DO NOT EDIT!&#x0A;// NO CHECKED-IN PROTOBUF GENCODE&#x0A;// source: third_party/protobuf/editions/golden/simple_proto3.proto&#x0A;// Protobuf C++ Version: 0.20240718.0&#x0A;&#x0A;#ifndef GOOGLE_PROTOBUF_INCLUDED_third_5fparty_2fprotobuf_2feditions_2fgolden_2fsimple_5fproto3_2eproto_2epb_2eh&#x0A;#define GOOGLE_PROTOBUF_INCLUDED_third_5fparty_2fprotobuf_2feditions_2fgolden_2fsimple_5fproto3_2eproto_2epb_2eh&#x0A;&#x0A;#include &lt;limits&gt;&#x0A;#include &lt;string&gt;&#x0A;#include &lt;type_traits&gt;&#x0A;#include &lt;utility&gt;&#x0A;&#x0A;#include &quot;third_party/protobuf/runtime_version.h&quot;&#x0A;#if PROTOBUF_VERSION != 20240718&#x0A;#error &quot;Protobuf C++ gencode is built with an incompatible version of&quot;&#x0A;#error &quot;Protobuf C++ headers/runtime. See&quot;&#x0A;#error &quot;https://protobuf.dev/support/cross-version-runtime-guarantee/#cpp&quot;&#x0A;#endif&#x0A;#include &quot;third_party/protobuf/io/coded_stream.h&quot;&#x0A;#include &quot;third_party/protobuf/arena.h&quot;&#x0A;#include &quot;third_party/protobuf/arenastring.h&quot;&#x0A;#include &quot;third_party/protobuf/generated_message_tctable_decl.h&quot;&#x0A;#include &quot;third_party/protobuf/generated_message_util.h&quot;&#x0A;#include &quot;third_party/protobuf/metadata_lite.h&quot;&#x0A;#include &quot;third_party/protobuf/message_lite.h&quot;&#x0A;// @@protoc_insertion_point(includes)&#x0A;&#x0A;// Must be included last.&#x0A;&#x0A;#endif // GOOGLE_PROTOBUF_INCLUDED_third_5fparty_2fprotobuf_2feditions_2fgolden_2fsimple_5fproto3_2eproto_2epb_2eh&#x0A;, with the difference:&#x0A;@@ @@&#x0A; enum : int {&#x0A; kInt32FieldFieldNumber = 1,&#x0A; };&#x0A;- // optional int32 int32_field = 1;&#x0A;+ // int32 int32_field = 1;&#x0A; bool has_int32_field() const;&#x0A; void clear_int32_field() ;&#x0A; ::int32_t int32_field() const;&#x0A;@@ @@&#x0A; &#x0A; // SimpleProto3&#x0A; &#x0A;-// optional int32 int32_field = 1;&#x0A;+// int32 int32_field = 1;&#x0A; inline bool SimpleProto3::has_int32_field() const {&#x0A; bool value = (_impl_._has_bits_[0] &amp; 0x00000001u) != 0;&#x0A; return value;" type=""></failure>

107
src/google/protobuf/compiler/cpp/message.cc
Unescape View File

@ -1213,6 +1213,55 @@ class AccessorVerifier {
} // namespace
void MessageGenerator::EmitUpdateByteSizeForField(
const FieldDescriptor* field, io::Printer* p,
int& cached_has_word_index) const {
p->Emit(
{{"comment", [&] { PrintFieldComment(Formatter{p}, field, options_); }},
{"update_byte_size_for_field",
[&] { field_generators_.get(field).GenerateByteSize(p); }},
{"update_cached_has_bits",
[&] {
if (!HasHasbit(field) || field->options().weak()) return;
int has_bit_index = has_bit_indices_[field->index()];
if (cached_has_word_index == (has_bit_index / 32)) return;
cached_has_word_index = (has_bit_index / 32);
p->Emit({{"index", cached_has_word_index}},
R"cc(
cached_has_bits = this_.$has_bits$[$index$];
)cc");
}},
{"check_if_field_present",
[&] {
if (!HasHasbit(field)) {
MayEmitIfNonDefaultCheck(p, "this_.", field);
return;
}
if (field->options().weak()) {
p->Emit("if (has_$name$())");
return;
}
int has_bit_index = has_bit_indices_[field->index()];
p->Emit(
{{"mask", absl::StrFormat("0x%08xu",
uint32_t{1} << (has_bit_index % 32))}},
"if (cached_has_bits & $mask$)");
}}},
R"cc(
$comment$;
$update_cached_has_bits$;
$check_if_field_present$ {
//~ Force newline.
$update_byte_size_for_field$;
}
)cc");
}
void MessageGenerator::GenerateFieldAccessorDefinitions(io::Printer* p) {
p->Emit("// $classname$\n\n");
@ -4748,62 +4797,8 @@ void MessageGenerator::GenerateByteSize(io::Printer* p) {
// Go back and emit checks for each of the fields we
// processed.
for (const auto* field : fields) {
p->Emit(
{{"comment",
[&] {
PrintFieldComment(Formatter{p}, field,
options_);
}},
{"update_byte_size_for_field",
[&] {
field_generators_.get(field).GenerateByteSize(
p);
}},
{"update_cached_has_bits",
[&] {
if (!HasHasbit(field) ||
field->options().weak())
return;
int has_bit_index =
has_bit_indices_[field->index()];
if (cached_has_word_index ==
(has_bit_index / 32))
return;
cached_has_word_index = (has_bit_index / 32);
p->Emit({{"index", cached_has_word_index}},
R"cc(
cached_has_bits =
this_.$has_bits$[$index$];
)cc");
}},
{"check_if_field_present",
[&] {
if (!HasHasbit(field)) {
MayEmitIfNonDefaultCheck(p, "this_.", field);
return;
}
if (field->options().weak()) {
p->Emit("if (has_$name$())");
return;
}
int has_bit_index =
has_bit_indices_[field->index()];
p->Emit(
{{"mask", absl::StrFormat(
"0x%08xu",
1u << (has_bit_index % 32))}},
"if (cached_has_bits & $mask$)");
}}},
R"cc(
$comment$;
$update_cached_has_bits$;
$check_if_field_present$ {
//~ Force newline.
$update_byte_size_for_field$;
}
)cc");
EmitUpdateByteSizeForField(field, p,
cached_has_word_index);
}
}},
{"may_update_cached_has_word_index",

4
src/google/protobuf/compiler/cpp/message.h
Unescape View File

@ -186,6 +186,10 @@ class MessageGenerator {
int HasWordIndex(const FieldDescriptor* field) const;
std::vector<uint32_t> RequiredFieldsBitMask() const;
// Helper function to reduce nesting levels of deep Emit calls.
void EmitUpdateByteSizeForField(const FieldDescriptor* field, io::Printer* p,
int& cached_has_word_index) const;
const Descriptor* descriptor_;
int index_in_file_messages_;
Options options_;

3
src/google/protobuf/descriptor.pb.cc
Unescape View File

@ -10530,8 +10530,7 @@ PROTOBUF_NOINLINE void OneofOptions::Clear() {
}
{
// optional .google.protobuf.FeatureSet features = 1;
cached_has_bits =
this_._impl_._has_bits_[0];
cached_has_bits = this_._impl_._has_bits_[0];
if (cached_has_bits & 0x00000001u) {
total_size += 1 +
::google::protobuf::internal::WireFormatLite::MessageSize(*this_._impl_.features_);

Write Preview
Loading…
Cancel
Save