From de51f2cc061a90b975054dda3e4101144b067600 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Tue, 17 Sep 2024 09:39:42 -0700 Subject: [PATCH] Prepare for `google::protobuf::MapKey` to no longer own string keys PiperOrigin-RevId: 675601391 --- python/google/protobuf/pyext/map_container.cc | 23 +++++++++++-------- src/google/protobuf/map_field.cc | 4 +++- src/google/protobuf/map_field.h | 4 ++-- src/google/protobuf/reflection_tester.cc | 12 +++++++--- 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/python/google/protobuf/pyext/map_container.cc b/python/google/protobuf/pyext/map_container.cc index 8a1cf1a48e..90c01228cc 100644 --- a/python/google/protobuf/pyext/map_container.cc +++ b/python/google/protobuf/pyext/map_container.cc @@ -96,7 +96,8 @@ static bool PyStringToSTL(PyObject* py_string, std::string* stl_string) { } } -static bool PythonToMapKey(MapContainer* self, PyObject* obj, MapKey* key) { +static bool PythonToMapKey(MapContainer* self, PyObject* obj, MapKey* key, + std::string* key_string) { const FieldDescriptor* field_descriptor = self->parent_field_descriptor->message_type()->map_key(); switch (field_descriptor->cpp_type()) { @@ -126,11 +127,10 @@ static bool PythonToMapKey(MapContainer* self, PyObject* obj, MapKey* key) { break; } case FieldDescriptor::CPPTYPE_STRING: { - std::string str; - if (!PyStringToSTL(CheckString(obj, field_descriptor), &str)) { + if (!PyStringToSTL(CheckString(obj, field_descriptor), key_string)) { return false; } - key->SetStringValue(str); + key->SetStringValue(*key_string); break; } default: @@ -334,9 +334,10 @@ PyObject* MapReflectionFriend::Contains(PyObject* _self, PyObject* key) { const Message* message = self->parent->message; const Reflection* reflection = message->GetReflection(); + std::string map_key_string; MapKey map_key; - if (!PythonToMapKey(self, key, &map_key)) { + if (!PythonToMapKey(self, key, &map_key, &map_key_string)) { return nullptr; } @@ -379,10 +380,11 @@ PyObject* MapReflectionFriend::ScalarMapGetItem(PyObject* _self, Message* message = self->GetMutableMessage(); const Reflection* reflection = message->GetReflection(); + std::string map_key_string; MapKey map_key; MapValueRef value; - if (!PythonToMapKey(self, key, &map_key)) { + if (!PythonToMapKey(self, key, &map_key, &map_key_string)) { return nullptr; } @@ -400,10 +402,11 @@ int MapReflectionFriend::ScalarMapSetItem(PyObject* _self, PyObject* key, Message* message = self->GetMutableMessage(); const Reflection* reflection = message->GetReflection(); + std::string map_key_string; MapKey map_key; MapValueRef value; - if (!PythonToMapKey(self, key, &map_key)) { + if (!PythonToMapKey(self, key, &map_key, &map_key_string)) { return -1; } @@ -593,12 +596,13 @@ int MapReflectionFriend::MessageMapSetItem(PyObject* _self, PyObject* key, MessageMapContainer* self = GetMessageMap(_self); Message* message = self->GetMutableMessage(); const Reflection* reflection = message->GetReflection(); + std::string map_key_string; MapKey map_key; MapValueRef value; self->version++; - if (!PythonToMapKey(self, key, &map_key)) { + if (!PythonToMapKey(self, key, &map_key, &map_key_string)) { return -1; } @@ -635,10 +639,11 @@ PyObject* MapReflectionFriend::MessageMapGetItem(PyObject* _self, Message* message = self->GetMutableMessage(); const Reflection* reflection = message->GetReflection(); + std::string map_key_string; MapKey map_key; MapValueRef value; - if (!PythonToMapKey(self, key, &map_key)) { + if (!PythonToMapKey(self, key, &map_key, &map_key_string)) { return nullptr; } diff --git a/src/google/protobuf/map_field.cc b/src/google/protobuf/map_field.cc index dccef67d8a..359026adf2 100644 --- a/src/google/protobuf/map_field.cc +++ b/src/google/protobuf/map_field.cc @@ -338,10 +338,12 @@ void MapFieldBase::SyncMapWithRepeatedFieldNoLock() { for (const Message& elem : rep) { // MapKey type will be set later. + Reflection::ScratchSpace map_key_scratch_space; MapKey map_key; switch (key_des->cpp_type()) { case FieldDescriptor::CPPTYPE_STRING: - map_key.SetStringValue(reflection->GetString(elem, key_des)); + map_key.SetStringValue( + reflection->GetStringView(elem, key_des, map_key_scratch_space)); break; case FieldDescriptor::CPPTYPE_INT64: map_key.SetInt64Value(reflection->GetInt64(elem, key_des)); diff --git a/src/google/protobuf/map_field.h b/src/google/protobuf/map_field.h index c40070072d..40d6ad0062 100644 --- a/src/google/protobuf/map_field.h +++ b/src/google/protobuf/map_field.h @@ -112,9 +112,9 @@ class PROTOBUF_EXPORT MapKey { SetType(FieldDescriptor::CPPTYPE_BOOL); val_.bool_value = value; } - void SetStringValue(std::string val) { + void SetStringValue(absl::string_view val) { SetType(FieldDescriptor::CPPTYPE_STRING); - *val_.string_value.get_mutable() = std::move(val); + val_.string_value.get_mutable()->assign(val.data(), val.size()); } int64_t GetInt64Value() const { diff --git a/src/google/protobuf/reflection_tester.cc b/src/google/protobuf/reflection_tester.cc index 259f0fb713..e7ed260c72 100644 --- a/src/google/protobuf/reflection_tester.cc +++ b/src/google/protobuf/reflection_tester.cc @@ -7,6 +7,7 @@ #include "google/protobuf/reflection_tester.h" #include +#include #include #include "absl/container/flat_hash_map.h" @@ -402,6 +403,7 @@ void MapReflectionTester::SetMapFieldsViaMapReflection(Message* message) { MapValueConstRef map_val_const; // Add first element. + std::string map_key_string; MapKey map_key; map_key.SetInt32Value(0); EXPECT_FALSE(reflection->LookupMapValue(*message, F("map_int32_int32"), @@ -476,7 +478,8 @@ void MapReflectionTester::SetMapFieldsViaMapReflection(Message* message) { map_key, &map_val)); map_val.SetBoolValue(false); - map_key.SetStringValue(long_string()); + map_key_string = long_string(); + map_key.SetStringValue(map_key_string); EXPECT_FALSE(reflection->LookupMapValue(*message, F("map_string_string"), map_key, &map_val_const)); EXPECT_TRUE(reflection->InsertOrLookupMapValue( @@ -576,7 +579,8 @@ void MapReflectionTester::SetMapFieldsViaMapReflection(Message* message) { &map_val); map_val.SetBoolValue(true); - map_key.SetStringValue(long_string_2()); + map_key_string = long_string_2(); + map_key.SetStringValue(map_key_string); reflection->InsertOrLookupMapValue(message, F("map_string_string"), map_key, &map_val); map_val.SetStringValue(long_string_2()); @@ -659,6 +663,7 @@ void MapReflectionTester::ModifyMapFieldsViaReflection(Message* message) { Message* sub_foreign_message; // Modify the second element + std::string map_key_string; MapKey map_key; map_key.SetInt32Value(1); EXPECT_FALSE(reflection->InsertOrLookupMapValue(message, F("map_int32_int32"), @@ -725,7 +730,8 @@ void MapReflectionTester::ModifyMapFieldsViaReflection(Message* message) { &map_val); map_val.SetBoolValue(false); - map_key.SetStringValue(long_string_2()); + map_key_string = long_string_2(); + map_key.SetStringValue(map_key_string); reflection->InsertOrLookupMapValue(message, F("map_string_string"), map_key, &map_val); map_val.SetStringValue("2");