Merge pull request #5430 from acozzette/fixes-for-3.7

Cherry-picked some internal fixes from Google

python/google/protobuf/pyext/map_container.cc

@@ -346,11 +346,11 @@ PyObject* MapReflectionFriend::MergeFrom(PyObject* _self, PyObject* arg) {
   const Message* other_message = other_map->message;
   const Reflection* reflection = message->GetReflection();
   const Reflection* other_reflection = other_message->GetReflection();
-  internal::MapFieldBase* field = reflection->MapData(
+  internal::MapFieldBase* field = reflection->MutableMapData(
       message, self->parent_field_descriptor);
-  internal::MapFieldBase* other_field =
-      other_reflection->MapData(const_cast<Message*>(other_message),
-                                self->parent_field_descriptor);
+  const internal::MapFieldBase* other_field =
+      other_reflection->GetMapData(*other_message,
+                                   self->parent_field_descriptor);
   field->MergeFrom(*other_field);
   self->version++;
   Py_RETURN_NONE;

src/google/protobuf/generated_message_reflection.cc

@@ -2224,7 +2224,7 @@ void* GeneratedMessageReflection::RepeatedFieldData(
   }
 }
 
-MapFieldBase* GeneratedMessageReflection::MapData(
+MapFieldBase* GeneratedMessageReflection::MutableMapData(
     Message* message, const FieldDescriptor* field) const {
   USAGE_CHECK(IsMapFieldInApi(field),
               "GetMapData",
@@ -2232,6 +2232,14 @@ MapFieldBase* GeneratedMessageReflection::MapData(
   return MutableRaw<MapFieldBase>(message, field);
 }
 
+const MapFieldBase* GeneratedMessageReflection::GetMapData(
+    const Message& message, const FieldDescriptor* field) const {
+  USAGE_CHECK(IsMapFieldInApi(field),
+              "GetMapData",
+              "Field is not a map field.");
+  return &(GetRaw<MapFieldBase>(message, field));
+}
+
 namespace {
 
 // Helper function to transform migration schema into reflection schema.

src/google/protobuf/generated_message_reflection.h

@@ -670,8 +670,11 @@ class GeneratedMessageReflection final : public Reflection {
                                       Message* sub_message,
                                       const FieldDescriptor* field) const;
 
-  internal::MapFieldBase* MapData(Message* message,
-                                  const FieldDescriptor* field) const override;
+  internal::MapFieldBase* MutableMapData(
+      Message* message, const FieldDescriptor* field) const override;
+
+  const internal::MapFieldBase* GetMapData(
+      const Message& message, const FieldDescriptor* field) const override;
 
   friend inline  // inline so nobody can call this function.
       void

src/google/protobuf/map_field.h

@@ -742,7 +742,7 @@ class PROTOBUF_EXPORT MapIterator {
  public:
   MapIterator(Message* message, const FieldDescriptor* field) {
     const Reflection* reflection = message->GetReflection();
-    map_ = reflection->MapData(message, field);
+    map_ = reflection->MutableMapData(message, field);
     key_.SetType(field->message_type()->FindFieldByName("key")->cpp_type());
     value_.SetType(field->message_type()->FindFieldByName("value")->cpp_type());
     map_->InitializeIterator(this);

src/google/protobuf/map_test.cc

@@ -2042,19 +2042,30 @@ TEST(GeneratedMapFieldTest, DynamicMessageMergeFromDynamicMessage) {
       unittest::TestMap::descriptor());
   reflection_tester.SetMapFieldsViaMapReflection(message1.get());
 
+  // message2 is created by same factory.
   std::unique_ptr<Message> message2;
   message2.reset(
       factory.GetPrototype(unittest::TestMap::descriptor())->New());
   reflection_tester.SetMapFieldsViaMapReflection(message2.get());
 
+  // message3 is created by different factory.
+  DynamicMessageFactory factory3;
+  std::unique_ptr<Message> message3;
+  message3.reset(
+      factory3.GetPrototype(unittest::TestMap::descriptor())->New());
+  reflection_tester.SetMapFieldsViaMapReflection(message3.get());
+
   message2->MergeFrom(*message1);
+  message3->MergeFrom(*message1);
 
   // Test MergeFrom does not sync to repeated fields and
   // there is no duplicate keys in text format.
-  string output1, output2;
+  string output1, output2, output3;
   TextFormat::PrintToString(*message1, &output1);
   TextFormat::PrintToString(*message2, &output2);
+  TextFormat::PrintToString(*message3, &output3);
   EXPECT_EQ(output1, output2);
+  EXPECT_EQ(output1, output3);
 }
 
 TEST(GeneratedMapFieldTest, DynamicMessageCopyFrom) {

src/google/protobuf/message.h

@@ -1049,11 +1049,16 @@ class PROTOBUF_EXPORT Reflection {
 
   // Help method for MapIterator.
   friend class MapIterator;
-  virtual internal::MapFieldBase* MapData(
+  virtual internal::MapFieldBase* MutableMapData(
       Message* /* message */, const FieldDescriptor* /* field */) const {
     return NULL;
   }
 
+  virtual const internal::MapFieldBase* GetMapData(
+      const Message& /* message */, const FieldDescriptor* /* field */) const {
+    return NULL;
+  }
+
   GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(Reflection);
 };
 

src/google/protobuf/reflection_ops.cc

@@ -77,6 +77,10 @@ void ReflectionOps::Merge(const Message& from, Message* to) {
 
   const Reflection* from_reflection = GetReflectionOrDie(from);
   const Reflection* to_reflection = GetReflectionOrDie(*to);
+  bool is_from_generated = (from_reflection->GetMessageFactory() ==
+                            google::protobuf::MessageFactory::generated_factory());
+  bool is_to_generated = (to_reflection->GetMessageFactory() ==
+                          google::protobuf::MessageFactory::generated_factory());
 
   std::vector<const FieldDescriptor*> fields;
   from_reflection->ListFields(from, &fields);
@@ -84,15 +88,17 @@ void ReflectionOps::Merge(const Message& from, Message* to) {
     const FieldDescriptor* field = fields[i];
 
     if (field->is_repeated()) {
-      if (field->is_map()) {
-        MapFieldBase* from_field =
-            from_reflection->MapData(const_cast<Message*>(&from), field);
+      // Use map reflection if both are in map status and have the
+      // same map type to avoid sync with repeated field.
+      // Note: As from and to messages have the same descriptor, the
+      // map field types are the same if they are both generated
+      // messages or both dynamic messages.
+      if (is_from_generated == is_to_generated && field->is_map()) {
+        const MapFieldBase* from_field =
+            from_reflection->GetMapData(from, field);
         MapFieldBase* to_field =
-            to_reflection->MapData(const_cast<Message*>(to), field);
-        // Use map reflection if both are in map status and have the
-        // same map type to avoid sync with repeated field.
-        if (to_field->IsMapValid() && from_field->IsMapValid()
-            && typeid(*from_field) == typeid(*to_field)) {
+            to_reflection->MutableMapData(to, field);
+        if (to_field->IsMapValid() && from_field->IsMapValid()) {
           to_field->MergeFrom(*from_field);
           continue;
         }
@@ -189,8 +195,8 @@ bool ReflectionOps::IsInitialized(const Message& message) {
       if (field->is_map()) {
         const FieldDescriptor* value_field = field->message_type()->field(1);
         if (value_field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) {
-          MapFieldBase* map_field =
-              reflection->MapData(const_cast<Message*>(&message), field);
+          const MapFieldBase* map_field =
+              reflection->GetMapData(message, field);
           if (map_field->IsMapValid()) {
             MapIterator iter(const_cast<Message*>(&message), field);
             MapIterator end(const_cast<Message*>(&message), field);

src/google/protobuf/text_format.cc

@@ -2049,7 +2049,7 @@ bool MapFieldPrinterHelper::SortMap(
     std::vector<const Message*>* sorted_map_field) {
   bool need_release = false;
   const MapFieldBase& base =
-      *reflection->MapData(const_cast<Message*>(&message), field);
+      *reflection->GetMapData(message, field);
 
   if (base.IsRepeatedFieldValid()) {
     const RepeatedPtrField<Message>& map_field =

src/google/protobuf/wire_format.cc

@@ -907,8 +907,8 @@ void WireFormat::SerializeFieldWithCachedSizes(
   // internal state and existing references that came from map reflection remain
   // valid for both reading and writing.
   if (field->is_map()) {
-    MapFieldBase* map_field =
-        message_reflection->MapData(const_cast<Message*>(&message), field);
+    const MapFieldBase* map_field =
+        message_reflection->GetMapData(message, field);
     if (map_field->IsMapValid()) {
       if (output->IsSerializationDeterministic()) {
         std::vector<MapKey> sorted_key_list =
@@ -1243,8 +1243,8 @@ size_t WireFormat::FieldDataOnlyByteSize(
   size_t data_size = 0;
 
   if (field->is_map()) {
-    MapFieldBase* map_field =
-        message_reflection->MapData(const_cast<Message*>(&message), field);
+    const MapFieldBase* map_field =
+        message_reflection->GetMapData(message, field);
     if (map_field->IsMapValid()) {
       MapIterator iter(const_cast<Message*>(&message), field);
       MapIterator end(const_cast<Message*>(&message), field);