Skip to content

Commit

Permalink
Use dedicated helper functions for map descriptors (#9099)
Browse files Browse the repository at this point in the history
The Descriptor class now has map_key() and map_value() methods for
accessing the special key and value fields in generated map entry
messages. This commit updates all the relevant code to use these
accessors instead of the clunkier FindFieldByName("key") or
FindFieldByName("value") approach.
  • Loading branch information
acozzette authored Oct 15, 2021
1 parent 6b0a1c2 commit f367bfb
Show file tree
Hide file tree
Showing 16 changed files with 54 additions and 54 deletions.
4 changes: 2 additions & 2 deletions python/google/protobuf/pyext/message.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1074,7 +1074,7 @@ int InitAttributes(CMessage* self, PyObject* args, PyObject* kwargs) {
if (descriptor->is_map()) {
ScopedPyObjectPtr map(GetFieldValue(self, descriptor));
const FieldDescriptor* value_descriptor =
descriptor->message_type()->FindFieldByName("value");
descriptor->message_type()->map_value();
if (value_descriptor->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) {
ScopedPyObjectPtr iter(PyObject_GetIter(value));
if (iter == NULL) {
Expand Down Expand Up @@ -2582,7 +2582,7 @@ PyObject* GetFieldValue(CMessage* self,
ContainerBase* py_container = nullptr;
if (field_descriptor->is_map()) {
const Descriptor* entry_type = field_descriptor->message_type();
const FieldDescriptor* value_type = entry_type->FindFieldByName("value");
const FieldDescriptor* value_type = entry_type->map_value();
if (value_type->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) {
CMessageClass* value_class = message_factory::GetMessageClass(
GetFactoryForMessage(self), value_type->message_type());
Expand Down
8 changes: 4 additions & 4 deletions src/google/protobuf/compiler/cpp/cpp_map_field.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ void SetMessageVariables(const FieldDescriptor* descriptor,
(*variables)["full_name"] = descriptor->full_name();

const FieldDescriptor* key =
descriptor->message_type()->FindFieldByName("key");
descriptor->message_type()->map_key();
const FieldDescriptor* val =
descriptor->message_type()->FindFieldByName("value");
descriptor->message_type()->map_value();
(*variables)["key_cpp"] = PrimitiveTypeName(options, key->cpp_type());
switch (val->cpp_type()) {
case FieldDescriptor::CPPTYPE_MESSAGE:
Expand Down Expand Up @@ -207,9 +207,9 @@ void MapFieldGenerator::GenerateSerializeWithCachedSizesToArray(
format("if (!this->_internal_$name$().empty()) {\n");
format.Indent();
const FieldDescriptor* key_field =
descriptor_->message_type()->FindFieldByName("key");
descriptor_->message_type()->map_key();
const FieldDescriptor* value_field =
descriptor_->message_type()->FindFieldByName("value");
descriptor_->message_type()->map_value();
const bool string_key = key_field->type() == FieldDescriptor::TYPE_STRING;
const bool string_value = value_field->type() == FieldDescriptor::TYPE_STRING;

Expand Down
4 changes: 2 additions & 2 deletions src/google/protobuf/compiler/cpp/cpp_message.cc
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,8 @@ void CollectMapInfo(const Options& options, const Descriptor* descriptor,
std::map<std::string, std::string>* variables) {
GOOGLE_CHECK(IsMapEntryMessage(descriptor));
std::map<std::string, std::string>& vars = *variables;
const FieldDescriptor* key = descriptor->FindFieldByName("key");
const FieldDescriptor* val = descriptor->FindFieldByName("value");
const FieldDescriptor* key = descriptor->map_key();
const FieldDescriptor* val = descriptor->map_value();
vars["key_cpp"] = PrimitiveTypeName(options, key->cpp_type());
switch (val->cpp_type()) {
case FieldDescriptor::CPPTYPE_MESSAGE:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,7 @@ void ParseFunctionGenerator::GenerateLengthDelim(Formatter& format,
case FieldDescriptor::TYPE_MESSAGE: {
if (field->is_map()) {
const FieldDescriptor* val =
field->message_type()->FindFieldByName("value");
field->message_type()->map_value();
GOOGLE_CHECK(val);
if (val->type() == FieldDescriptor::TYPE_ENUM &&
!HasPreservingUnknownEnumSemantics(field)) {
Expand Down
4 changes: 2 additions & 2 deletions src/google/protobuf/compiler/csharp/csharp_map_field.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ MapFieldGenerator::~MapFieldGenerator() {

void MapFieldGenerator::GenerateMembers(io::Printer* printer) {
const FieldDescriptor* key_descriptor =
descriptor_->message_type()->FindFieldByName("key");
descriptor_->message_type()->map_key();
const FieldDescriptor* value_descriptor =
descriptor_->message_type()->FindFieldByName("value");
descriptor_->message_type()->map_value();
variables_["key_type_name"] = type_name(key_descriptor);
variables_["value_type_name"] = type_name(value_descriptor);
std::unique_ptr<FieldGeneratorBase> key_generator(
Expand Down
2 changes: 1 addition & 1 deletion src/google/protobuf/compiler/java/java_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1050,7 +1050,7 @@ int GetExperimentalJavaFieldType(const FieldDescriptor* field) {
if (field->is_map()) {
if (!SupportUnknownEnumValue(field)) {
const FieldDescriptor* value =
field->message_type()->FindFieldByName("value");
field->message_type()->map_value();
if (GetJavaType(value) == JAVATYPE_ENUM) {
extra_bits |= kMapWithProto2EnumValue;
}
Expand Down
4 changes: 2 additions & 2 deletions src/google/protobuf/compiler/java/java_map_field.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ const FieldDescriptor* KeyField(const FieldDescriptor* descriptor) {
GOOGLE_CHECK_EQ(FieldDescriptor::TYPE_MESSAGE, descriptor->type());
const Descriptor* message = descriptor->message_type();
GOOGLE_CHECK(message->options().map_entry());
return message->FindFieldByName("key");
return message->map_key();
}

const FieldDescriptor* ValueField(const FieldDescriptor* descriptor) {
GOOGLE_CHECK_EQ(FieldDescriptor::TYPE_MESSAGE, descriptor->type());
const Descriptor* message = descriptor->message_type();
GOOGLE_CHECK(message->options().map_entry());
return message->FindFieldByName("value");
return message->map_value();
}

std::string TypeName(const FieldDescriptor* field,
Expand Down
4 changes: 2 additions & 2 deletions src/google/protobuf/compiler/java/java_map_field_lite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ const FieldDescriptor* KeyField(const FieldDescriptor* descriptor) {
GOOGLE_CHECK_EQ(FieldDescriptor::TYPE_MESSAGE, descriptor->type());
const Descriptor* message = descriptor->message_type();
GOOGLE_CHECK(message->options().map_entry());
return message->FindFieldByName("key");
return message->map_key();
}

const FieldDescriptor* ValueField(const FieldDescriptor* descriptor) {
GOOGLE_CHECK_EQ(FieldDescriptor::TYPE_MESSAGE, descriptor->type());
const Descriptor* message = descriptor->message_type();
GOOGLE_CHECK(message->options().map_entry());
return message->FindFieldByName("value");
return message->map_value();
}

std::string TypeName(const FieldDescriptor* field,
Expand Down
2 changes: 1 addition & 1 deletion src/google/protobuf/compiler/java/java_message.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ using internal::WireFormatLite;
namespace {
std::string MapValueImmutableClassdName(const Descriptor* descriptor,
ClassNameResolver* name_resolver) {
const FieldDescriptor* value_field = descriptor->FindFieldByName("value");
const FieldDescriptor* value_field = descriptor->map_value();
GOOGLE_CHECK_EQ(FieldDescriptor::TYPE_MESSAGE, value_field->type());
return name_resolver->GetImmutableClassName(value_field->message_type());
}
Expand Down
2 changes: 1 addition & 1 deletion src/google/protobuf/compiler/java/java_message_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ namespace java {
namespace {
std::string MapValueImmutableClassdName(const Descriptor* descriptor,
ClassNameResolver* name_resolver) {
const FieldDescriptor* value_field = descriptor->FindFieldByName("value");
const FieldDescriptor* value_field = descriptor->map_value();
GOOGLE_CHECK_EQ(FieldDescriptor::TYPE_MESSAGE, value_field->type());
return name_resolver->GetImmutableClassName(value_field->message_type());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ void MapFieldGenerator::FinishInitialization(void) {
// Use the array_comment support in RepeatedFieldGenerator to output what the
// values in the map are.
const FieldDescriptor* value_descriptor =
descriptor_->message_type()->FindFieldByName("value");
descriptor_->message_type()->map_value();
if (GetObjectiveCType(value_descriptor) == OBJECTIVECTYPE_ENUM) {
variables_["array_comment"] =
"// |" + variables_["name"] + "| values are |" + value_field_generator_->variable("storage_type") + "|\n";
Expand All @@ -164,7 +164,7 @@ void MapFieldGenerator::DetermineForwardDeclarations(
std::set<std::string>* fwd_decls) const {
RepeatedFieldGenerator::DetermineForwardDeclarations(fwd_decls);
const FieldDescriptor* value_descriptor =
descriptor_->message_type()->FindFieldByName("value");
descriptor_->message_type()->map_value();
if (GetObjectiveCType(value_descriptor) == OBJECTIVECTYPE_MESSAGE) {
const std::string& value_storage_type =
value_field_generator_->variable("storage_type");
Expand All @@ -176,7 +176,7 @@ void MapFieldGenerator::DetermineObjectiveCClassDefinitions(
std::set<std::string>* fwd_decls) const {
// Class name is already in "storage_type".
const FieldDescriptor* value_descriptor =
descriptor_->message_type()->FindFieldByName("value");
descriptor_->message_type()->map_value();
if (GetObjectiveCType(value_descriptor) == OBJECTIVECTYPE_MESSAGE) {
fwd_decls->insert(ObjCClassDeclaration(
value_field_generator_->variable("storage_type")));
Expand Down
8 changes: 4 additions & 4 deletions src/google/protobuf/compiler/php/php_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -741,8 +741,8 @@ void GenerateFieldAccessor(const FieldDescriptor* field, const Options& options,
// Type check.
if (field->is_map()) {
const Descriptor* map_entry = field->message_type();
const FieldDescriptor* key = map_entry->FindFieldByName("key");
const FieldDescriptor* value = map_entry->FindFieldByName("value");
const FieldDescriptor* key = map_entry->map_key();
const FieldDescriptor* value = map_entry->map_value();
printer->Print(
"$arr = GPBUtil::checkMapField($var, "
"\\Google\\Protobuf\\Internal\\GPBType::^key_type^, "
Expand Down Expand Up @@ -889,9 +889,9 @@ void GenerateMessageToPool(const std::string& name_prefix,
const FieldDescriptor* field = message->field(i);
if (field->is_map()) {
const FieldDescriptor* key =
field->message_type()->FindFieldByName("key");
field->message_type()->map_key();
const FieldDescriptor* val =
field->message_type()->FindFieldByName("value");
field->message_type()->map_value();
printer->Print(
"->map('^field^', \\Google\\Protobuf\\Internal\\GPBType::^key^, "
"\\Google\\Protobuf\\Internal\\GPBType::^value^, ^number^^other^)\n",
Expand Down
4 changes: 2 additions & 2 deletions src/google/protobuf/generated_message_reflection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2325,7 +2325,7 @@ bool Reflection::InsertOrLookupMapValue(Message* message,
MapValueRef* val) const {
USAGE_CHECK(IsMapFieldInApi(field), "InsertOrLookupMapValue",
"Field is not a map field.");
val->SetType(field->message_type()->FindFieldByName("value")->cpp_type());
val->SetType(field->message_type()->map_value()->cpp_type());
return MutableRaw<MapFieldBase>(message, field)
->InsertOrLookupMapValue(key, val);
}
Expand All @@ -2335,7 +2335,7 @@ bool Reflection::LookupMapValue(const Message& message,
MapValueConstRef* val) const {
USAGE_CHECK(IsMapFieldInApi(field), "LookupMapValue",
"Field is not a map field.");
val->SetType(field->message_type()->FindFieldByName("value")->cpp_type());
val->SetType(field->message_type()->map_value()->cpp_type());
return GetRaw<MapFieldBase>(message, field).LookupMapValue(key, val);
}

Expand Down
4 changes: 2 additions & 2 deletions src/google/protobuf/map_field.h
Original file line number Diff line number Diff line change
Expand Up @@ -855,8 +855,8 @@ class PROTOBUF_EXPORT MapIterator {
MapIterator(Message* message, const FieldDescriptor* field) {
const Reflection* reflection = message->GetReflection();
map_ = reflection->MutableMapData(message, field);
key_.SetType(field->message_type()->FindFieldByName("key")->cpp_type());
value_.SetType(field->message_type()->FindFieldByName("value")->cpp_type());
key_.SetType(field->message_type()->map_key()->cpp_type());
value_.SetType(field->message_type()->map_value()->cpp_type());
map_->InitializeIterator(this);
}
MapIterator(const MapIterator& other) {
Expand Down
48 changes: 24 additions & 24 deletions src/google/protobuf/map_test.inc
Original file line number Diff line number Diff line change
Expand Up @@ -1259,21 +1259,21 @@ TEST_F(MapFieldReflectionTest, RegularFields) {
desc->FindFieldByName("map_int32_foreign_message");

const FieldDescriptor* fd_map_int32_in32_key =
fd_map_int32_int32->message_type()->FindFieldByName("key");
fd_map_int32_int32->message_type()->map_key();
const FieldDescriptor* fd_map_int32_in32_value =
fd_map_int32_int32->message_type()->FindFieldByName("value");
fd_map_int32_int32->message_type()->map_value();
const FieldDescriptor* fd_map_int32_double_key =
fd_map_int32_double->message_type()->FindFieldByName("key");
fd_map_int32_double->message_type()->map_key();
const FieldDescriptor* fd_map_int32_double_value =
fd_map_int32_double->message_type()->FindFieldByName("value");
fd_map_int32_double->message_type()->map_value();
const FieldDescriptor* fd_map_string_string_key =
fd_map_string_string->message_type()->FindFieldByName("key");
fd_map_string_string->message_type()->map_key();
const FieldDescriptor* fd_map_string_string_value =
fd_map_string_string->message_type()->FindFieldByName("value");
fd_map_string_string->message_type()->map_value();
const FieldDescriptor* fd_map_int32_foreign_message_key =
fd_map_int32_foreign_message->message_type()->FindFieldByName("key");
fd_map_int32_foreign_message->message_type()->map_key();
const FieldDescriptor* fd_map_int32_foreign_message_value =
fd_map_int32_foreign_message->message_type()->FindFieldByName("value");
fd_map_int32_foreign_message->message_type()->map_value();

// Get RepeatedPtrField objects for all fields of interest.
const RepeatedPtrField<Message>& mf_int32_int32 =
Expand Down Expand Up @@ -1446,21 +1446,21 @@ TEST_F(MapFieldReflectionTest, RepeatedFieldRefForRegularFields) {
desc->FindFieldByName("map_int32_foreign_message");

const FieldDescriptor* fd_map_int32_in32_key =
fd_map_int32_int32->message_type()->FindFieldByName("key");
fd_map_int32_int32->message_type()->map_key();
const FieldDescriptor* fd_map_int32_in32_value =
fd_map_int32_int32->message_type()->FindFieldByName("value");
fd_map_int32_int32->message_type()->map_value();
const FieldDescriptor* fd_map_int32_double_key =
fd_map_int32_double->message_type()->FindFieldByName("key");
fd_map_int32_double->message_type()->map_key();
const FieldDescriptor* fd_map_int32_double_value =
fd_map_int32_double->message_type()->FindFieldByName("value");
fd_map_int32_double->message_type()->map_value();
const FieldDescriptor* fd_map_string_string_key =
fd_map_string_string->message_type()->FindFieldByName("key");
fd_map_string_string->message_type()->map_key();
const FieldDescriptor* fd_map_string_string_value =
fd_map_string_string->message_type()->FindFieldByName("value");
fd_map_string_string->message_type()->map_value();
const FieldDescriptor* fd_map_int32_foreign_message_key =
fd_map_int32_foreign_message->message_type()->FindFieldByName("key");
fd_map_int32_foreign_message->message_type()->map_key();
const FieldDescriptor* fd_map_int32_foreign_message_value =
fd_map_int32_foreign_message->message_type()->FindFieldByName("value");
fd_map_int32_foreign_message->message_type()->map_value();

// Get RepeatedFieldRef objects for all fields of interest.
const RepeatedFieldRef<Message> mf_int32_int32 =
Expand Down Expand Up @@ -2038,7 +2038,7 @@ TEST_F(MapFieldReflectionTest, MapSizeWithDuplicatedKey) {

const Reflection* entry_reflection = entry1->GetReflection();
const FieldDescriptor* key_field =
entry1->GetDescriptor()->FindFieldByName("key");
entry1->GetDescriptor()->map_key();
entry_reflection->SetInt32(entry1, key_field, 1);
entry_reflection->SetInt32(entry2, key_field, 1);

Expand All @@ -2059,7 +2059,7 @@ TEST_F(MapFieldReflectionTest, MapSizeWithDuplicatedKey) {

const Reflection* entry_reflection = entry1->GetReflection();
const FieldDescriptor* key_field =
entry1->GetDescriptor()->FindFieldByName("key");
entry1->GetDescriptor()->map_key();
entry_reflection->SetInt32(entry1, key_field, 1);
entry_reflection->SetInt32(entry2, key_field, 1);

Expand Down Expand Up @@ -2951,7 +2951,7 @@ TEST(GeneratedMapFieldReflectionTest, EmbedProto2Message) {
UNITTEST::TestMessageMap::descriptor()->FindFieldByName(
"map_int32_message");
const FieldDescriptor* value =
map_field->message_type()->FindFieldByName("value");
map_field->message_type()->map_value();

Message* entry_message =
message.GetReflection()->AddMessage(&message, map_field);
Expand All @@ -2971,9 +2971,9 @@ TEST(GeneratedMapFieldReflectionTest, MergeFromClearMapEntry) {
const FieldDescriptor* map_field =
UNITTEST::TestMap::descriptor()->FindFieldByName("map_int32_int32");
const FieldDescriptor* key =
map_field->message_type()->FindFieldByName("key");
map_field->message_type()->map_key();
const FieldDescriptor* value =
map_field->message_type()->FindFieldByName("value");
map_field->message_type()->map_value();

Message* entry_message1 =
message.GetReflection()->AddMessage(&message, map_field);
Expand Down Expand Up @@ -3005,7 +3005,7 @@ TEST(GeneratedMapFieldReflectionTest, Proto2MapEntryClear) {
const FieldDescriptor* field_descriptor =
descriptor->FindFieldByName("known_map_field");
const FieldDescriptor* value_descriptor =
field_descriptor->message_type()->FindFieldByName("value");
field_descriptor->message_type()->map_value();
Message* sub_message =
message.GetReflection()->AddMessage(&message, field_descriptor);
EXPECT_EQ(0, sub_message->GetReflection()->GetEnumValue(*sub_message,
Expand Down Expand Up @@ -3133,7 +3133,7 @@ TEST_F(MapFieldInDynamicMessageTest, MapEntryReferernceValidAfterSerialize) {
message.get(), "map_int32_foreign_message", 0);
const Reflection* map_entry_reflection = map_entry->GetReflection();
const Descriptor* map_entry_desc = map_entry->GetDescriptor();
const FieldDescriptor* value_field = map_entry_desc->FindFieldByName("value");
const FieldDescriptor* value_field = map_entry_desc->map_value();
Message* submsg =
map_entry_reflection->MutableMessage(map_entry, value_field);

Expand Down Expand Up @@ -3696,7 +3696,7 @@ TEST(TextFormatMapTest, NoDisableReflectionIterator) {
// Modify map via the iterator (invalidated in previous implementation.).
const Reflection* map_entry_reflection = iter->GetReflection();
const FieldDescriptor* value_field_desc =
iter->GetDescriptor()->FindFieldByName("value");
iter->GetDescriptor()->map_value();
map_entry_reflection->SetInt32(&(*iter), value_field_desc, 2);
GOOGLE_LOG(INFO) << iter->DebugString();

Expand Down
2 changes: 1 addition & 1 deletion src/google/protobuf/util/message_differencer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3408,7 +3408,7 @@ class LengthMapKeyComparator
const Reflection* reflection1 = message1.GetReflection();
const Reflection* reflection2 = message2.GetReflection();
const FieldDescriptor* key_field =
message1.GetDescriptor()->FindFieldByName("key");
message1.GetDescriptor()->map_key();
return reflection1->GetString(message1, key_field).size() ==
reflection2->GetString(message2, key_field).size();
}
Expand Down

0 comments on commit f367bfb

Please sign in to comment.