From 1d7bda4a78e1f9f7001df3378e38827666f6cdd8 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Tue, 14 Mar 2023 18:28:40 -0700 Subject: [PATCH] Add hasbit to proto3 fields even when 'optional' is missing. PiperOrigin-RevId: 516687577 --- src/google/protobuf/compiler/java/message_field.cc | 6 ++++-- src/google/protobuf/descriptor.cc | 14 +------------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/src/google/protobuf/compiler/java/message_field.cc b/src/google/protobuf/compiler/java/message_field.cc index 00fdba80d1..5616c059a3 100644 --- a/src/google/protobuf/compiler/java/message_field.cc +++ b/src/google/protobuf/compiler/java/message_field.cc @@ -322,8 +322,10 @@ void ImmutableMessageFieldGenerator::GenerateBuilderMembers( "$name$Builder_.mergeFrom(value);\n", - "$set_has_field_bit_builder$\n" - "$on_changed$\n" + "if ($name$_ != null) {\n" + " $set_has_field_bit_builder$\n" + " $on_changed$\n" + "}\n" "return this;\n"); // Message.Builder clearField() diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index fd7e914096..25c212e636 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -8426,19 +8426,7 @@ bool HasPreservingUnknownEnumSemantics(const FieldDescriptor* field) { } bool HasHasbit(const FieldDescriptor* field) { - // This predicate includes proto3 message fields only if they have "optional". - // Foo submsg1 = 1; // HasHasbit() == false - // optional Foo submsg2 = 2; // HasHasbit() == true - // This is slightly odd, as adding "optional" to a singular proto3 field does - // not change the semantics or API. However whenever any field in a message - // has a hasbit, it forces reflection to include hasbit offsets for *all* - // fields, even if almost all of them are set to -1 (no hasbit). So to avoid - // causing a sudden size regression for ~all proto3 messages, we give proto3 - // message fields a hasbit only if "optional" is present. If the user is - // explicitly writing "optional", it is likely they are writing it on - // primitive fields also. - return (FieldDescriptorLegacy(field).has_optional_keyword() || - field->is_required()) && + return field->has_presence() && !field->real_containing_oneof() && !field->options().weak(); }