Skip to content

Commit

Permalink
Do not expose mutable methods in the CProxy.
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 561075614
  • Loading branch information
@protobuf-github-bot @copybara-github
protobuf-github-bot authored and copybara-github committed Aug 31, 2023
1 parent fcebbd1 commit 8f6f190
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 32 deletions.
47 changes: 26 additions & 21 deletions upb/protos/protos.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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();
Expand All @@ -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) {
return typename T::CProxy(msg, arena);
typename T::CProxy CreateMessage(const upb_Message* msg, upb_Arena* arena) {
return PrivateAccess::CProxy<T>(msg, arena);
}

class ExtensionMiniTableProvider {
Expand Down Expand Up @@ -261,11 +266,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) {
return typename T::Proxy(
typename T::Proxy CloneMessage(Ptr<T> message, upb_Arena* arena) {
return internal::PrivateAccess::Proxy<T>(
::protos::internal::DeepClone(internal::GetInternalMsg(message),
T::minitable(), arena.ptr()),
arena.ptr());
T::minitable(), arena),
arena);
}

template <typename T>
Expand Down
16 changes: 12 additions & 4 deletions upb/protos_generator/gen_accessors.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
10 changes: 3 additions & 7 deletions upb/protos_generator/gen_messages.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand All @@ -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);
Expand All @@ -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){};
friend $0::CProxy(::protos::internal::CreateMessage<$0>(
upb_Message* msg, upb_Arena* arena));
$0CProxy(const void* msg, upb_Arena* arena)
: internal::$0Access(($1*)msg, arena){};
friend struct ::protos::internal::PrivateAccess;
friend class RepeatedFieldProxy;
friend class ::protos::Ptr<$0>;
Expand Down
65 changes: 65 additions & 0 deletions upb/protos_generator/tests/test_generated.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
#include "protos_generator/tests/no_package.upb.proto.h"
#include "protos_generator/tests/test_model.upb.proto.h"

namespace {

using ::protos_generator::test::protos::ChildModel1;
using ::protos_generator::test::protos::other_ext;
using ::protos_generator::test::protos::RED;
Expand All @@ -59,6 +61,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); }
Expand Down Expand Up @@ -929,6 +937,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";
}
Expand Down Expand Up @@ -1033,3 +1096,5 @@ TEST(CppGeneratedCode, ClearConstMessageShouldFail) {
::protos::ClearMessage(model.child_model_1());
}
#endif

} // namespace

0 comments on commit 8f6f190

Please sign in to comment.