Do not expose mutable methods in the CProxy.

PiperOrigin-RevId: 562800851

upb/protos/protos.h

@@ -102,21 +102,6 @@ class Ptr final {
   Proxy<T> p_;
 };
 
-namespace internal {
-struct PrivateAccess {
-  template <typename T>
-  static auto* GetInternalMsg(T&& message) {
-    return message->msg();
-  }
-};
-
-template <typename T>
-auto* GetInternalMsg(T&& message) {
-  return PrivateAccess::GetInternalMsg(std::forward<T>(message));
-}
-
-}  // namespace internal
-
 inline absl::string_view UpbStrToStringView(upb_StringView str) {
   return absl::string_view(str.data, str.size);
 }
@@ -163,6 +148,26 @@ absl::Status MessageEncodeError(upb_EncodeStatus status,
                                 SourceLocation loc = SourceLocation::current());
 
 namespace internal {
+struct PrivateAccess {
+  template <typename T>
+  static auto* GetInternalMsg(T&& message) {
+    return message->msg();
+  }
+  template <typename T>
+  static auto Proxy(void* p, upb_Arena* arena) {
+    return typename T::Proxy(p, arena);
+  }
+  template <typename T>
+  static auto CProxy(const void* p, upb_Arena* arena) {
+    return typename T::CProxy(p, arena);
+  }
+};
+
+template <typename T>
+auto* GetInternalMsg(T&& message) {
+  return PrivateAccess::GetInternalMsg(std::forward<T>(message));
+}
+
 template <typename T>
 T CreateMessage() {
   return T();
@@ -174,8 +179,8 @@ typename T::Proxy CreateMessageProxy(void* msg, upb_Arena* arena) {
 }
 
 template <typename T>
-typename T::CProxy CreateMessage(upb_Message* msg, upb_Arena* arena) {
+typename T::CProxy CreateMessage(const upb_Message* msg, upb_Arena* arena) {
-  return typename T::CProxy(msg, arena);
+  return PrivateAccess::CProxy<T>(msg, arena);
 }
 
 class ExtensionMiniTableProvider {
@@ -269,11 +274,11 @@ void DeepCopy(Ptr<const T> source_message, Ptr<T> target_message) {
 }
 
 template <typename T>
-typename T::Proxy CloneMessage(Ptr<T> message, upb::Arena& arena) {
+typename T::Proxy CloneMessage(Ptr<T> message, upb_Arena* arena) {
-  return typename T::Proxy(
+  return internal::PrivateAccess::Proxy<T>(
       ::protos::internal::DeepClone(internal::GetInternalMsg(message),
-                                    T::minitable(), arena.ptr()),
+                                    T::minitable(), arena),
-      arena.ptr());
+      arena);
 }
 
 template <typename T>

upb/protos_generator/gen_accessors.cc

@@ -447,18 +447,26 @@ void WriteUsingAccessorsInHeader(const protobuf::Descriptor* desc,
     // Generate hazzer (if any).
     if (field->has_presence()) {
       output("using $0Access::has_$1;\n", class_name, resolved_field_name);
-      output("using $0Access::clear_$1;\n", class_name, resolved_field_name);
+      if (!read_only) {
+        output("using $0Access::clear_$1;\n", class_name, resolved_field_name);
+      }
     }
     if (field->is_map()) {
       output(
           R"cc(
             using $0Access::$1_size;
-            using $0Access::clear_$1;
-            using $0Access::delete_$1;
             using $0Access::get_$1;
-            using $0Access::set_$1;
           )cc",
           class_name, resolved_field_name);
+      if (!read_only) {
+        output(
+            R"cc(
+              using $0Access::clear_$1;
+              using $0Access::delete_$1;
+              using $0Access::set_$1;
+            )cc",
+            class_name, resolved_field_name);
+      }
     } else if (desc->options().map_entry()) {
       // TODO(b/237399867) Implement map entry
     } else if (field->is_repeated()) {

upb/protos_generator/gen_messages.cc

@@ -283,9 +283,6 @@ void WriteModelProxyDeclaration(const protobuf::Descriptor* descriptor,
             ::protos::Ptr<$0Proxy> message);
         friend upb_Arena* ::protos::internal::GetArena<$2>($2* message);
         friend upb_Arena* ::protos::internal::GetArena<$2>(::protos::Ptr<$2> message);
-        friend $0Proxy(::protos::CloneMessage(::protos::Ptr<$2> message,
-                                              ::upb::Arena& arena));
-
         static void Rebind($0Proxy& lhs, const $0Proxy& rhs) {
           lhs.msg_ = rhs.msg_;
           lhs.arena_ = rhs.arena_;
@@ -312,7 +309,7 @@ void WriteModelCProxyDeclaration(const protobuf::Descriptor* descriptor,
       )cc",
       ClassName(descriptor), MessageName(descriptor));
 
-  WriteUsingAccessorsInHeader(descriptor, MessageClassType::kMessageProxy,
+  WriteUsingAccessorsInHeader(descriptor, MessageClassType::kMessageCProxy,
                               output);
 
   output.Indent(1);
@@ -322,9 +319,8 @@ void WriteModelCProxyDeclaration(const protobuf::Descriptor* descriptor,
         using AsNonConst = $0Proxy;
         const void* msg() const { return msg_; }
 
-        $0CProxy(void* msg, upb_Arena* arena) : internal::$0Access(($1*)msg, arena){};
+        $0CProxy(const void* msg, upb_Arena* arena)
-        friend $0::CProxy(::protos::internal::CreateMessage<$0>(
+            : internal::$0Access(($1*)msg, arena){};
-            upb_Message* msg, upb_Arena* arena));
         friend struct ::protos::internal::PrivateAccess;
         friend class RepeatedFieldProxy;
         friend class ::protos::Ptr<$0>;

upb/protos_generator/tests/test_generated.cc

@@ -46,6 +46,8 @@
 #include "protos_generator/tests/test_model.upb.proto.h"
 #include "upb/mem/arena.h"
 
+namespace {
+
 using ::protos_generator::test::protos::ChildModel1;
 using ::protos_generator::test::protos::other_ext;
 using ::protos_generator::test::protos::RED;
@@ -60,6 +62,12 @@ using ::protos_generator::test::protos::ThemeExtension;
 using ::testing::ElementsAre;
 using ::testing::HasSubstr;
 
+// C++17 port of C++20 `requires`
+template <typename... T, typename F>
+constexpr bool Requires(F) {
+  return std::is_invocable_v<F, T...>;
+}
+
 TEST(CppGeneratedCode, Constructor) { TestModel test_model; }
 
 TEST(CppGeneratedCode, MessageEnum) { EXPECT_EQ(5, TestModel_Category_IMAGES); }
@@ -963,6 +971,61 @@ TEST(CppGeneratedCode, ProxyToCProxy) {
   (void)child2;
 }
 
+TEST(CppGeneratedCode, MutableAccessorsAreHiddenInCProxy) {
+  TestModel model;
+  ::protos::Ptr<TestModel> proxy = &model;
+  ::protos::Ptr<const TestModel> cproxy = proxy;
+
+  const auto test_const_accessors = [](auto p) {
+    // We don't want to run it, just check it compiles.
+    if (false) {
+      (void)p->has_str1();
+      (void)p->str1();
+      (void)p->has_value();
+      (void)p->value();
+      (void)p->has_oneof_member1();
+      (void)p->oneof_member1();
+      (void)p->value_array();
+      (void)p->value_array_size();
+      (void)p->value_array(1);
+      (void)p->has_nested_child_1();
+      (void)p->nested_child_1();
+      (void)p->child_models();
+      (void)p->child_models_size();
+      (void)p->child_models(1);
+      (void)p->child_map_size();
+      (void)p->get_child_map(1);
+    }
+  };
+
+  test_const_accessors(proxy);
+  test_const_accessors(cproxy);
+
+  const auto test_mutable_accessors = [](auto p, bool expected) {
+    const auto r = [&](auto l) { return Requires<decltype(p)>(l) == expected; };
+    EXPECT_TRUE(r([](auto p) -> decltype(p->set_str1("")) {}));
+    EXPECT_TRUE(r([](auto p) -> decltype(p->clear_str1()) {}));
+    EXPECT_TRUE(r([](auto p) -> decltype(p->set_value(1)) {}));
+    EXPECT_TRUE(r([](auto p) -> decltype(p->clear_value()) {}));
+    EXPECT_TRUE(r([](auto p) -> decltype(p->set_oneof_member1("")) {}));
+    EXPECT_TRUE(r([](auto p) -> decltype(p->clear_oneof_member1()) {}));
+    EXPECT_TRUE(r([](auto p) -> decltype(p->mutable_nested_child_1()) {}));
+    EXPECT_TRUE(r([](auto p) -> decltype(p->clear_nested_child_1()) {}));
+    EXPECT_TRUE(r([](auto p) -> decltype(p->add_value_array(1)) {}));
+    EXPECT_TRUE(r([](auto p) -> decltype(p->mutable_value_array()) {}));
+    EXPECT_TRUE(r([](auto p) -> decltype(p->resize_value_array(1)) {}));
+    EXPECT_TRUE(r([](auto p) -> decltype(p->set_value_array(1, 1)) {}));
+    EXPECT_TRUE(r([](auto p) -> decltype(p->add_child_models()) {}));
+    EXPECT_TRUE(r([](auto p) -> decltype(p->mutable_child_models(1)) {}));
+    EXPECT_TRUE(r([](auto p) -> decltype(p->clear_child_map()) {}));
+    EXPECT_TRUE(r([](auto p) -> decltype(p->delete_child_map(1)) {}));
+    EXPECT_TRUE(r(
+        [](auto p) -> decltype(p->set_child_map(1, *p->get_child_map(1))) {}));
+  };
+  test_mutable_accessors(proxy, true);
+  test_mutable_accessors(cproxy, false);
+}
+
 bool ProxyToCProxyMethod(::protos::Ptr<const ChildModel1> child) {
   return child->child_str1() == "text in child";
 }
@@ -1067,3 +1130,5 @@ TEST(CppGeneratedCode, ClearConstMessageShouldFail) {
   ::protos::ClearMessage(model.child_model_1());
 }
 #endif
+
+}  // namespace