Remove unnecessary has bits from proto2 Java.

Message fields in oneofs generate has bits in their parent message, but these are never used.

PiperOrigin-RevId: 514754886
pull/12103/head
Mike Kruskal 2 years ago committed by Copybara-Service
parent 7e2ccc3e2f
commit c440da9e13
  1. 127
      java/core/src/main/java/com/google/protobuf/DescriptorMessageInfoFactory.java
  2. 2
      src/google/protobuf/compiler/java/enum_field.cc
  3. 11
      src/google/protobuf/compiler/java/enum_field_lite.cc
  4. 8
      src/google/protobuf/compiler/java/helpers.h
  5. 22
      src/google/protobuf/compiler/java/message.cc
  6. 2
      src/google/protobuf/compiler/java/primitive_field.cc
  7. 10
      src/google/protobuf/compiler/java/primitive_field_lite.cc
  8. 2
      src/google/protobuf/compiler/java/string_field.cc
  9. 10
      src/google/protobuf/compiler/java/string_field_lite.cc

@ -315,80 +315,81 @@ final class DescriptorMessageInfoFactory implements MessageInfoFactory {
if (fd.getContainingOneof() != null) {
// Build a oneof member field.
builder.withField(buildOneofMember(messageType, fd, oneofState, enforceUtf8, enumVerifier));
} else {
Field field = field(messageType, fd);
int number = fd.getNumber();
FieldType type = getFieldType(fd);
continue;
}
if (fd.isMapField()) {
// Map field points to an auto-generated message entry type with the definition:
// message MapEntry {
// K key = 1;
// V value = 2;
// }
final FieldDescriptor valueField = fd.getMessageType().findFieldByNumber(2);
if (valueField.getJavaType() == Descriptors.FieldDescriptor.JavaType.ENUM) {
enumVerifier =
new Internal.EnumVerifier() {
@Override
public boolean isInRange(int number) {
return valueField.getEnumType().findValueByNumber(number) != null;
}
};
}
builder.withField(
forMapField(
field,
number,
SchemaUtil.getMapDefaultEntry(messageType, fd.getName()),
enumVerifier));
continue;
Field field = field(messageType, fd);
int number = fd.getNumber();
FieldType type = getFieldType(fd);
if (fd.isMapField()) {
// Map field points to an auto-generated message entry type with the definition:
// message MapEntry {
// K key = 1;
// V value = 2;
// }
final FieldDescriptor valueField = fd.getMessageType().findFieldByNumber(2);
if (valueField.getJavaType() == Descriptors.FieldDescriptor.JavaType.ENUM) {
enumVerifier =
new Internal.EnumVerifier() {
@Override
public boolean isInRange(int number) {
return valueField.getEnumType().findValueByNumber(number) != null;
}
};
}
builder.withField(
forMapField(
field,
number,
SchemaUtil.getMapDefaultEntry(messageType, fd.getName()),
enumVerifier));
continue;
}
if (fd.isRepeated()) {
// Repeated fields are not presence-checked.
if (enumVerifier != null) {
if (fd.isPacked()) {
builder.withField(
forPackedFieldWithEnumVerifier(
field, number, type, enumVerifier, cachedSizeField(messageType, fd)));
} else {
builder.withField(forFieldWithEnumVerifier(field, number, type, enumVerifier));
}
} else if (fd.getJavaType() == FieldDescriptor.JavaType.MESSAGE) {
if (fd.isRepeated()) {
// Repeated fields are not presence-checked.
if (enumVerifier != null) {
if (fd.isPacked()) {
builder.withField(
forRepeatedMessageField(
field, number, type, getTypeForRepeatedMessageField(messageType, fd)));
forPackedFieldWithEnumVerifier(
field, number, type, enumVerifier, cachedSizeField(messageType, fd)));
} else {
if (fd.isPacked()) {
builder.withField(
forPackedField(field, number, type, cachedSizeField(messageType, fd)));
} else {
builder.withField(forField(field, number, type, enforceUtf8));
}
builder.withField(forFieldWithEnumVerifier(field, number, type, enumVerifier));
}
continue;
}
if (bitField == null) {
// Lazy-create the next bitfield since we know it must exist.
bitField = bitField(messageType, bitFieldIndex);
}
// It's a presence-checked field.
if (fd.isRequired()) {
} else if (fd.getJavaType() == FieldDescriptor.JavaType.MESSAGE) {
builder.withField(
forProto2RequiredField(
field, number, type, bitField, presenceMask, enforceUtf8, enumVerifier));
forRepeatedMessageField(
field, number, type, getTypeForRepeatedMessageField(messageType, fd)));
} else {
builder.withField(
forProto2OptionalField(
field, number, type, bitField, presenceMask, enforceUtf8, enumVerifier));
if (fd.isPacked()) {
builder.withField(
forPackedField(field, number, type, cachedSizeField(messageType, fd)));
} else {
builder.withField(forField(field, number, type, enforceUtf8));
}
}
continue;
}
if (bitField == null) {
// Lazy-create the next bitfield since we know it must exist.
bitField = bitField(messageType, bitFieldIndex);
}
// It's a presence-checked field.
if (fd.isRequired()) {
builder.withField(
forProto2RequiredField(
field, number, type, bitField, presenceMask, enforceUtf8, enumVerifier));
} else {
builder.withField(
forProto2OptionalField(
field, number, type, bitField, presenceMask, enforceUtf8, enumVerifier));
}
// Update the presence mask for the next iteration. If the shift clears out the mask, we will
// go to the next bitField.
// Update the presence mask for the next iteration. If the shift clears out the mask, we
// will go to the next bitField.
presenceMask <<= 1;
if (presenceMask == 0) {
bitField = null;

@ -88,7 +88,7 @@ void SetEnumVariables(
? absl::StrCat("@kotlin.Deprecated(message = \"Field ",
(*variables)["name"], " is deprecated\") ")
: ""});
if (descriptor->has_presence()) {
if (HasHasbit(descriptor)) {
// For singular messages and builders, one bit is used for the hasField bit.
(*variables)["get_has_field_bit_message"] = GenerateGetBit(messageBitIndex);
// Note that these have a trailing ";".

@ -97,7 +97,7 @@ void SetEnumVariables(
: ""});
(*variables)["required"] = descriptor->is_required() ? "true" : "false";
if (descriptor->has_presence()) {
if (HasHasbit(descriptor)) {
if (!context->options().opensource_runtime) {
(*variables)["bit_field_id"] = absl::StrCat(messageBitIndex / 32);
(*variables)["bit_field_name"] = GetBitFieldNameForBit(messageBitIndex);
@ -123,11 +123,6 @@ void SetEnumVariables(
(*variables)["default"], ".getNumber()")});
}
(*variables)["get_has_field_bit_from_local"] =
GenerateGetBitFromLocal(builderBitIndex);
(*variables)["set_has_field_bit_to_local"] =
GenerateSetBitToLocal(messageBitIndex);
if (SupportUnknownEnumValue(descriptor)) {
variables->insert(
{"unknown", absl::StrCat((*variables)["type"], ".UNRECOGNIZED")});
@ -193,7 +188,7 @@ void ImmutableEnumFieldLiteGenerator::GenerateMembers(
" fieldNumber=$number$,\n"
" type=com.google.protobuf.FieldType.$annotation_field_type$,\n"
" isRequired=$required$)\n");
if (descriptor_->has_presence()) {
if (HasHasbit(descriptor_)) {
printer->Print(variables_,
"@com.google.protobuf.ProtoPresenceCheckedField(\n"
" presenceBitsId=$bit_field_id$,\n"
@ -369,7 +364,7 @@ void ImmutableEnumFieldLiteGenerator::GenerateFieldInfo(
WriteIntToUtf16CharSequence(descriptor_->number(), output);
WriteIntToUtf16CharSequence(GetExperimentalJavaFieldType(descriptor_),
output);
if (descriptor_->has_presence()) {
if (HasHasbit(descriptor_)) {
WriteIntToUtf16CharSequence(messageBitIndex_, output);
}
printer->Print(variables_, "\"$name$_\",\n");

@ -360,13 +360,7 @@ inline bool IsRealOneof(const FieldDescriptor* descriptor) {
}
inline bool HasHasbit(const FieldDescriptor* descriptor) {
// Note that currently message fields inside oneofs have hasbits. This is
// surprising, as the oneof case should avoid any need for a hasbit. But if
// you change this method to remove hasbits for oneofs, a few tests fail.
// TODO(b/124347790): remove hasbits for oneofs
return !descriptor->is_repeated() &&
(descriptor->has_optional_keyword() ||
descriptor->file()->syntax() == FileDescriptor::SYNTAX_PROTO2);
return internal::cpp::HasHasbit(descriptor);
}
// Whether generate classes expose public PARSER instances.

@ -971,18 +971,6 @@ void ImmutableMessageGenerator::GenerateIsInitialized(io::Printer* printer) {
// ===================================================================
namespace {
bool CheckHasBitsForEqualsAndHashCode(const FieldDescriptor* field) {
if (field->is_repeated()) {
return false;
}
if (HasHasbit(field)) {
return true;
}
return GetJavaType(field) == JAVATYPE_MESSAGE && !IsRealOneof(field);
}
} // namespace
void ImmutableMessageGenerator::GenerateEqualsAndHashCode(
io::Printer* printer) {
printer->Print(
@ -1011,8 +999,7 @@ void ImmutableMessageGenerator::GenerateEqualsAndHashCode(
const FieldDescriptor* field = descriptor_->field(i);
if (!IsRealOneof(field)) {
const FieldGeneratorInfo* info = context_->GetFieldGeneratorInfo(field);
bool check_has_bits = CheckHasBitsForEqualsAndHashCode(field);
if (check_has_bits) {
if (field->has_presence()) {
printer->Print(
"if (has$name$() != other.has$name$()) return false;\n"
"if (has$name$()) {\n",
@ -1020,7 +1007,7 @@ void ImmutableMessageGenerator::GenerateEqualsAndHashCode(
printer->Indent();
}
field_generators_.get(field).GenerateEqualsCode(printer);
if (check_has_bits) {
if (field->has_presence()) {
printer->Outdent();
printer->Print("}\n");
}
@ -1095,13 +1082,12 @@ void ImmutableMessageGenerator::GenerateEqualsAndHashCode(
const FieldDescriptor* field = descriptor_->field(i);
if (!IsRealOneof(field)) {
const FieldGeneratorInfo* info = context_->GetFieldGeneratorInfo(field);
bool check_has_bits = CheckHasBitsForEqualsAndHashCode(field);
if (check_has_bits) {
if (field->has_presence()) {
printer->Print("if (has$name$()) {\n", "name", info->capitalized_name);
printer->Indent();
}
field_generators_.get(field).GenerateHashCode(printer);
if (check_has_bits) {
if (field->has_presence()) {
printer->Outdent();
printer->Print("}\n");
}

@ -141,7 +141,7 @@ void SetPrimitiveVariables(
}
(*variables)["on_changed"] = "onChanged();";
if (descriptor->has_presence()) {
if (HasHasbit(descriptor)) {
// For singular messages and builders, one bit is used for the hasField bit.
(*variables)["get_has_field_bit_message"] = GenerateGetBit(messageBitIndex);
// Note that these have a trailing ";".

@ -151,7 +151,7 @@ void SetPrimitiveVariables(
(*variables)["fixed_size"] = absl::StrCat(fixed_size);
}
if (descriptor->has_presence()) {
if (HasHasbit(descriptor)) {
// For singular messages and builders, one bit is used for the hasField bit.
(*variables)["get_has_field_bit_message"] = GenerateGetBit(messageBitIndex);
@ -187,10 +187,6 @@ void SetPrimitiveVariables(
}
}
(*variables)["get_has_field_bit_from_local"] =
GenerateGetBitFromLocal(builderBitIndex);
(*variables)["set_has_field_bit_to_local"] =
GenerateSetBitToLocal(messageBitIndex);
// Annotations often use { and } variables to denote ranges.
(*variables)["{"] = "";
(*variables)["}"] = "";
@ -246,7 +242,7 @@ void ImmutablePrimitiveFieldLiteGenerator::GenerateMembers(
" fieldNumber=$number$,\n"
" type=com.google.protobuf.FieldType.$annotation_field_type$,\n"
" isRequired=$required$)\n");
if (descriptor_->has_presence()) {
if (HasHasbit(descriptor_)) {
printer->Print(variables_,
"@com.google.protobuf.ProtoPresenceCheckedField(\n"
" presenceBitsId=$bit_field_id$,\n"
@ -378,7 +374,7 @@ void ImmutablePrimitiveFieldLiteGenerator::GenerateFieldInfo(
WriteIntToUtf16CharSequence(descriptor_->number(), output);
WriteIntToUtf16CharSequence(GetExperimentalJavaFieldType(descriptor_),
output);
if (descriptor_->has_presence()) {
if (HasHasbit(descriptor_)) {
WriteIntToUtf16CharSequence(messageBitIndex_, output);
}
printer->Print(variables_, "\"$name$_\",\n");

@ -102,7 +102,7 @@ void SetPrimitiveVariables(
: ""});
(*variables)["on_changed"] = "onChanged();";
if (descriptor->has_presence()) {
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)["set_has_field_bit_to_local"] =

@ -97,7 +97,7 @@ void SetPrimitiveVariables(
(*variables)["enforce_utf8"] = CheckUtf8(descriptor) ? "true" : "false";
}
if (descriptor->has_presence()) {
if (HasHasbit(descriptor)) {
if (!context->options().opensource_runtime) {
(*variables)["bit_field_id"] = absl::StrCat(messageBitIndex / 32);
(*variables)["bit_field_name"] = GetBitFieldNameForBit(messageBitIndex);
@ -122,10 +122,6 @@ void SetPrimitiveVariables(
absl::StrCat("!", (*variables)["name"], "_.isEmpty()")});
}
(*variables)["get_has_field_bit_from_local"] =
GenerateGetBitFromLocal(builderBitIndex);
(*variables)["set_has_field_bit_to_local"] =
GenerateSetBitToLocal(messageBitIndex);
// Annotations often use { and } variables to denote text ranges.
(*variables)["{"] = "";
(*variables)["}"] = "";
@ -207,7 +203,7 @@ void ImmutableStringFieldLiteGenerator::GenerateMembers(
" type=com.google.protobuf.FieldType.$annotation_field_type$,\n"
" isRequired=$required$,\n"
" isEnforceUtf8=$enforce_utf8$)\n");
if (descriptor_->has_presence()) {
if (HasHasbit(descriptor_)) {
printer->Print(variables_,
"@com.google.protobuf.ProtoPresenceCheckedField(\n"
" presenceBitsId=$bit_field_id$,\n"
@ -377,7 +373,7 @@ void ImmutableStringFieldLiteGenerator::GenerateFieldInfo(
WriteIntToUtf16CharSequence(descriptor_->number(), output);
WriteIntToUtf16CharSequence(GetExperimentalJavaFieldType(descriptor_),
output);
if (descriptor_->has_presence()) {
if (HasHasbit(descriptor_)) {
WriteIntToUtf16CharSequence(messageBitIndex_, output);
}
printer->Print(variables_, "\"$name$_\",\n");

Loading…
Cancel
Save