From 2326aef1a454a4eea363cc6ed8b8def8b88365f5 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Thu, 3 Nov 2022 13:07:56 -0700 Subject: [PATCH] Use bit-field int values in buildPartial to skip work on unset groups of fields. Changes to make this improvement: 1) All non-repeated builder fields (including maps) now have a presence bit regardless of syntax. 2) The buildPartial method is split into one method per 32-field block (aligned with bit-mask ints) 3) If a presence bit-mask int is set to 0, no fields are present, so we can skip the logic for all of those fields in the buildPartial step. For messages with a lot of fields (> 100) that are sparsely populated this can result in a significant improvement. Not only does it potentially skip a lot of field copying logic, but also breaks the buildPartial method into chunks that should be more easily digested by the JIT compiler as discussed in this issue: https://github.com/protocolbuffers/protobuf/issues/10247. PiperOrigin-RevId: 485952448 --- .../protobuf/compiler/java/enum_field.cc | 73 ++--- .../protobuf/compiler/java/enum_field.h | 11 +- src/google/protobuf/compiler/java/field.h | 2 + .../protobuf/compiler/java/map_field.cc | 249 ++++++++++------- src/google/protobuf/compiler/java/map_field.h | 4 + .../protobuf/compiler/java/message_builder.cc | 227 ++++++++++----- .../protobuf/compiler/java/message_builder.h | 5 + .../protobuf/compiler/java/message_field.cc | 262 ++++++++---------- .../protobuf/compiler/java/message_field.h | 35 +-- .../protobuf/compiler/java/primitive_field.cc | 103 +++---- .../protobuf/compiler/java/primitive_field.h | 11 +- .../protobuf/compiler/java/string_field.cc | 97 +++---- .../protobuf/compiler/java/string_field.h | 12 +- 13 files changed, 595 insertions(+), 496 deletions(-) diff --git a/src/google/protobuf/compiler/java/enum_field.cc b/src/google/protobuf/compiler/java/enum_field.cc index 022511ff71e6..f52174365b7f 100644 --- a/src/google/protobuf/compiler/java/enum_field.cc +++ b/src/google/protobuf/compiler/java/enum_field.cc @@ -91,22 +91,15 @@ void SetEnumVariables( if (HasHasbit(descriptor)) { // For singular messages and builders, one bit is used for the hasField bit. (*variables)["get_has_field_bit_message"] = GenerateGetBit(messageBitIndex); - (*variables)["get_has_field_bit_builder"] = GenerateGetBit(builderBitIndex); - // Note that these have a trailing ";". (*variables)["set_has_field_bit_message"] = GenerateSetBit(messageBitIndex) + ";"; - (*variables)["set_has_field_bit_builder"] = - GenerateSetBit(builderBitIndex) + ";"; - (*variables)["clear_has_field_bit_builder"] = - GenerateClearBit(builderBitIndex) + ";"; - + (*variables)["set_has_field_bit_to_local"] = + GenerateSetBitToLocal(messageBitIndex); (*variables)["is_field_present_message"] = GenerateGetBit(messageBitIndex); } else { (*variables)["set_has_field_bit_message"] = ""; - (*variables)["set_has_field_bit_builder"] = ""; - (*variables)["clear_has_field_bit_builder"] = ""; - + (*variables)["set_has_field_bit_to_local"] = ""; variables->insert({"is_field_present_message", absl::StrCat((*variables)["name"], "_ != ", (*variables)["default"], ".getNumber()")}); @@ -117,10 +110,15 @@ void SetEnumVariables( (*variables)["set_mutable_bit_builder"] = GenerateSetBit(builderBitIndex); (*variables)["clear_mutable_bit_builder"] = GenerateClearBit(builderBitIndex); + (*variables)["get_has_field_bit_builder"] = GenerateGetBit(builderBitIndex); + + // Note that these have a trailing ";". + (*variables)["set_has_field_bit_builder"] = + GenerateSetBit(builderBitIndex) + ";"; + (*variables)["clear_has_field_bit_builder"] = + GenerateClearBit(builderBitIndex) + ";"; (*variables)["get_has_field_bit_from_local"] = GenerateGetBitFromLocal(builderBitIndex); - (*variables)["set_has_field_bit_to_local"] = - GenerateSetBitToLocal(messageBitIndex); if (SupportUnknownEnumValue(descriptor->file())) { variables->insert( @@ -137,7 +135,10 @@ void SetEnumVariables( ImmutableEnumFieldGenerator::ImmutableEnumFieldGenerator( const FieldDescriptor* descriptor, int messageBitIndex, int builderBitIndex, Context* context) - : descriptor_(descriptor), name_resolver_(context->GetNameResolver()) { + : descriptor_(descriptor), + message_bit_index_(messageBitIndex), + builder_bit_index_(builderBitIndex), + name_resolver_(context->GetNameResolver()) { SetEnumVariables(descriptor, messageBitIndex, builderBitIndex, context->GetFieldGeneratorInfo(descriptor), name_resolver_, &variables_, context); @@ -145,13 +146,19 @@ ImmutableEnumFieldGenerator::ImmutableEnumFieldGenerator( ImmutableEnumFieldGenerator::~ImmutableEnumFieldGenerator() {} +int ImmutableEnumFieldGenerator::GetMessageBitIndex() const { + return message_bit_index_; +} + +int ImmutableEnumFieldGenerator::GetBuilderBitIndex() const { + return builder_bit_index_; +} + int ImmutableEnumFieldGenerator::GetNumBitsForMessage() const { return HasHasbit(descriptor_) ? 1 : 0; } -int ImmutableEnumFieldGenerator::GetNumBitsForBuilder() const { - return GetNumBitsForMessage(); -} +int ImmutableEnumFieldGenerator::GetNumBitsForBuilder() const { return 1; } void ImmutableEnumFieldGenerator::GenerateInterfaceMembers( io::Printer* printer) const { @@ -170,7 +177,7 @@ void ImmutableEnumFieldGenerator::GenerateInterfaceMembers( } void ImmutableEnumFieldGenerator::GenerateMembers(io::Printer* printer) const { - printer->Print(variables_, "private int $name$_;\n"); + printer->Print(variables_, "private int $name$_ = $default_number$;\n"); PrintExtraFieldInfo(variables_, printer); if (HasHazzer(descriptor_)) { WriteFieldAccessorDocComment(printer, descriptor_, HAZZER); @@ -225,8 +232,8 @@ void ImmutableEnumFieldGenerator::GenerateBuilderMembers( printer->Print(variables_, "$deprecation$public Builder " "${$set$capitalized_name$Value$}$(int value) {\n" - " $set_has_field_bit_builder$\n" " $name$_ = value;\n" + " $set_has_field_bit_builder$\n" " onChanged();\n" " return this;\n" "}\n"); @@ -321,9 +328,7 @@ void ImmutableEnumFieldGenerator::GenerateInitializationCode( void ImmutableEnumFieldGenerator::GenerateBuilderClearCode( io::Printer* printer) const { - printer->Print(variables_, - "$name$_ = $default_number$;\n" - "$clear_has_field_bit_builder$\n"); + printer->Print(variables_, "$name$_ = $default_number$;\n"); } void ImmutableEnumFieldGenerator::GenerateMergingCode( @@ -346,13 +351,13 @@ void ImmutableEnumFieldGenerator::GenerateMergingCode( void ImmutableEnumFieldGenerator::GenerateBuildingCode( io::Printer* printer) const { - if (HasHazzer(descriptor_)) { - printer->Print(variables_, - "if ($get_has_field_bit_from_local$) {\n" - " $set_has_field_bit_to_local$;\n" - "}\n"); + printer->Print(variables_, + "if ($get_has_field_bit_from_local$) {\n" + " result.$name$_ = $name$_;\n"); + if (GetNumBitsForMessage() > 0) { + printer->Print(variables_, " $set_has_field_bit_to_local$;\n"); } - printer->Print(variables_, "result.$name$_ = $name$_;\n"); + printer->Print("}\n"); } void ImmutableEnumFieldGenerator::GenerateBuilderParsingCode( @@ -504,6 +509,7 @@ void ImmutableEnumOneofFieldGenerator::GenerateBuilderMembers( " return $default$;\n" "}\n"); printer->Annotate("{", "}", descriptor_); + WriteFieldAccessorDocComment(printer, descriptor_, SETTER, /* builder */ true); printer->Print(variables_, @@ -518,6 +524,7 @@ void ImmutableEnumOneofFieldGenerator::GenerateBuilderMembers( " return this;\n" "}\n"); printer->Annotate("{", "}", descriptor_); + WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, /* builder */ true); printer->Print( @@ -540,10 +547,7 @@ void ImmutableEnumOneofFieldGenerator::GenerateBuilderClearCode( void ImmutableEnumOneofFieldGenerator::GenerateBuildingCode( io::Printer* printer) const { - printer->Print(variables_, - "if ($has_oneof_case_message$) {\n" - " result.$oneof_name$_ = $oneof_name$_;\n" - "}\n"); + // No-Op: Handled by single statement for the oneof } void ImmutableEnumOneofFieldGenerator::GenerateMergingCode( @@ -632,11 +636,8 @@ void ImmutableEnumOneofFieldGenerator::GenerateHashCode( RepeatedImmutableEnumFieldGenerator::RepeatedImmutableEnumFieldGenerator( const FieldDescriptor* descriptor, int messageBitIndex, int builderBitIndex, Context* context) - : descriptor_(descriptor), name_resolver_(context->GetNameResolver()) { - SetEnumVariables(descriptor, messageBitIndex, builderBitIndex, - context->GetFieldGeneratorInfo(descriptor), name_resolver_, - &variables_, context); -} + : ImmutableEnumFieldGenerator(descriptor, messageBitIndex, builderBitIndex, + context) {} RepeatedImmutableEnumFieldGenerator::~RepeatedImmutableEnumFieldGenerator() {} diff --git a/src/google/protobuf/compiler/java/enum_field.h b/src/google/protobuf/compiler/java/enum_field.h index 4371f6768793..ed3d08a506d7 100644 --- a/src/google/protobuf/compiler/java/enum_field.h +++ b/src/google/protobuf/compiler/java/enum_field.h @@ -68,6 +68,8 @@ class ImmutableEnumFieldGenerator : public ImmutableFieldGenerator { // implements ImmutableFieldGenerator // --------------------------------------- + int GetMessageBitIndex() const override; + int GetBuilderBitIndex() const override; int GetNumBitsForMessage() const override; int GetNumBitsForBuilder() const override; void GenerateInterfaceMembers(io::Printer* printer) const override; @@ -90,6 +92,8 @@ class ImmutableEnumFieldGenerator : public ImmutableFieldGenerator { protected: const FieldDescriptor* descriptor_; + int message_bit_index_; + int builder_bit_index_; absl::flat_hash_map variables_; ClassNameResolver* name_resolver_; }; @@ -117,7 +121,7 @@ class ImmutableEnumOneofFieldGenerator : public ImmutableEnumFieldGenerator { void GenerateHashCode(io::Printer* printer) const override; }; -class RepeatedImmutableEnumFieldGenerator : public ImmutableFieldGenerator { +class RepeatedImmutableEnumFieldGenerator : public ImmutableEnumFieldGenerator { public: explicit RepeatedImmutableEnumFieldGenerator( const FieldDescriptor* descriptor, int messageBitIndex, @@ -150,11 +154,6 @@ class RepeatedImmutableEnumFieldGenerator : public ImmutableFieldGenerator { void GenerateKotlinDslMembers(io::Printer* printer) const override; std::string GetBoxedType() const override; - - private: - const FieldDescriptor* descriptor_; - absl::flat_hash_map variables_; - ClassNameResolver* name_resolver_; }; } // namespace java diff --git a/src/google/protobuf/compiler/java/field.h b/src/google/protobuf/compiler/java/field.h index 028e554775dc..c0c44e0eb4cb 100644 --- a/src/google/protobuf/compiler/java/field.h +++ b/src/google/protobuf/compiler/java/field.h @@ -71,6 +71,8 @@ class ImmutableFieldGenerator { ImmutableFieldGenerator& operator=(const ImmutableFieldGenerator&) = delete; virtual ~ImmutableFieldGenerator(); + virtual int GetMessageBitIndex() const = 0; + virtual int GetBuilderBitIndex() const = 0; virtual int GetNumBitsForMessage() const = 0; virtual int GetNumBitsForBuilder() const = 0; virtual void GenerateInterfaceMembers(io::Printer* printer) const = 0; diff --git a/src/google/protobuf/compiler/java/map_field.cc b/src/google/protobuf/compiler/java/map_field.cc index 4c7715c69ec0..1f27a2743cc2 100644 --- a/src/google/protobuf/compiler/java/map_field.cc +++ b/src/google/protobuf/compiler/java/map_field.cc @@ -127,9 +127,8 @@ void SetMessageVariables( : ""; (*variables)["value_null_check"] = valueJavaType != JAVATYPE_ENUM && IsReferenceType(valueJavaType) - ? "if (value == null) {\n" - " throw new NullPointerException(\"map value\");\n" - "}\n" + ? "if (value == null) { " + "throw new NullPointerException(\"map value\"); }" : ""; if (valueJavaType == JAVATYPE_ENUM) { // We store enums as Integers internally. @@ -195,6 +194,14 @@ void SetMessageVariables( name_resolver->GetImmutableClassName(descriptor->file()) + ".internal_" + UniqueFileScopeIdentifier(descriptor->message_type()) + "_descriptor, "; (*variables)["ver"] = GeneratedCodeVersionSuffix(); + + (*variables)["get_has_field_bit_builder"] = GenerateGetBit(builderBitIndex); + (*variables)["get_has_field_bit_from_local"] = + GenerateGetBitFromLocal(builderBitIndex); + (*variables)["set_has_field_bit_builder"] = + GenerateSetBit(builderBitIndex) + ";"; + (*variables)["clear_has_field_bit_builder"] = + GenerateClearBit(builderBitIndex) + ";"; } } // namespace @@ -203,6 +210,8 @@ ImmutableMapFieldGenerator::ImmutableMapFieldGenerator( const FieldDescriptor* descriptor, int messageBitIndex, int builderBitIndex, Context* context) : descriptor_(descriptor), + message_bit_index_(messageBitIndex), + builder_bit_index_(builderBitIndex), name_resolver_(context->GetNameResolver()), context_(context) { SetMessageVariables(descriptor, messageBitIndex, builderBitIndex, @@ -212,6 +221,14 @@ ImmutableMapFieldGenerator::ImmutableMapFieldGenerator( ImmutableMapFieldGenerator::~ImmutableMapFieldGenerator() {} +int ImmutableMapFieldGenerator::GetMessageBitIndex() const { + return message_bit_index_; +} + +int ImmutableMapFieldGenerator::GetBuilderBitIndex() const { + return builder_bit_index_; +} + int ImmutableMapFieldGenerator::GetNumBitsForMessage() const { return 0; } int ImmutableMapFieldGenerator::GetNumBitsForBuilder() const { return 1; } @@ -275,17 +292,16 @@ void ImmutableMapFieldGenerator::GenerateInterfaceMembers( printer->Annotate("{", "}", descriptor_); WriteFieldDocComment(printer, descriptor_); printer->Print(variables_, - "$deprecation$\n" - "$value_type_pass_through_nullness$ " + "$deprecation$$value_type_pass_through_nullness$ " "${$get$capitalized_name$ValueOrDefault$}$(\n" " $key_type$ key,\n" " $value_type_pass_through_nullness$ defaultValue);\n"); printer->Annotate("{", "}", descriptor_); WriteFieldDocComment(printer, descriptor_); - printer->Print(variables_, - "$deprecation$\n" - "$value_type$ ${$get$capitalized_name$ValueOrThrow$}$(\n" - " $key_type$ key);\n"); + printer->Print( + variables_, + "$deprecation$$value_type$ ${$get$capitalized_name$ValueOrThrow$}$(\n" + " $key_type$ key);\n"); printer->Annotate("{", "}", descriptor_); } } else { @@ -306,17 +322,16 @@ void ImmutableMapFieldGenerator::GenerateInterfaceMembers( printer->Annotate("{", "}", descriptor_); WriteFieldDocComment(printer, descriptor_); printer->Print(variables_, - "$deprecation$\n" - "$value_type_pass_through_nullness$ " + "$deprecation$$value_type_pass_through_nullness$ " "${$get$capitalized_name$OrDefault$}$(\n" " $key_type$ key,\n" " $value_type_pass_through_nullness$ defaultValue);\n"); printer->Annotate("{", "}", descriptor_); WriteFieldDocComment(printer, descriptor_); - printer->Print(variables_, - "$deprecation$\n" - "$value_type$ ${$get$capitalized_name$OrThrow$}$(\n" - " $key_type$ key);\n"); + printer->Print( + variables_, + "$deprecation$$value_type$ ${$get$capitalized_name$OrThrow$}$(\n" + " $key_type$ key);\n"); printer->Annotate("{", "}", descriptor_); } } @@ -372,42 +387,45 @@ void ImmutableMapFieldGenerator::GenerateMembers(io::Printer* printer) const { void ImmutableMapFieldGenerator::GenerateBuilderMembers( io::Printer* printer) const { - printer->Print(variables_, - "private com.google.protobuf.MapField<\n" - " $type_parameters$> $name$_;\n" - "private com.google.protobuf.MapField<$type_parameters$>\n" - "internalGet$capitalized_name$() {\n" - " if ($name$_ == null) {\n" - " return com.google.protobuf.MapField.emptyMapField(\n" - " $map_field_parameter$);\n" - " }\n" - " return $name$_;\n" - "}\n" - "private com.google.protobuf.MapField<$type_parameters$>\n" - "internalGetMutable$capitalized_name$() {\n" - " $on_changed$;\n" - " if ($name$_ == null) {\n" - " $name$_ = com.google.protobuf.MapField.newMapField(\n" - " $map_field_parameter$);\n" - " }\n" - " if (!$name$_.isMutable()) {\n" - " $name$_ = $name$_.copy();\n" - " }\n" - " return $name$_;\n" - "}\n"); + printer->Print( + variables_, + "private com.google.protobuf.MapField<\n" + " $type_parameters$> $name$_;\n" + "$deprecation$private com.google.protobuf.MapField<$type_parameters$>\n" + " internalGet$capitalized_name$() {\n" + " if ($name$_ == null) {\n" + " return com.google.protobuf.MapField.emptyMapField(\n" + " $map_field_parameter$);\n" + " }\n" + " return $name$_;\n" + "}\n" + "$deprecation$private com.google.protobuf.MapField<$type_parameters$>\n" + " internalGetMutable$capitalized_name$() {\n" + " if ($name$_ == null) {\n" + " $name$_ = com.google.protobuf.MapField.newMapField(\n" + " $map_field_parameter$);\n" + " }\n" + " if (!$name$_.isMutable()) {\n" + " $name$_ = $name$_.copy();\n" + " }\n" + " $set_has_field_bit_builder$\n" + " $on_changed$\n" + " return $name$_;\n" + "}\n"); GenerateMapGetters(printer); - printer->Print(variables_, - "$deprecation$\n" - "public Builder ${$clear$capitalized_name$$}$() {\n" - " internalGetMutable$capitalized_name$().getMutableMap()\n" - " .clear();\n" - " return this;\n" - "}\n"); + printer->Print( + variables_, + "$deprecation$public Builder ${$clear$capitalized_name$$}$() {\n" + " $clear_has_field_bit_builder$\n" + " internalGetMutable$capitalized_name$().getMutableMap()\n" + " .clear();\n" + " return this;\n" + "}\n"); printer->Annotate("{", "}", descriptor_); + WriteFieldDocComment(printer, descriptor_); printer->Print(variables_, - "$deprecation$\n" - "public Builder ${$remove$capitalized_name$$}$(\n" + "$deprecation$public Builder ${$remove$capitalized_name$$}$(\n" " $key_type$ key) {\n" " $key_null_check$\n" " internalGetMutable$capitalized_name$().getMutableMap()\n" @@ -415,6 +433,7 @@ void ImmutableMapFieldGenerator::GenerateBuilderMembers( " return this;\n" "}\n"); printer->Annotate("{", "}", descriptor_); + if (GetJavaType(ValueField(descriptor_)) == JAVATYPE_ENUM) { if (context_->options().opensource_runtime) { printer->Print( @@ -424,12 +443,14 @@ void ImmutableMapFieldGenerator::GenerateBuilderMembers( " */\n" "@java.lang.Deprecated\n" "public java.util.Map<$boxed_key_type$, $value_enum_type$>\n" - "${$getMutable$capitalized_name$$}$() {\n" + " ${$getMutable$capitalized_name$$}$() {\n" + " $set_has_field_bit_builder$\n" " return internalGetAdapted$capitalized_name$Map(\n" " internalGetMutable$capitalized_name$().getMutableMap());\n" "}\n"); printer->Annotate("{", "}", descriptor_); } + WriteFieldDocComment(printer, descriptor_); printer->Print(variables_, "$deprecation$public Builder ${$put$capitalized_name$$}$(\n" @@ -439,9 +460,11 @@ void ImmutableMapFieldGenerator::GenerateBuilderMembers( " $value_null_check$\n" " internalGetMutable$capitalized_name$().getMutableMap()\n" " .put(key, $name$ValueConverter.doBackward(value));\n" + " $set_has_field_bit_builder$\n" " return this;\n" "}\n"); printer->Annotate("{", "}", descriptor_); + WriteFieldDocComment(printer, descriptor_); printer->Print( variables_, @@ -450,9 +473,11 @@ void ImmutableMapFieldGenerator::GenerateBuilderMembers( " internalGetAdapted$capitalized_name$Map(\n" " internalGetMutable$capitalized_name$().getMutableMap())\n" " .putAll(values);\n" + " $set_has_field_bit_builder$\n" " return this;\n" "}\n"); printer->Annotate("{", "}", descriptor_); + if (SupportUnknownEnumValue(descriptor_->file())) { if (context_->options().opensource_runtime) { printer->Print( @@ -463,10 +488,12 @@ void ImmutableMapFieldGenerator::GenerateBuilderMembers( "@java.lang.Deprecated\n" "public java.util.Map<$boxed_key_type$, $boxed_value_type$>\n" "${$getMutable$capitalized_name$Value$}$() {\n" + " $set_has_field_bit_builder$\n" " return internalGetMutable$capitalized_name$().getMutableMap();\n" "}\n"); printer->Annotate("{", "}", descriptor_); } + WriteFieldDocComment(printer, descriptor_); printer->Print( variables_, @@ -477,9 +504,11 @@ void ImmutableMapFieldGenerator::GenerateBuilderMembers( " $value_null_check$\n" " internalGetMutable$capitalized_name$().getMutableMap()\n" " .put(key, value);\n" + " $set_has_field_bit_builder$\n" " return this;\n" "}\n"); printer->Annotate("{", "}", descriptor_); + WriteFieldDocComment(printer, descriptor_); printer->Print( variables_, @@ -487,6 +516,7 @@ void ImmutableMapFieldGenerator::GenerateBuilderMembers( " java.util.Map<$boxed_key_type$, $boxed_value_type$> values) {\n" " internalGetMutable$capitalized_name$().getMutableMap()\n" " .putAll(values);\n" + " $set_has_field_bit_builder$\n" " return this;\n" "}\n"); printer->Annotate("{", "}", descriptor_); @@ -500,56 +530,61 @@ void ImmutableMapFieldGenerator::GenerateBuilderMembers( " */\n" "@java.lang.Deprecated\n" "public java.util.Map<$type_parameters$>\n" - "${$getMutable$capitalized_name$$}$() {\n" + " ${$getMutable$capitalized_name$$}$() {\n" + " $set_has_field_bit_builder$\n" " return internalGetMutable$capitalized_name$().getMutableMap();\n" "}\n"); printer->Annotate("{", "}", descriptor_); } + WriteFieldDocComment(printer, descriptor_); printer->Print(variables_, - "$deprecation$" - "public Builder ${$put$capitalized_name$$}$(\n" + "$deprecation$public Builder ${$put$capitalized_name$$}$(\n" " $key_type$ key,\n" " $value_type$ value) {\n" " $key_null_check$\n" " $value_null_check$\n" " internalGetMutable$capitalized_name$().getMutableMap()\n" " .put(key, value);\n" + " $set_has_field_bit_builder$\n" " return this;\n" "}\n"); printer->Annotate("{", "}", descriptor_); + WriteFieldDocComment(printer, descriptor_); - printer->Print(variables_, - "$deprecation$\n" - "public Builder ${$putAll$capitalized_name$$}$(\n" - " java.util.Map<$type_parameters$> values) {\n" - " internalGetMutable$capitalized_name$().getMutableMap()\n" - " .putAll(values);\n" - " return this;\n" - "}\n"); + printer->Print( + variables_, + "$deprecation$public Builder ${$putAll$capitalized_name$$}$(\n" + " java.util.Map<$type_parameters$> values) {\n" + " internalGetMutable$capitalized_name$().getMutableMap()\n" + " .putAll(values);\n" + " $set_has_field_bit_builder$\n" + " return this;\n" + "}\n"); printer->Annotate("{", "}", descriptor_); } } void ImmutableMapFieldGenerator::GenerateMapGetters( io::Printer* printer) const { - printer->Print(variables_, - "$deprecation$\n" - "public int ${$get$capitalized_name$Count$}$() {\n" - " return internalGet$capitalized_name$().getMap().size();\n" - "}\n"); + printer->Print( + variables_, + "$deprecation$public int ${$get$capitalized_name$Count$}$() {\n" + " return internalGet$capitalized_name$().getMap().size();\n" + "}\n"); printer->Annotate("{", "}", descriptor_); + WriteFieldDocComment(printer, descriptor_); printer->Print( variables_, - "$deprecation$\n" "@java.lang.Override\n" - "public boolean ${$contains$capitalized_name$$}$(\n" + "$deprecation$public boolean ${$contains$capitalized_name$$}$(\n" " $key_type$ key) {\n" " $key_null_check$\n" " return internalGet$capitalized_name$().getMap().containsKey(key);\n" "}\n"); printer->Annotate("{", "}", descriptor_); + if (GetJavaType(ValueField(descriptor_)) == JAVATYPE_ENUM) { if (context_->options().opensource_runtime) { printer->Print( @@ -565,22 +600,23 @@ void ImmutableMapFieldGenerator::GenerateMapGetters( "}\n"); printer->Annotate("{", "}", descriptor_); } + WriteFieldDocComment(printer, descriptor_); printer->Print(variables_, "@java.lang.Override\n" - "$deprecation$\n" - "public java.util.Map<$boxed_key_type$, $value_enum_type$>\n" + "$deprecation$public java.util.Map<$boxed_key_type$, " + "$value_enum_type$>\n" "${$get$capitalized_name$Map$}$() {\n" " return internalGetAdapted$capitalized_name$Map(\n" " internalGet$capitalized_name$().getMap());" "}\n"); printer->Annotate("{", "}", descriptor_); + WriteFieldDocComment(printer, descriptor_); printer->Print( variables_, "@java.lang.Override\n" - "$deprecation$\n" - "public $value_enum_type_pass_through_nullness$ " + "$deprecation$public $value_enum_type_pass_through_nullness$ " "${$get$capitalized_name$OrDefault$}$(\n" " $key_type$ key,\n" " $value_enum_type_pass_through_nullness$ defaultValue) {\n" @@ -592,12 +628,12 @@ void ImmutableMapFieldGenerator::GenerateMapGetters( " : defaultValue;\n" "}\n"); printer->Annotate("{", "}", descriptor_); + WriteFieldDocComment(printer, descriptor_); printer->Print( variables_, "@java.lang.Override\n" - "$deprecation$\n" - "public $value_enum_type$ ${$get$capitalized_name$OrThrow$}$(\n" + "$deprecation$public $value_enum_type$ get$capitalized_name$OrThrow(\n" " $key_type$ key) {\n" " $key_null_check$\n" " java.util.Map<$boxed_key_type$, $boxed_value_type$> map =\n" @@ -608,6 +644,7 @@ void ImmutableMapFieldGenerator::GenerateMapGetters( " return $name$ValueConverter.doForward(map.get(key));\n" "}\n"); printer->Annotate("{", "}", descriptor_); + if (SupportUnknownEnumValue(descriptor_->file())) { printer->Print( variables_, @@ -622,21 +659,19 @@ void ImmutableMapFieldGenerator::GenerateMapGetters( "}\n"); printer->Annotate("{", "}", descriptor_); WriteFieldDocComment(printer, descriptor_); - printer->Print( - variables_, - "@java.lang.Override\n" - "$deprecation$\n" - "public java.util.Map<$boxed_key_type$, $boxed_value_type$>\n" - "${$get$capitalized_name$ValueMap$}$() {\n" - " return internalGet$capitalized_name$().getMap();\n" - "}\n"); + printer->Print(variables_, + "@java.lang.Override\n" + "$deprecation$public java.util.Map<$boxed_key_type$, " + "$boxed_value_type$>\n" + "${$get$capitalized_name$ValueMap$}$() {\n" + " return internalGet$capitalized_name$().getMap();\n" + "}\n"); printer->Annotate("{", "}", descriptor_); WriteFieldDocComment(printer, descriptor_); printer->Print( variables_, "@java.lang.Override\n" - "$deprecation$\n" - "public $value_type_pass_through_nullness$ " + "$deprecation$public $value_type_pass_through_nullness$ " "${$get$capitalized_name$ValueOrDefault$}$(\n" " $key_type$ key,\n" " $value_type_pass_through_nullness$ defaultValue) {\n" @@ -650,8 +685,8 @@ void ImmutableMapFieldGenerator::GenerateMapGetters( printer->Print( variables_, "@java.lang.Override\n" - "$deprecation$\n" - "public $value_type$ ${$get$capitalized_name$ValueOrThrow$}$(\n" + "$deprecation$public $value_type$ " + "${$get$capitalized_name$ValueOrThrow$}$(\n" " $key_type$ key) {\n" " $key_null_check$\n" " java.util.Map<$boxed_key_type$, $boxed_value_type$> map =\n" @@ -680,8 +715,7 @@ void ImmutableMapFieldGenerator::GenerateMapGetters( WriteFieldDocComment(printer, descriptor_); printer->Print(variables_, "@java.lang.Override\n" - "$deprecation$\n" - "public java.util.Map<$type_parameters$> " + "$deprecation$public java.util.Map<$type_parameters$> " "${$get$capitalized_name$Map$}$() {\n" " return internalGet$capitalized_name$().getMap();\n" "}\n"); @@ -690,8 +724,7 @@ void ImmutableMapFieldGenerator::GenerateMapGetters( printer->Print( variables_, "@java.lang.Override\n" - "$deprecation$\n" - "public $value_type_pass_through_nullness$ " + "$deprecation$public $value_type_pass_through_nullness$ " "${$get$capitalized_name$OrDefault$}$(\n" " $key_type$ key,\n" " $value_type_pass_through_nullness$ defaultValue) {\n" @@ -702,19 +735,19 @@ void ImmutableMapFieldGenerator::GenerateMapGetters( "}\n"); printer->Annotate("{", "}", descriptor_); WriteFieldDocComment(printer, descriptor_); - printer->Print(variables_, - "@java.lang.Override\n" - "$deprecation$\n" - "public $value_type$ ${$get$capitalized_name$OrThrow$}$(\n" - " $key_type$ key) {\n" - " $key_null_check$\n" - " java.util.Map<$type_parameters$> map =\n" - " internalGet$capitalized_name$().getMap();\n" - " if (!map.containsKey(key)) {\n" - " throw new java.lang.IllegalArgumentException();\n" - " }\n" - " return map.get(key);\n" - "}\n"); + printer->Print( + variables_, + "@java.lang.Override\n" + "$deprecation$public $value_type$ ${$get$capitalized_name$OrThrow$}$(\n" + " $key_type$ key) {\n" + " $key_null_check$\n" + " java.util.Map<$type_parameters$> map =\n" + " internalGet$capitalized_name$().getMap();\n" + " if (!map.containsKey(key)) {\n" + " throw new java.lang.IllegalArgumentException();\n" + " }\n" + " return map.get(key);\n" + "}\n"); printer->Annotate("{", "}", descriptor_); } } @@ -813,6 +846,7 @@ void ImmutableMapFieldGenerator::GenerateInitializationCode( void ImmutableMapFieldGenerator::GenerateBuilderClearCode( io::Printer* printer) const { + // No need to clear the has-bit since we clear the bitField ints all at once. printer->Print(variables_, "internalGetMutable$capitalized_name$().clear();\n"); } @@ -821,14 +855,17 @@ void ImmutableMapFieldGenerator::GenerateMergingCode( io::Printer* printer) const { printer->Print(variables_, "internalGetMutable$capitalized_name$().mergeFrom(\n" - " other.internalGet$capitalized_name$());\n"); + " other.internalGet$capitalized_name$());\n" + "$set_has_field_bit_builder$\n"); } void ImmutableMapFieldGenerator::GenerateBuildingCode( io::Printer* printer) const { printer->Print(variables_, - "result.$name$_ = internalGet$capitalized_name$();\n" - "result.$name$_.makeImmutable();\n"); + "if ($get_has_field_bit_from_local$) {\n" + " result.$name$_ = internalGet$capitalized_name$();\n" + " result.$name$_.makeImmutable();\n" + "}\n"); } void ImmutableMapFieldGenerator::GenerateBuilderParsingCode( @@ -845,6 +882,7 @@ void ImmutableMapFieldGenerator::GenerateBuilderParsingCode( "} else {\n" " internalGetMutable$capitalized_name$().getMutableMap().put(\n" " $name$__.getKey(), $name$__.getValue());\n" + " $set_has_field_bit_builder$\n" "}\n"); } else { printer->Print( @@ -853,7 +891,8 @@ void ImmutableMapFieldGenerator::GenerateBuilderParsingCode( "$name$__ = input.readMessage(\n" " $default_entry$.getParserForType(), extensionRegistry);\n" "internalGetMutable$capitalized_name$().getMutableMap().put(\n" - " $name$__.getKey(), $name$__.getValue());\n"); + " $name$__.getKey(), $name$__.getValue());\n" + "$set_has_field_bit_builder$\n"); } } void ImmutableMapFieldGenerator::GenerateSerializationCode( diff --git a/src/google/protobuf/compiler/java/map_field.h b/src/google/protobuf/compiler/java/map_field.h index 39cdbde7677e..5454ea081a56 100644 --- a/src/google/protobuf/compiler/java/map_field.h +++ b/src/google/protobuf/compiler/java/map_field.h @@ -46,6 +46,8 @@ class ImmutableMapFieldGenerator : public ImmutableFieldGenerator { ~ImmutableMapFieldGenerator() override; // implements ImmutableFieldGenerator --------------------------------------- + int GetMessageBitIndex() const override; + int GetBuilderBitIndex() const override; int GetNumBitsForMessage() const override; int GetNumBitsForBuilder() const override; void GenerateInterfaceMembers(io::Printer* printer) const override; @@ -68,6 +70,8 @@ class ImmutableMapFieldGenerator : public ImmutableFieldGenerator { private: const FieldDescriptor* descriptor_; + int message_bit_index_; + int builder_bit_index_; absl::flat_hash_map variables_; ClassNameResolver* name_resolver_; Context* context_; diff --git a/src/google/protobuf/compiler/java/message_builder.cc b/src/google/protobuf/compiler/java/message_builder.cc index 4d63c8820345..663436f884bf 100644 --- a/src/google/protobuf/compiler/java/message_builder.cc +++ b/src/google/protobuf/compiler/java/message_builder.cc @@ -36,6 +36,7 @@ #include #include +#include #include #include "google/protobuf/io/coded_stream.h" @@ -148,12 +149,11 @@ void MessageBuilderGenerator::Generate(io::Printer* printer) { "\n" "public Builder clear$oneof_capitalized_name$() {\n" " $oneof_name$Case_ = 0;\n" - " $oneof_name$_ = null;\n"); - printer->Print(" onChanged();\n"); - printer->Print( - " return this;\n" - "}\n" - "\n"); + " $oneof_name$_ = null;\n" + " onChanged();\n" + " return this;\n" + "}\n" + "\n"); } // Integers for bit fields. @@ -360,6 +360,11 @@ void MessageBuilderGenerator::GenerateCommonBuilderMethods( " super.clear();\n"); printer->Indent(); + int totalBuilderInts = (descriptor_->field_count() + 31) / 32; + for (int i = 0; i < totalBuilderInts; i++) { + printer->Print("$bit_field_name$ = 0;\n", "bit_field_name", + GetBitFieldName(i)); + } for (int i = 0; i < descriptor_->field_count(); i++) { field_generators_.get(descriptor_->field(i)) @@ -411,64 +416,7 @@ void MessageBuilderGenerator::GenerateCommonBuilderMethods( "\n", "classname", name_resolver_->GetImmutableClassName(descriptor_)); - printer->Print( - "@java.lang.Override\n" - "public $classname$ buildPartial() {\n" - " $classname$ result = new $classname$(this);\n", - "classname", name_resolver_->GetImmutableClassName(descriptor_)); - - printer->Indent(); - - int totalBuilderBits = 0; - int totalMessageBits = 0; - for (int i = 0; i < descriptor_->field_count(); i++) { - const ImmutableFieldGenerator& field = - field_generators_.get(descriptor_->field(i)); - totalBuilderBits += field.GetNumBitsForBuilder(); - totalMessageBits += field.GetNumBitsForMessage(); - } - int totalBuilderInts = (totalBuilderBits + 31) / 32; - int totalMessageInts = (totalMessageBits + 31) / 32; - - // Local vars for from and to bit fields to avoid accessing the builder and - // message over and over for these fields. Seems to provide a slight - // perforamance improvement in micro benchmark and this is also what proto1 - // code does. - for (int i = 0; i < totalBuilderInts; i++) { - printer->Print("int from_$bit_field_name$ = $bit_field_name$;\n", - "bit_field_name", GetBitFieldName(i)); - } - for (int i = 0; i < totalMessageInts; i++) { - printer->Print("int to_$bit_field_name$ = 0;\n", "bit_field_name", - GetBitFieldName(i)); - } - - // Output generation code for each field. - for (int i = 0; i < descriptor_->field_count(); i++) { - field_generators_.get(descriptor_->field(i)).GenerateBuildingCode(printer); - } - - // Copy the bit field results to the generated message - for (int i = 0; i < totalMessageInts; i++) { - printer->Print("result.$bit_field_name$ = to_$bit_field_name$;\n", - "bit_field_name", GetBitFieldName(i)); - } - - for (auto& kv : oneofs_) { - printer->Print("result.$oneof_name$Case_ = $oneof_name$Case_;\n", - "oneof_name", - context_->GetOneofGeneratorInfo(kv.second)->name); - } - - printer->Outdent(); - - printer->Print(" onBuilt();\n"); - - printer->Print( - " return result;\n" - "}\n" - "\n", - "classname", name_resolver_->GetImmutableClassName(descriptor_)); + GenerateBuildPartial(printer); if (context_->options().opensource_runtime) { // Override methods declared in GeneratedMessage to return the concrete @@ -619,6 +567,157 @@ void MessageBuilderGenerator::GenerateCommonBuilderMethods( } } +void MessageBuilderGenerator::GenerateBuildPartial(io::Printer* printer) { + printer->Print( + "@java.lang.Override\n" + "public $classname$ buildPartial() {\n" + " $classname$ result = new $classname$(this);\n", + "classname", name_resolver_->GetImmutableClassName(descriptor_)); + + printer->Indent(); + + // Handle the repeated fields first so that the "mutable bits" are cleared. + bool has_repeated_fields = false; + for (int i = 0; i < descriptor_->field_count(); ++i) { + if (descriptor_->field(i)->is_repeated() && + !IsMapField(descriptor_->field(i))) { + has_repeated_fields = true; + printer->Print("buildPartialRepeatedFields(result);\n"); + break; + } + } + + // One buildPartial#() per from_bit_field + int totalBuilderInts = (descriptor_->field_count() + 31) / 32; + if (totalBuilderInts > 0) { + for (int i = 0; i < totalBuilderInts; ++i) { + printer->Print( + "if ($bit_field_name$ != 0) { buildPartial$piece$(result); }\n", + "bit_field_name", GetBitFieldName(i), "piece", absl::StrCat(i)); + } + } + + if (!oneofs_.empty()) { + printer->Print("buildPartialOneofs(result);\n"); + } + + printer->Outdent(); + printer->Print( + " onBuilt();\n" + " return result;\n" + "}\n" + "\n", + "classname", name_resolver_->GetImmutableClassName(descriptor_)); + + // Build Repeated Fields + if (has_repeated_fields) { + printer->Print( + "private void buildPartialRepeatedFields($classname$ result) {\n", + "classname", name_resolver_->GetImmutableClassName(descriptor_)); + printer->Indent(); + for (int i = 0; i < descriptor_->field_count(); ++i) { + if (descriptor_->field(i)->is_repeated() && + !IsMapField(descriptor_->field(i))) { + const ImmutableFieldGenerator& field = + field_generators_.get(descriptor_->field(i)); + field.GenerateBuildingCode(printer); + } + } + printer->Outdent(); + printer->Print("}\n\n"); + } + + // Build non-oneof fields + int start_field = 0; + for (int i = 0; i < totalBuilderInts; i++) { + start_field = GenerateBuildPartialPiece(printer, i, start_field); + } + + // Build Oneofs + if (!oneofs_.empty()) { + printer->Print("private void buildPartialOneofs($classname$ result) {\n", + "classname", + name_resolver_->GetImmutableClassName(descriptor_)); + printer->Indent(); + for (auto& kv : oneofs_) { + const OneofDescriptor* oneof = kv.second; + printer->Print( + "result.$oneof_name$Case_ = $oneof_name$Case_;\n" + "result.$oneof_name$_ = this.$oneof_name$_;\n", + "oneof_name", context_->GetOneofGeneratorInfo(oneof)->name); + for (int i = 0; i < oneof->field_count(); ++i) { + if (oneof->field(i)->message_type() != nullptr) { + const ImmutableFieldGenerator& field = + field_generators_.get(oneof->field(i)); + field.GenerateBuildingCode(printer); + } + } + } + printer->Outdent(); + printer->Print("}\n\n"); + } +} + +int MessageBuilderGenerator::GenerateBuildPartialPiece(io::Printer* printer, + int piece, + int first_field) { + printer->Print( + "private void buildPartial$piece$($classname$ result) {\n" + " int from_$bit_field_name$ = $bit_field_name$;\n", + "classname", name_resolver_->GetImmutableClassName(descriptor_), "piece", + absl::StrCat(piece), "bit_field_name", GetBitFieldName(piece)); + printer->Indent(); + std::set declared_to_bitfields; + + int bit = 0; + int next = first_field; + for (; bit < 32 && next < descriptor_->field_count(); ++next) { + const ImmutableFieldGenerator& field = + field_generators_.get(descriptor_->field(next)); + bit += field.GetNumBitsForBuilder(); + + // Skip oneof fields that are handled separately + if (IsRealOneof(descriptor_->field(next))) { + continue; + } + + // Skip repeated fields because they are currently handled + // in separate buildPartial sub-methods. + if (descriptor_->field(next)->is_repeated() && + !IsMapField(descriptor_->field(next))) { + continue; + } + // Skip fields without presence bits in the builder + if (field.GetNumBitsForBuilder() == 0) { + continue; + } + + // Track message bits if necessary + if (field.GetNumBitsForMessage() > 0) { + int to_bitfield = field.GetMessageBitIndex() / 32; + if (declared_to_bitfields.count(to_bitfield) == 0) { + printer->Print("int to_$bit_field_name$ = 0;\n", "bit_field_name", + GetBitFieldName(to_bitfield)); + declared_to_bitfields.insert(to_bitfield); + } + } + + // Copy the field from the builder to the message + field.GenerateBuildingCode(printer); + } + + // Copy the bit field results to the generated message + for (int to_bitfield : declared_to_bitfields) { + printer->Print("result.$bit_field_name$ |= to_$bit_field_name$;\n", + "bit_field_name", GetBitFieldName(to_bitfield)); + } + + printer->Outdent(); + printer->Print("}\n\n"); + + return next; +} + // =================================================================== void MessageBuilderGenerator::GenerateBuilderParsingMethods( diff --git a/src/google/protobuf/compiler/java/message_builder.h b/src/google/protobuf/compiler/java/message_builder.h index ec9b56fcf5bb..efafd36bcc7a 100644 --- a/src/google/protobuf/compiler/java/message_builder.h +++ b/src/google/protobuf/compiler/java/message_builder.h @@ -71,6 +71,11 @@ class MessageBuilderGenerator { private: void GenerateCommonBuilderMethods(io::Printer* printer); + void GenerateBuildPartial(io::Printer* printer); + int GenerateBuildPartialPiece(io::Printer* printer, int piece, + int first_field); + int GenerateBuildPartialPieceWithoutPresence(io::Printer* printer, int piece, + int first_field); void GenerateDescriptorMethods(io::Printer* printer); void GenerateBuilderParsingMethods(io::Printer* printer); void GenerateBuilderFieldParsingCases(io::Printer* printer); diff --git a/src/google/protobuf/compiler/java/message_field.cc b/src/google/protobuf/compiler/java/message_field.cc index c77d345e7c85..a0d73f22753f 100644 --- a/src/google/protobuf/compiler/java/message_field.cc +++ b/src/google/protobuf/compiler/java/message_field.cc @@ -91,22 +91,14 @@ void SetMessageVariables( if (HasHasbit(descriptor)) { // For singular messages and builders, one bit is used for the hasField bit. (*variables)["get_has_field_bit_message"] = GenerateGetBit(messageBitIndex); - (*variables)["get_has_field_bit_builder"] = GenerateGetBit(builderBitIndex); // Note that these have a trailing ";". - (*variables)["set_has_field_bit_message"] = - GenerateSetBit(messageBitIndex) + ";"; - (*variables)["set_has_field_bit_builder"] = - GenerateSetBit(builderBitIndex) + ";"; - (*variables)["clear_has_field_bit_builder"] = - GenerateClearBit(builderBitIndex) + ";"; + (*variables)["set_has_field_bit_to_local"] = + GenerateSetBitToLocal(messageBitIndex); (*variables)["is_field_present_message"] = GenerateGetBit(messageBitIndex); } else { - (*variables)["set_has_field_bit_message"] = ""; - (*variables)["set_has_field_bit_builder"] = ""; - (*variables)["clear_has_field_bit_builder"] = ""; - + (*variables)["set_has_field_bit_to_local"] = ""; variables->insert({"is_field_present_message", absl::StrCat((*variables)["name"], "_ != null")}); } @@ -116,10 +108,13 @@ void SetMessageVariables( (*variables)["set_mutable_bit_builder"] = GenerateSetBit(builderBitIndex); (*variables)["clear_mutable_bit_builder"] = GenerateClearBit(builderBitIndex); + (*variables)["get_has_field_bit_builder"] = GenerateGetBit(builderBitIndex); + (*variables)["set_has_field_bit_builder"] = + GenerateSetBit(builderBitIndex) + ";"; + (*variables)["clear_has_field_bit_builder"] = + GenerateClearBit(builderBitIndex) + ";"; (*variables)["get_has_field_bit_from_local"] = GenerateGetBitFromLocal(builderBitIndex); - (*variables)["set_has_field_bit_to_local"] = - GenerateSetBitToLocal(messageBitIndex); } } // namespace @@ -130,6 +125,8 @@ ImmutableMessageFieldGenerator::ImmutableMessageFieldGenerator( const FieldDescriptor* descriptor, int messageBitIndex, int builderBitIndex, Context* context) : descriptor_(descriptor), + message_bit_index_(messageBitIndex), + builder_bit_index_(builderBitIndex), name_resolver_(context->GetNameResolver()), context_(context) { SetMessageVariables(descriptor, messageBitIndex, builderBitIndex, @@ -139,13 +136,19 @@ ImmutableMessageFieldGenerator::ImmutableMessageFieldGenerator( ImmutableMessageFieldGenerator::~ImmutableMessageFieldGenerator() {} +int ImmutableMessageFieldGenerator::GetMessageBitIndex() const { + return message_bit_index_; +} + +int ImmutableMessageFieldGenerator::GetBuilderBitIndex() const { + return builder_bit_index_; +} + int ImmutableMessageFieldGenerator::GetNumBitsForMessage() const { return HasHasbit(descriptor_) ? 1 : 0; } -int ImmutableMessageFieldGenerator::GetNumBitsForBuilder() const { - return GetNumBitsForMessage(); -} +int ImmutableMessageFieldGenerator::GetNumBitsForBuilder() const { return 1; } void ImmutableMessageFieldGenerator::GenerateInterfaceMembers( io::Printer* printer) const { @@ -178,24 +181,6 @@ void ImmutableMessageFieldGenerator::GenerateMembers( " return $get_has_field_bit_message$;\n" "}\n"); printer->Annotate("{", "}", descriptor_); - WriteFieldAccessorDocComment(printer, descriptor_, GETTER); - printer->Print( - variables_, - "@java.lang.Override\n" - "$deprecation$public $type$ ${$get$capitalized_name$$}$() {\n" - " return $name$_ == null ? $type$.getDefaultInstance() : $name$_;\n" - "}\n"); - printer->Annotate("{", "}", descriptor_); - - WriteFieldDocComment(printer, descriptor_); - printer->Print( - variables_, - "@java.lang.Override\n" - "$deprecation$public $type$OrBuilder " - "${$get$capitalized_name$OrBuilder$}$() {\n" - " return $name$_ == null ? $type$.getDefaultInstance() : $name$_;\n" - "}\n"); - printer->Annotate("{", "}", descriptor_); } else { WriteFieldAccessorDocComment(printer, descriptor_, HAZZER); printer->Print( @@ -205,24 +190,25 @@ void ImmutableMessageFieldGenerator::GenerateMembers( " return $name$_ != null;\n" "}\n"); printer->Annotate("{", "}", descriptor_); - WriteFieldAccessorDocComment(printer, descriptor_, GETTER); - printer->Print( - variables_, - "@java.lang.Override\n" - "$deprecation$public $type$ ${$get$capitalized_name$$}$() {\n" - " return $name$_ == null ? $type$.getDefaultInstance() : $name$_;\n" - "}\n"); - printer->Annotate("{", "}", descriptor_); - - WriteFieldDocComment(printer, descriptor_); - printer->Print(variables_, - "@java.lang.Override\n" - "$deprecation$public $type$OrBuilder " - "${$get$capitalized_name$OrBuilder$}$() {\n" - " return get$capitalized_name$();\n" - "}\n"); - printer->Annotate("{", "}", descriptor_); } + WriteFieldAccessorDocComment(printer, descriptor_, GETTER); + printer->Print( + variables_, + "@java.lang.Override\n" + "$deprecation$public $type$ ${$get$capitalized_name$$}$() {\n" + " return $name$_ == null ? $type$.getDefaultInstance() : $name$_;\n" + "}\n"); + printer->Annotate("{", "}", descriptor_); + + WriteFieldDocComment(printer, descriptor_); + printer->Print( + variables_, + "@java.lang.Override\n" + "$deprecation$public $type$OrBuilder " + "${$get$capitalized_name$OrBuilder$}$() {\n" + " return $name$_ == null ? $type$.getDefaultInstance() : $name$_;\n" + "}\n"); + printer->Annotate("{", "}", descriptor_); } void ImmutableMessageFieldGenerator::PrintNestedBuilderCondition( @@ -260,9 +246,6 @@ void ImmutableMessageFieldGenerator::GenerateBuilderMembers( // When using nested-builders, the code initially works just like the // non-nested builder case. It only creates a nested builder lazily on // demand and then forever delegates to it after creation. - - bool has_hasbit = HasHasbit(descriptor_); - printer->Print(variables_, "private $type$ $name$_;\n"); printer->Print(variables_, @@ -277,21 +260,11 @@ void ImmutableMessageFieldGenerator::GenerateBuilderMembers( // boolean hasField() WriteFieldAccessorDocComment(printer, descriptor_, HAZZER); - if (has_hasbit) { - printer->Print( - variables_, - "$deprecation$public boolean ${$has$capitalized_name$$}$() {\n" - " return $get_has_field_bit_builder$;\n" - "}\n"); - printer->Annotate("{", "}", descriptor_); - } else { - printer->Print( - variables_, - "$deprecation$public boolean ${$has$capitalized_name$$}$() {\n" - " return $name$Builder_ != null || $name$_ != null;\n" - "}\n"); - printer->Annotate("{", "}", descriptor_); - } + printer->Print(variables_, + "$deprecation$public boolean ${$has$capitalized_name$$}$() {\n" + " return $get_has_field_bit_builder$;\n" + "}\n"); + printer->Annotate("{", "}", descriptor_); // Field getField() WriteFieldAccessorDocComment(printer, descriptor_, GETTER); @@ -309,12 +282,12 @@ void ImmutableMessageFieldGenerator::GenerateBuilderMembers( "if (value == null) {\n" " throw new NullPointerException();\n" "}\n" - "$name$_ = value;\n" - "$on_changed$\n", + "$name$_ = value;\n", "$name$Builder_.setMessage(value);\n", "$set_has_field_bit_builder$\n" + "$on_changed$\n" "return this;\n"); // Field.Builder setField(Field.Builder builderForValue) @@ -324,58 +297,48 @@ void ImmutableMessageFieldGenerator::GenerateBuilderMembers( "$deprecation$public Builder ${$set$capitalized_name$$}$(\n" " $type$.Builder builderForValue)", - "$name$_ = builderForValue.build();\n" - "$on_changed$\n", + "$name$_ = builderForValue.build();\n", "$name$Builder_.setMessage(builderForValue.build());\n", "$set_has_field_bit_builder$\n" + "$on_changed$\n" "return this;\n"); - // Field.Builder mergeField(Field value) + // Message.Builder mergeField(Field value) WriteFieldDocComment(printer, descriptor_); PrintNestedBuilderFunction( printer, "$deprecation$public Builder ${$merge$capitalized_name$$}$($type$ value)", - - has_hasbit - ? "if ($get_has_field_bit_builder$ &&\n" - " $name$_ != null &&\n" - " $name$_ != $type$.getDefaultInstance()) {\n" - " $name$_ =\n" - " $type$.newBuilder($name$_).mergeFrom(value).buildPartial();\n" - "} else {\n" - " $name$_ = value;\n" - "}\n" - "$on_changed$\n" - : "if ($name$_ != null) {\n" - " $name$_ =\n" - " $type$.newBuilder($name$_).mergeFrom(value).buildPartial();\n" - "} else {\n" - " $name$_ = value;\n" - "}\n" - "$on_changed$\n", + "if ($get_has_field_bit_builder$ &&\n" + " $name$_ != null &&\n" + " $name$_ != $type$.getDefaultInstance()) {\n" + " get$capitalized_name$Builder().mergeFrom(value);\n" + "} else {\n" + " $name$_ = value;\n" + "}\n", "$name$Builder_.mergeFrom(value);\n", "$set_has_field_bit_builder$\n" + "$on_changed$\n" "return this;\n"); - // Field.Builder clearField() + // Message.Builder clearField() WriteFieldDocComment(printer, descriptor_); - PrintNestedBuilderFunction( - printer, "$deprecation$public Builder ${$clear$capitalized_name$$}$()", - - "$name$_ = null;\n" - "$on_changed$\n", - - has_hasbit ? "$name$Builder_.clear();\n" - : "$name$_ = null;\n" - "$name$Builder_ = null;\n", - - "$clear_has_field_bit_builder$\n" - "return this;\n"); + printer->Print(variables_, + "$deprecation$public Builder clear$capitalized_name$() {\n" + " $clear_has_field_bit_builder$\n" + " $name$_ = null;\n" + " if ($name$Builder_ != null) {\n" + " $name$Builder_.dispose();\n" + " $name$Builder_ = null;\n" + " }\n" + " $on_changed$\n" + " return this;\n" + "}\n"); + // Field.Builder getFieldBuilder() WriteFieldDocComment(printer, descriptor_); printer->Print(variables_, "$deprecation$public $type$.Builder " @@ -385,6 +348,8 @@ void ImmutableMessageFieldGenerator::GenerateBuilderMembers( " return get$capitalized_name$FieldBuilder().getBuilder();\n" "}\n"); printer->Annotate("{", "}", descriptor_); + + // FieldOrBuilder getFieldOrBuilder() WriteFieldDocComment(printer, descriptor_); printer->Print(variables_, "$deprecation$public $type$OrBuilder " @@ -397,6 +362,8 @@ void ImmutableMessageFieldGenerator::GenerateBuilderMembers( " }\n" "}\n"); printer->Annotate("{", "}", descriptor_); + + // SingleFieldBuilder getFieldFieldBuilder WriteFieldDocComment(printer, descriptor_); printer->Print( variables_, @@ -455,9 +422,7 @@ void ImmutableMessageFieldGenerator::GenerateKotlinOrNull(io::Printer* printer) void ImmutableMessageFieldGenerator::GenerateFieldBuilderInitializationCode( io::Printer* printer) const { - if (HasHasbit(descriptor_)) { - printer->Print(variables_, "get$capitalized_name$FieldBuilder();\n"); - } + printer->Print(variables_, "get$capitalized_name$FieldBuilder();\n"); } void ImmutableMessageFieldGenerator::GenerateInitializationCode( @@ -465,17 +430,13 @@ void ImmutableMessageFieldGenerator::GenerateInitializationCode( void ImmutableMessageFieldGenerator::GenerateBuilderClearCode( io::Printer* printer) const { - if (HasHasbit(descriptor_)) { - PrintNestedBuilderCondition(printer, "$name$_ = null;\n", - - "$name$Builder_.clear();\n"); - printer->Print(variables_, "$clear_has_field_bit_builder$\n"); - } else { - PrintNestedBuilderCondition(printer, "$name$_ = null;\n", - - "$name$_ = null;\n" - "$name$Builder_ = null;\n"); - } + // No need to clear the has-bit since we clear the bitField ints all at once. + printer->Print(variables_, + "$name$_ = null;\n" + "if ($name$Builder_ != null) {\n" + " $name$Builder_.dispose();\n" + " $name$Builder_ = null;\n" + "}\n"); } void ImmutableMessageFieldGenerator::GenerateMergingCode( @@ -488,19 +449,15 @@ void ImmutableMessageFieldGenerator::GenerateMergingCode( void ImmutableMessageFieldGenerator::GenerateBuildingCode( io::Printer* printer) const { - if (HasHasbit(descriptor_)) { - printer->Print(variables_, "if ($get_has_field_bit_from_local$) {\n"); - printer->Indent(); - PrintNestedBuilderCondition(printer, "result.$name$_ = $name$_;\n", - "result.$name$_ = $name$Builder_.build();\n"); - printer->Outdent(); - printer->Print(variables_, - " $set_has_field_bit_to_local$;\n" - "}\n"); - } else { - PrintNestedBuilderCondition(printer, "result.$name$_ = $name$_;\n", - "result.$name$_ = $name$Builder_.build();\n"); + printer->Print(variables_, + "if ($get_has_field_bit_from_local$) {\n" + " result.$name$_ = $name$Builder_ == null\n" + " ? $name$_\n" + " : $name$Builder_.build();\n"); + if (GetNumBitsForMessage() > 0) { + printer->Print(variables_, " $set_has_field_bit_to_local$;\n"); } + printer->Print("}\n"); } void ImmutableMessageFieldGenerator::GenerateBuilderParsingCode( @@ -581,6 +538,7 @@ void ImmutableMessageOneofFieldGenerator::GenerateMembers( " return $has_oneof_case_message$;\n" "}\n"); printer->Annotate("{", "}", descriptor_); + WriteFieldAccessorDocComment(printer, descriptor_, GETTER); printer->Print(variables_, "@java.lang.Override\n" @@ -764,7 +722,7 @@ void ImmutableMessageOneofFieldGenerator::GenerateBuilderMembers( " $oneof_name$_ = null;\n" " }\n" " $set_oneof_case_message$;\n" - " $on_changed$;\n" + " $on_changed$\n" " return $name$Builder_;\n" "}\n"); printer->Annotate("{", "}", descriptor_); @@ -781,16 +739,11 @@ void ImmutableMessageOneofFieldGenerator::GenerateBuilderClearCode( void ImmutableMessageOneofFieldGenerator::GenerateBuildingCode( io::Printer* printer) const { - printer->Print(variables_, "if ($has_oneof_case_message$) {\n"); - printer->Indent(); - - PrintNestedBuilderCondition( - printer, "result.$oneof_name$_ = $oneof_name$_;\n", - - "result.$oneof_name$_ = $name$Builder_.build();\n"); - - printer->Outdent(); - printer->Print("}\n"); + printer->Print(variables_, + "if ($has_oneof_case_message$ &&\n" + " $name$Builder_ != null) {\n" + " result.$oneof_name$_ = $name$Builder_.build();\n" + "}\n"); } void ImmutableMessageOneofFieldGenerator::GenerateMergingCode( @@ -840,11 +793,8 @@ void ImmutableMessageOneofFieldGenerator::GenerateSerializedSizeCode( RepeatedImmutableMessageFieldGenerator::RepeatedImmutableMessageFieldGenerator( const FieldDescriptor* descriptor, int messageBitIndex, int builderBitIndex, Context* context) - : descriptor_(descriptor), name_resolver_(context->GetNameResolver()) { - SetMessageVariables(descriptor, messageBitIndex, builderBitIndex, - context->GetFieldGeneratorInfo(descriptor), - name_resolver_, &variables_, context); -} + : ImmutableMessageFieldGenerator(descriptor, messageBitIndex, + builderBitIndex, context) {} RepeatedImmutableMessageFieldGenerator:: ~RepeatedImmutableMessageFieldGenerator() {} @@ -898,6 +848,8 @@ void RepeatedImmutableMessageFieldGenerator::GenerateMembers( " return $name$_;\n" // note: unmodifiable list "}\n"); printer->Annotate("{", "}", descriptor_); + + // List getFieldOrBuilderList() WriteFieldDocComment(printer, descriptor_); printer->Print( variables_, @@ -907,6 +859,8 @@ void RepeatedImmutableMessageFieldGenerator::GenerateMembers( " return $name$_;\n" "}\n"); printer->Annotate("{", "}", descriptor_); + + // int getFieldCount() WriteFieldDocComment(printer, descriptor_); printer->Print( variables_, @@ -915,6 +869,8 @@ void RepeatedImmutableMessageFieldGenerator::GenerateMembers( " return $name$_.size();\n" "}\n"); printer->Annotate("{", "}", descriptor_); + + // Field getField(int index) WriteFieldDocComment(printer, descriptor_); printer->Print( variables_, @@ -923,6 +879,8 @@ void RepeatedImmutableMessageFieldGenerator::GenerateMembers( " return $name$_.get(index);\n" "}\n"); printer->Annotate("{", "}", descriptor_); + + // FieldOrBuilder getFieldOrBuilder(int index) WriteFieldDocComment(printer, descriptor_); printer->Print(variables_, "@java.lang.Override\n" @@ -1148,7 +1106,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateBuilderMembers( "return this;\n"); - // Builder clearAllRepeatedField() + // Builder clearRepeatedField() WriteFieldDocComment(printer, descriptor_); PrintNestedBuilderFunction( printer, "$deprecation$public Builder ${$clear$capitalized_name$$}$()", @@ -1175,6 +1133,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateBuilderMembers( "return this;\n"); + // Field.Builder getRepeatedFieldBuilder(int index) WriteFieldDocComment(printer, descriptor_); printer->Print( variables_, @@ -1184,6 +1143,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateBuilderMembers( "}\n"); printer->Annotate("{", "}", descriptor_); + // FieldOrBuilder getRepeatedFieldOrBuilder(int index) WriteFieldDocComment(printer, descriptor_); printer->Print(variables_, "$deprecation$public $type$OrBuilder " @@ -1197,6 +1157,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateBuilderMembers( "}\n"); printer->Annotate("{", "}", descriptor_); + // List getRepeatedFieldOrBuilderList() WriteFieldDocComment(printer, descriptor_); printer->Print( variables_, @@ -1210,6 +1171,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateBuilderMembers( "}\n"); printer->Annotate("{", "}", descriptor_); + // Field.Builder addRepeatedField() WriteFieldDocComment(printer, descriptor_); printer->Print(variables_, "$deprecation$public $type$.Builder " @@ -1218,6 +1180,8 @@ void RepeatedImmutableMessageFieldGenerator::GenerateBuilderMembers( " $type$.getDefaultInstance());\n" "}\n"); printer->Annotate("{", "}", descriptor_); + + // Field.Builder addRepeatedFieldBuilder(int index) WriteFieldDocComment(printer, descriptor_); printer->Print( variables_, @@ -1227,6 +1191,8 @@ void RepeatedImmutableMessageFieldGenerator::GenerateBuilderMembers( " index, $type$.getDefaultInstance());\n" "}\n"); printer->Annotate("{", "}", descriptor_); + + // List getRepeatedFieldBuilderList() WriteFieldDocComment(printer, descriptor_); printer->Print( variables_, diff --git a/src/google/protobuf/compiler/java/message_field.h b/src/google/protobuf/compiler/java/message_field.h index e3612194fd7f..acefb3669177 100644 --- a/src/google/protobuf/compiler/java/message_field.h +++ b/src/google/protobuf/compiler/java/message_field.h @@ -69,6 +69,8 @@ class ImmutableMessageFieldGenerator : public ImmutableFieldGenerator { // implements ImmutableFieldGenerator // --------------------------------------- + int GetMessageBitIndex() const override; + int GetBuilderBitIndex() const override; int GetNumBitsForMessage() const override; int GetNumBitsForBuilder() const override; void GenerateInterfaceMembers(io::Printer* printer) const override; @@ -91,18 +93,20 @@ class ImmutableMessageFieldGenerator : public ImmutableFieldGenerator { protected: const FieldDescriptor* descriptor_; + int message_bit_index_; + int builder_bit_index_; absl::flat_hash_map variables_; ClassNameResolver* name_resolver_; Context* context_; - void PrintNestedBuilderCondition(io::Printer* printer, - const char* regular_case, - const char* nested_builder_case) const; - void PrintNestedBuilderFunction(io::Printer* printer, - const char* method_prototype, - const char* regular_case, - const char* nested_builder_case, - const char* trailing_code) const; + virtual void PrintNestedBuilderCondition( + io::Printer* printer, const char* regular_case, + const char* nested_builder_case) const; + virtual void PrintNestedBuilderFunction(io::Printer* printer, + const char* method_prototype, + const char* regular_case, + const char* nested_builder_case, + const char* trailing_code) const; private: void GenerateKotlinOrNull(io::Printer* printer) const; @@ -130,7 +134,8 @@ class ImmutableMessageOneofFieldGenerator void GenerateSerializedSizeCode(io::Printer* printer) const override; }; -class RepeatedImmutableMessageFieldGenerator : public ImmutableFieldGenerator { +class RepeatedImmutableMessageFieldGenerator + : public ImmutableMessageFieldGenerator { public: explicit RepeatedImmutableMessageFieldGenerator( const FieldDescriptor* descriptor, int messageBitIndex, @@ -163,18 +168,14 @@ class RepeatedImmutableMessageFieldGenerator : public ImmutableFieldGenerator { std::string GetBoxedType() const override; protected: - const FieldDescriptor* descriptor_; - absl::flat_hash_map variables_; - ClassNameResolver* name_resolver_; - - void PrintNestedBuilderCondition(io::Printer* printer, - const char* regular_case, - const char* nested_builder_case) const; + void PrintNestedBuilderCondition( + io::Printer* printer, const char* regular_case, + const char* nested_builder_case) const override; void PrintNestedBuilderFunction(io::Printer* printer, const char* method_prototype, const char* regular_case, const char* nested_builder_case, - const char* trailing_code) const; + const char* trailing_code) const override; }; } // namespace java diff --git a/src/google/protobuf/compiler/java/primitive_field.cc b/src/google/protobuf/compiler/java/primitive_field.cc index 36754350b333..8a9eee414aa4 100644 --- a/src/google/protobuf/compiler/java/primitive_field.cc +++ b/src/google/protobuf/compiler/java/primitive_field.cc @@ -122,9 +122,7 @@ void SetPrimitiveVariables( WireFormat::TagSize(descriptor->number(), GetType(descriptor))); if (IsReferenceType(GetJavaType(descriptor))) { (*variables)["null_check"] = - " if (value == null) {\n" - " throw new NullPointerException();\n" - " }\n"; + "if (value == null) { throw new NullPointerException(); }"; } else { (*variables)["null_check"] = ""; } @@ -146,22 +144,12 @@ void SetPrimitiveVariables( if (HasHasbit(descriptor)) { // For singular messages and builders, one bit is used for the hasField bit. (*variables)["get_has_field_bit_message"] = GenerateGetBit(messageBitIndex); - (*variables)["get_has_field_bit_builder"] = GenerateGetBit(builderBitIndex); - // Note that these have a trailing ";". - (*variables)["set_has_field_bit_message"] = - GenerateSetBit(messageBitIndex) + ";"; - (*variables)["set_has_field_bit_builder"] = - GenerateSetBit(builderBitIndex) + ";"; - (*variables)["clear_has_field_bit_builder"] = - GenerateClearBit(builderBitIndex) + ";"; - + (*variables)["set_has_field_bit_to_local"] = + GenerateSetBitToLocal(messageBitIndex) + ";"; (*variables)["is_field_present_message"] = GenerateGetBit(messageBitIndex); } else { - (*variables)["set_has_field_bit_message"] = ""; - (*variables)["set_has_field_bit_builder"] = ""; - (*variables)["clear_has_field_bit_builder"] = ""; - + (*variables)["set_has_field_bit_to_local"] = ""; switch (descriptor->type()) { case FieldDescriptor::TYPE_BYTES: (*variables)["is_field_present_message"] = @@ -188,10 +176,15 @@ void SetPrimitiveVariables( (*variables)["set_mutable_bit_builder"] = GenerateSetBit(builderBitIndex); (*variables)["clear_mutable_bit_builder"] = GenerateClearBit(builderBitIndex); + // Always track the presence of a field explicitly in the builder, regardless + // of syntax. + (*variables)["get_has_field_bit_builder"] = GenerateGetBit(builderBitIndex); (*variables)["get_has_field_bit_from_local"] = GenerateGetBitFromLocal(builderBitIndex); - (*variables)["set_has_field_bit_to_local"] = - GenerateSetBitToLocal(messageBitIndex); + (*variables)["set_has_field_bit_builder"] = + GenerateSetBit(builderBitIndex) + ";"; + (*variables)["clear_has_field_bit_builder"] = + GenerateClearBit(builderBitIndex) + ";"; } } // namespace @@ -201,7 +194,10 @@ void SetPrimitiveVariables( ImmutablePrimitiveFieldGenerator::ImmutablePrimitiveFieldGenerator( const FieldDescriptor* descriptor, int messageBitIndex, int builderBitIndex, Context* context) - : descriptor_(descriptor), name_resolver_(context->GetNameResolver()) { + : descriptor_(descriptor), + message_bit_index_(messageBitIndex), + builder_bit_index_(builderBitIndex), + name_resolver_(context->GetNameResolver()) { SetPrimitiveVariables(descriptor, messageBitIndex, builderBitIndex, context->GetFieldGeneratorInfo(descriptor), name_resolver_, &variables_, context); @@ -209,13 +205,19 @@ ImmutablePrimitiveFieldGenerator::ImmutablePrimitiveFieldGenerator( ImmutablePrimitiveFieldGenerator::~ImmutablePrimitiveFieldGenerator() {} +int ImmutablePrimitiveFieldGenerator::GetMessageBitIndex() const { + return message_bit_index_; +} + +int ImmutablePrimitiveFieldGenerator::GetBuilderBitIndex() const { + return builder_bit_index_; +} + int ImmutablePrimitiveFieldGenerator::GetNumBitsForMessage() const { return HasHasbit(descriptor_) ? 1 : 0; } -int ImmutablePrimitiveFieldGenerator::GetNumBitsForBuilder() const { - return GetNumBitsForMessage(); -} +int ImmutablePrimitiveFieldGenerator::GetNumBitsForBuilder() const { return 1; } void ImmutablePrimitiveFieldGenerator::GenerateInterfaceMembers( io::Printer* printer) const { @@ -230,7 +232,7 @@ void ImmutablePrimitiveFieldGenerator::GenerateInterfaceMembers( void ImmutablePrimitiveFieldGenerator::GenerateMembers( io::Printer* printer) const { - printer->Print(variables_, "private $field_type$ $name$_;\n"); + printer->Print(variables_, "private $field_type$ $name$_ = $default$;\n"); PrintExtraFieldInfo(variables_, printer); if (HasHazzer(descriptor_)) { WriteFieldAccessorDocComment(printer, descriptor_, HAZZER); @@ -280,9 +282,9 @@ void ImmutablePrimitiveFieldGenerator::GenerateBuilderMembers( printer->Print(variables_, "$deprecation$public Builder " "${$set$capitalized_name$$}$($type$ value) {\n" - "$null_check$" - " $set_has_field_bit_builder$\n" + " $null_check$\n" " $name$_ = value;\n" + " $set_has_field_bit_builder$\n" " $on_changed$\n" " return this;\n" "}\n"); @@ -355,9 +357,8 @@ void ImmutablePrimitiveFieldGenerator::GenerateInitializationCode( void ImmutablePrimitiveFieldGenerator::GenerateBuilderClearCode( io::Printer* printer) const { - printer->Print(variables_, - "$name$_ = $default$;\n" - "$clear_has_field_bit_builder$\n"); + // No need to clear the has-bit since we clear the bitField ints all at once. + printer->Print(variables_, "$name$_ = $default$;\n"); } void ImmutablePrimitiveFieldGenerator::GenerateMergingCode( @@ -377,23 +378,13 @@ void ImmutablePrimitiveFieldGenerator::GenerateMergingCode( void ImmutablePrimitiveFieldGenerator::GenerateBuildingCode( io::Printer* printer) const { - if (HasHazzer(descriptor_)) { - if (IsDefaultValueJavaDefault(descriptor_)) { - printer->Print(variables_, - "if ($get_has_field_bit_from_local$) {\n" - " result.$name$_ = $name$_;\n" - " $set_has_field_bit_to_local$;\n" - "}\n"); - } else { - printer->Print(variables_, - "if ($get_has_field_bit_from_local$) {\n" - " $set_has_field_bit_to_local$;\n" - "}\n" - "result.$name$_ = $name$_;\n"); - } - } else { - printer->Print(variables_, "result.$name$_ = $name$_;\n"); + printer->Print(variables_, + "if ($get_has_field_bit_from_local$) {\n" + " result.$name$_ = $name$_;\n"); + if (GetNumBitsForMessage() > 0) { + printer->Print(variables_, " $set_has_field_bit_to_local$\n"); } + printer->Print("}\n"); } void ImmutablePrimitiveFieldGenerator::GenerateBuilderParsingCode( @@ -582,7 +573,7 @@ void ImmutablePrimitiveOneofFieldGenerator::GenerateBuilderMembers( printer->Print(variables_, "$deprecation$public Builder " "${$set$capitalized_name$$}$($type$ value) {\n" - "$null_check$" + " $null_check$\n" " $set_oneof_case_message$;\n" " $oneof_name$_ = value;\n" " $on_changed$\n" @@ -613,10 +604,7 @@ void ImmutablePrimitiveOneofFieldGenerator::GenerateBuilderClearCode( void ImmutablePrimitiveOneofFieldGenerator::GenerateBuildingCode( io::Printer* printer) const { - printer->Print(variables_, - "if ($has_oneof_case_message$) {\n" - " result.$oneof_name$_ = $oneof_name$_;\n" - "}\n"); + // no-op } void ImmutablePrimitiveOneofFieldGenerator::GenerateMergingCode( @@ -674,11 +662,8 @@ RepeatedImmutablePrimitiveFieldGenerator:: int messageBitIndex, int builderBitIndex, Context* context) - : descriptor_(descriptor), name_resolver_(context->GetNameResolver()) { - SetPrimitiveVariables(descriptor, messageBitIndex, builderBitIndex, - context->GetFieldGeneratorInfo(descriptor), - name_resolver_, &variables_, context); -} + : ImmutablePrimitiveFieldGenerator(descriptor, messageBitIndex, + builderBitIndex, context) {} RepeatedImmutablePrimitiveFieldGenerator:: ~RepeatedImmutablePrimitiveFieldGenerator() {} @@ -758,7 +743,7 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateBuilderMembers( " if (!$get_mutable_bit_builder$) {\n" " $name$_ = $mutable_copy_list$;\n" " $set_mutable_bit_builder$;\n" - " }\n" + " }\n" "}\n"); // Note: We return an unmodifiable list because otherwise the caller @@ -793,7 +778,7 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateBuilderMembers( printer->Print(variables_, "$deprecation$public Builder ${$set$capitalized_name$$}$(\n" " int index, $type$ value) {\n" - "$null_check$" + " $null_check$\n" " ensure$capitalized_name$IsMutable();\n" " $repeated_set$(index, value);\n" " $on_changed$\n" @@ -805,7 +790,7 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateBuilderMembers( printer->Print(variables_, "$deprecation$public Builder " "${$add$capitalized_name$$}$($type$ value) {\n" - "$null_check$" + " $null_check$\n" " ensure$capitalized_name$IsMutable();\n" " $repeated_add$(value);\n" " $on_changed$\n" @@ -943,9 +928,7 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateInitializationCode( void RepeatedImmutablePrimitiveFieldGenerator::GenerateBuilderClearCode( io::Printer* printer) const { - printer->Print(variables_, - "$name$_ = $empty_list$;\n" - "$clear_mutable_bit_builder$;\n"); + printer->Print(variables_, "$name$_ = $empty_list$;\n"); } void RepeatedImmutablePrimitiveFieldGenerator::GenerateMergingCode( diff --git a/src/google/protobuf/compiler/java/primitive_field.h b/src/google/protobuf/compiler/java/primitive_field.h index 2537aaeea043..7e9868609f5a 100644 --- a/src/google/protobuf/compiler/java/primitive_field.h +++ b/src/google/protobuf/compiler/java/primitive_field.h @@ -69,6 +69,8 @@ class ImmutablePrimitiveFieldGenerator : public ImmutableFieldGenerator { // implements ImmutableFieldGenerator // --------------------------------------- + int GetMessageBitIndex() const override; + int GetBuilderBitIndex() const override; int GetNumBitsForMessage() const override; int GetNumBitsForBuilder() const override; void GenerateInterfaceMembers(io::Printer* printer) const override; @@ -91,6 +93,8 @@ class ImmutablePrimitiveFieldGenerator : public ImmutableFieldGenerator { protected: const FieldDescriptor* descriptor_; + int message_bit_index_; + int builder_bit_index_; absl::flat_hash_map variables_; ClassNameResolver* name_resolver_; }; @@ -118,7 +122,7 @@ class ImmutablePrimitiveOneofFieldGenerator }; class RepeatedImmutablePrimitiveFieldGenerator - : public ImmutableFieldGenerator { + : public ImmutablePrimitiveFieldGenerator { public: explicit RepeatedImmutablePrimitiveFieldGenerator( const FieldDescriptor* descriptor, int messageBitIndex, @@ -151,11 +155,6 @@ class RepeatedImmutablePrimitiveFieldGenerator void GenerateKotlinDslMembers(io::Printer* printer) const override; std::string GetBoxedType() const override; - - private: - const FieldDescriptor* descriptor_; - absl::flat_hash_map variables_; - ClassNameResolver* name_resolver_; }; } // namespace java diff --git a/src/google/protobuf/compiler/java/string_field.cc b/src/google/protobuf/compiler/java/string_field.cc index 4992ef2bcc95..ef7f354c19c4 100644 --- a/src/google/protobuf/compiler/java/string_field.cc +++ b/src/google/protobuf/compiler/java/string_field.cc @@ -79,9 +79,7 @@ void SetPrimitiveVariables( (*variables)["tag_size"] = absl::StrCat( WireFormat::TagSize(descriptor->number(), GetType(descriptor))); (*variables)["null_check"] = - " if (value == null) {\n" - " throw new NullPointerException();\n" - " }\n"; + "if (value == null) { throw new NullPointerException(); }"; (*variables)["isStringEmpty"] = "com.google.protobuf.GeneratedMessage" + GeneratedCodeVersionSuffix() + ".isStringEmpty"; @@ -106,21 +104,18 @@ void SetPrimitiveVariables( if (HasHasbit(descriptor)) { // For singular messages and builders, one bit is used for the hasField bit. (*variables)["get_has_field_bit_message"] = GenerateGetBit(messageBitIndex); - (*variables)["get_has_field_bit_builder"] = GenerateGetBit(builderBitIndex); + (*variables)["set_has_field_bit_to_local"] = + GenerateSetBitToLocal(messageBitIndex); // Note that these have a trailing ";". (*variables)["set_has_field_bit_message"] = GenerateSetBit(messageBitIndex) + ";"; - (*variables)["set_has_field_bit_builder"] = - GenerateSetBit(builderBitIndex) + ";"; - (*variables)["clear_has_field_bit_builder"] = - GenerateClearBit(builderBitIndex) + ";"; (*variables)["is_field_present_message"] = GenerateGetBit(messageBitIndex); } else { + (*variables)["get_has_field_bit_message"] = ""; + (*variables)["set_has_field_bit_to_local"] = ""; (*variables)["set_has_field_bit_message"] = ""; - (*variables)["set_has_field_bit_builder"] = ""; - (*variables)["clear_has_field_bit_builder"] = ""; variables->insert({"is_field_present_message", absl::StrCat("!", (*variables)["isStringEmpty"], "(", @@ -132,10 +127,13 @@ void SetPrimitiveVariables( (*variables)["set_mutable_bit_builder"] = GenerateSetBit(builderBitIndex); (*variables)["clear_mutable_bit_builder"] = GenerateClearBit(builderBitIndex); + (*variables)["get_has_field_bit_builder"] = GenerateGetBit(builderBitIndex); (*variables)["get_has_field_bit_from_local"] = GenerateGetBitFromLocal(builderBitIndex); - (*variables)["set_has_field_bit_to_local"] = - GenerateSetBitToLocal(messageBitIndex); + (*variables)["set_has_field_bit_builder"] = + GenerateSetBit(builderBitIndex) + ";"; + (*variables)["clear_has_field_bit_builder"] = + GenerateClearBit(builderBitIndex) + ";"; } } // namespace @@ -145,7 +143,10 @@ void SetPrimitiveVariables( ImmutableStringFieldGenerator::ImmutableStringFieldGenerator( const FieldDescriptor* descriptor, int messageBitIndex, int builderBitIndex, Context* context) - : descriptor_(descriptor), name_resolver_(context->GetNameResolver()) { + : descriptor_(descriptor), + message_bit_index_(messageBitIndex), + builder_bit_index_(builderBitIndex), + name_resolver_(context->GetNameResolver()) { SetPrimitiveVariables(descriptor, messageBitIndex, builderBitIndex, context->GetFieldGeneratorInfo(descriptor), name_resolver_, &variables_, context); @@ -153,13 +154,19 @@ ImmutableStringFieldGenerator::ImmutableStringFieldGenerator( ImmutableStringFieldGenerator::~ImmutableStringFieldGenerator() {} +int ImmutableStringFieldGenerator::GetMessageBitIndex() const { + return message_bit_index_; +} + +int ImmutableStringFieldGenerator::GetBuilderBitIndex() const { + return builder_bit_index_; +} + int ImmutableStringFieldGenerator::GetNumBitsForMessage() const { return HasHasbit(descriptor_) ? 1 : 0; } -int ImmutableStringFieldGenerator::GetNumBitsForBuilder() const { - return GetNumBitsForMessage(); -} +int ImmutableStringFieldGenerator::GetNumBitsForBuilder() const { return 1; } // A note about how strings are handled. This code used to just store a String // in the Message. This had two issues: @@ -211,8 +218,9 @@ void ImmutableStringFieldGenerator::GenerateInterfaceMembers( void ImmutableStringFieldGenerator::GenerateMembers( io::Printer* printer) const { - printer->Print(variables_, "@SuppressWarnings(\"serial\")\n" - "private volatile java.lang.Object $name$_;\n"); + printer->Print(variables_, + "@SuppressWarnings(\"serial\")\n" + "private volatile java.lang.Object $name$_ = $default$;\n"); PrintExtraFieldInfo(variables_, printer); if (HasHazzer(descriptor_)) { @@ -331,9 +339,9 @@ void ImmutableStringFieldGenerator::GenerateBuilderMembers( printer->Print(variables_, "$deprecation$public Builder ${$set$capitalized_name$$}$(\n" " java.lang.String value) {\n" - "$null_check$" - " $set_has_field_bit_builder$\n" + " $null_check$\n" " $name$_ = value;\n" + " $set_has_field_bit_builder$\n" " $on_changed$\n" " return this;\n" "}\n"); @@ -342,14 +350,14 @@ void ImmutableStringFieldGenerator::GenerateBuilderMembers( /* builder */ true); printer->Print( variables_, - "$deprecation$public Builder ${$clear$capitalized_name$$}$() {\n" - " $clear_has_field_bit_builder$\n"); + "$deprecation$public Builder ${$clear$capitalized_name$$}$() {\n"); printer->Annotate("{", "}", descriptor_); // The default value is not a simple literal so we want to avoid executing // it multiple times. Instead, get the default out of the default instance. printer->Print(variables_, " $name$_ = getDefaultInstance().get$capitalized_name$();\n"); printer->Print(variables_, + " $clear_has_field_bit_builder$\n" " $on_changed$\n" " return this;\n" "}\n"); @@ -360,14 +368,14 @@ void ImmutableStringFieldGenerator::GenerateBuilderMembers( variables_, "$deprecation$public Builder ${$set$capitalized_name$Bytes$}$(\n" " com.google.protobuf.ByteString value) {\n" - "$null_check$"); + " $null_check$\n"); printer->Annotate("{", "}", descriptor_); if (CheckUtf8(descriptor_)) { printer->Print(variables_, " checkByteStringIsUtf8(value);\n"); } printer->Print(variables_, - " $set_has_field_bit_builder$\n" " $name$_ = value;\n" + " $set_has_field_bit_builder$\n" " $on_changed$\n" " return this;\n" "}\n"); @@ -415,9 +423,7 @@ void ImmutableStringFieldGenerator::GenerateInitializationCode( void ImmutableStringFieldGenerator::GenerateBuilderClearCode( io::Printer* printer) const { - printer->Print(variables_, - "$name$_ = $default$;\n" - "$clear_has_field_bit_builder$\n"); + printer->Print(variables_, "$name$_ = $default$;\n"); } void ImmutableStringFieldGenerator::GenerateMergingCode( @@ -427,14 +433,15 @@ void ImmutableStringFieldGenerator::GenerateMergingCode( // all string fields to Strings when copying fields from a Message. printer->Print(variables_, "if (other.has$capitalized_name$()) {\n" - " $set_has_field_bit_builder$\n" " $name$_ = other.$name$_;\n" + " $set_has_field_bit_builder$\n" " $on_changed$\n" "}\n"); } else { printer->Print(variables_, "if (!other.get$capitalized_name$().isEmpty()) {\n" " $name$_ = other.$name$_;\n" + " $set_has_field_bit_builder$\n" " $on_changed$\n" "}\n"); } @@ -442,13 +449,13 @@ void ImmutableStringFieldGenerator::GenerateMergingCode( void ImmutableStringFieldGenerator::GenerateBuildingCode( io::Printer* printer) const { - if (HasHazzer(descriptor_)) { - printer->Print(variables_, - "if ($get_has_field_bit_from_local$) {\n" - " $set_has_field_bit_to_local$;\n" - "}\n"); + printer->Print(variables_, + "if ($get_has_field_bit_from_local$) {\n" + " result.$name$_ = $name$_;\n"); + if (GetNumBitsForMessage() > 0) { + printer->Print(variables_, " $set_has_field_bit_to_local$;\n"); } - printer->Print(variables_, "result.$name$_ = $name$_;\n"); + printer->Print("}\n"); } void ImmutableStringFieldGenerator::GenerateBuilderParsingCode( @@ -647,7 +654,7 @@ void ImmutableStringOneofFieldGenerator::GenerateBuilderMembers( printer->Print(variables_, "$deprecation$public Builder ${$set$capitalized_name$$}$(\n" " java.lang.String value) {\n" - "$null_check$" + " $null_check$\n" " $set_oneof_case_message$;\n" " $oneof_name$_ = value;\n" " $on_changed$\n" @@ -674,7 +681,7 @@ void ImmutableStringOneofFieldGenerator::GenerateBuilderMembers( variables_, "$deprecation$public Builder ${$set$capitalized_name$Bytes$}$(\n" " com.google.protobuf.ByteString value) {\n" - "$null_check$"); + " $null_check$\n"); printer->Annotate("{", "}", descriptor_); if (CheckUtf8(descriptor_)) { printer->Print(variables_, " checkByteStringIsUtf8(value);\n"); @@ -704,10 +711,7 @@ void ImmutableStringOneofFieldGenerator::GenerateMergingCode( void ImmutableStringOneofFieldGenerator::GenerateBuildingCode( io::Printer* printer) const { - printer->Print(variables_, - "if ($has_oneof_case_message$) {\n" - " result.$oneof_name$_ = $oneof_name$_;\n" - "}\n"); + // No-Op: oneof fields are built by a single statement } void ImmutableStringOneofFieldGenerator::GenerateBuilderParsingCode( @@ -746,11 +750,8 @@ void ImmutableStringOneofFieldGenerator::GenerateSerializedSizeCode( RepeatedImmutableStringFieldGenerator::RepeatedImmutableStringFieldGenerator( const FieldDescriptor* descriptor, int messageBitIndex, int builderBitIndex, Context* context) - : descriptor_(descriptor), name_resolver_(context->GetNameResolver()) { - SetPrimitiveVariables(descriptor, messageBitIndex, builderBitIndex, - context->GetFieldGeneratorInfo(descriptor), - name_resolver_, &variables_, context); -} + : ImmutableStringFieldGenerator(descriptor, messageBitIndex, + builderBitIndex, context) {} RepeatedImmutableStringFieldGenerator:: ~RepeatedImmutableStringFieldGenerator() {} @@ -889,7 +890,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateBuilderMembers( printer->Print(variables_, "$deprecation$public Builder ${$set$capitalized_name$$}$(\n" " int index, java.lang.String value) {\n" - "$null_check$" + " $null_check$\n" " ensure$capitalized_name$IsMutable();\n" " $name$_.set(index, value);\n" " $on_changed$\n" @@ -901,7 +902,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateBuilderMembers( printer->Print(variables_, "$deprecation$public Builder ${$add$capitalized_name$$}$(\n" " java.lang.String value) {\n" - "$null_check$" + " $null_check$\n" " ensure$capitalized_name$IsMutable();\n" " $name$_.add(value);\n" " $on_changed$\n" @@ -938,7 +939,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateBuilderMembers( variables_, "$deprecation$public Builder ${$add$capitalized_name$Bytes$}$(\n" " com.google.protobuf.ByteString value) {\n" - "$null_check$"); + " $null_check$\n"); printer->Annotate("{", "}", descriptor_); if (CheckUtf8(descriptor_)) { printer->Print(variables_, " checkByteStringIsUtf8(value);\n"); diff --git a/src/google/protobuf/compiler/java/string_field.h b/src/google/protobuf/compiler/java/string_field.h index 3d6894d20873..96f53b941a33 100644 --- a/src/google/protobuf/compiler/java/string_field.h +++ b/src/google/protobuf/compiler/java/string_field.h @@ -68,6 +68,8 @@ class ImmutableStringFieldGenerator : public ImmutableFieldGenerator { // implements ImmutableFieldGenerator // --------------------------------------- + int GetMessageBitIndex() const override; + int GetBuilderBitIndex() const override; int GetNumBitsForMessage() const override; int GetNumBitsForBuilder() const override; void GenerateInterfaceMembers(io::Printer* printer) const override; @@ -90,6 +92,8 @@ class ImmutableStringFieldGenerator : public ImmutableFieldGenerator { protected: const FieldDescriptor* descriptor_; + int message_bit_index_; + int builder_bit_index_; absl::flat_hash_map variables_; ClassNameResolver* name_resolver_; }; @@ -117,7 +121,8 @@ class ImmutableStringOneofFieldGenerator void GenerateSerializedSizeCode(io::Printer* printer) const override; }; -class RepeatedImmutableStringFieldGenerator : public ImmutableFieldGenerator { +class RepeatedImmutableStringFieldGenerator + : public ImmutableStringFieldGenerator { public: explicit RepeatedImmutableStringFieldGenerator( const FieldDescriptor* descriptor, int messageBitIndex, @@ -148,11 +153,6 @@ class RepeatedImmutableStringFieldGenerator : public ImmutableFieldGenerator { void GenerateKotlinDslMembers(io::Printer* printer) const override; std::string GetBoxedType() const override; - - private: - const FieldDescriptor* descriptor_; - absl::flat_hash_map variables_; - ClassNameResolver* name_resolver_; }; } // namespace java