From de31ae98175a4c95df009c7a18919fe0ffd3d350 Mon Sep 17 00:00:00 2001
From: Protobuf Team Bot <protobuf-github-bot@google.com>
Date: Tue, 18 Apr 2023 09:16:46 -0700
Subject: [PATCH] [Upb C++] Fix: Map setter does not follow value semantics
 when value is a message.

PiperOrigin-RevId: 525165538
---
 protos_generator/gen_accessors.cc         | 13 +++++++++----
 protos_generator/protoc-gen-upb-protos.cc |  1 +
 protos_generator/tests/test_generated.cc  |  7 ++++++-
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/protos_generator/gen_accessors.cc b/protos_generator/gen_accessors.cc
index 9f599cc8b0..8f498439e5 100644
--- a/protos_generator/gen_accessors.cc
+++ b/protos_generator/gen_accessors.cc
@@ -31,6 +31,7 @@
 #include "google/protobuf/descriptor.h"
 #include "protos_generator/gen_utils.h"
 #include "protos_generator/output.h"
+#include "upbc/common.h"
 #include "upbc/keywords.h"
 #include "upbc/names.h"
 
@@ -486,23 +487,27 @@ void WriteMapAccessorDefinitions(const protobuf::Descriptor* message,
     output(
         R"cc(
           bool $0::set_$1($2 key, $3 value) {
-            $6return $4_$8_set(msg_, $7, ($5*)value->msg(), arena_);
+            upb_Message* clone = upb_Message_DeepClone(value->msg(), &$9, arena_);
+            $6return $4_$8_set(msg_, $7, ($5*)clone, arena_);
           }
         )cc",
         class_name, resolved_field_name, CppConstType(key),
         MessagePtrConstType(val, /* is_const */ true), MessageName(message),
         MessageName(val->message_type()), optional_conversion_code,
-        converted_key_name, upbc_name);
+        converted_key_name, upbc_name,
+        ::upbc::MessageInit(val->message_type()->full_name()));
     output(
         R"cc(
           bool $0::set_$1($2 key, $3 value) {
-            $6return $4_$8_set(msg_, $7, ($5*)value->msg(), arena_);
+            upb_Message* clone = upb_Message_DeepClone(value->msg(), &$9, arena_);
+            $6return $4_$8_set(msg_, $7, ($5*)clone, arena_);
           }
         )cc",
         class_name, resolved_field_name, CppConstType(key),
         MessagePtrConstType(val, /* is_const */ false), MessageName(message),
         MessageName(val->message_type()), optional_conversion_code,
-        converted_key_name, upbc_name);
+        converted_key_name, upbc_name,
+        ::upbc::MessageInit(val->message_type()->full_name()));
     output(
         R"cc(
           absl::StatusOr<$3> $0::get_$1($2 key) {
diff --git a/protos_generator/protoc-gen-upb-protos.cc b/protos_generator/protoc-gen-upb-protos.cc
index caa71782c8..462898bff6 100644
--- a/protos_generator/protoc-gen-upb-protos.cc
+++ b/protos_generator/protoc-gen-upb-protos.cc
@@ -197,6 +197,7 @@ void WriteSource(const protobuf::FileDescriptor* file, Output& output,
       R"cc(
 #include <stddef.h>
 #include "absl/strings/string_view.h"
+#include "upb/message/copy.h"
 #include "upb/message/internal.h"
 #include "protos/protos.h"
 #include "$0"
diff --git a/protos_generator/tests/test_generated.cc b/protos_generator/tests/test_generated.cc
index dcdf460b0c..49dcae715b 100644
--- a/protos_generator/tests/test_generated.cc
+++ b/protos_generator/tests/test_generated.cc
@@ -404,16 +404,21 @@ TEST(CppGeneratedCode, RepeatedStrings) {
 TEST(CppGeneratedCode, MessageMapInt32KeyMessageValue) {
   const int key_test_value = 3;
   ::protos::Arena arena;
+  ::protos::Arena child_arena;
   auto test_model = ::protos::CreateMessage<TestModel>(arena);
   EXPECT_EQ(0, test_model.child_map_size());
   test_model.clear_child_map();
   EXPECT_EQ(0, test_model.child_map_size());
-  auto child_model1 = ::protos::CreateMessage<ChildModel1>(arena);
+  auto child_model1 = ::protos::CreateMessage<ChildModel1>(child_arena);
   child_model1.set_child_str1("abc");
   test_model.set_child_map(key_test_value, child_model1);
   auto map_result = test_model.get_child_map(key_test_value);
   EXPECT_EQ(true, map_result.ok());
   EXPECT_EQ("abc", map_result.value()->child_str1());
+  // Now mutate original child model to verify that value semantics are
+  // preserved.
+  child_model1.set_child_str1("abc V2");
+  EXPECT_EQ("abc", map_result.value()->child_str1());
   test_model.delete_child_map(key_test_value);
   auto map_result_after_delete = test_model.get_child_map(key_test_value);
   EXPECT_EQ(false, map_result_after_delete.ok());