From 7824085416962f7d59f4dcb111d99e81364f2ddb Mon Sep 17 00:00:00 2001 From: Hong Shin Date: Wed, 16 Oct 2024 05:45:55 -0700 Subject: [PATCH] hpb: add support for retrieving mutable handles on returned map values Before this CL, it was only possible to get CHandles when fetching messages from a map. proto2::cpp does not have this restriction, so we patch this gap in here. +utilize the new Emit :) PiperOrigin-RevId: 686475508 --- cmake/dependencies.cmake | 34 --------------- hpb_generator/gen_accessors.cc | 60 ++++++++++++++++++++------- hpb_generator/tests/test_generated.cc | 17 ++++++++ 3 files changed, 61 insertions(+), 50 deletions(-) delete mode 100644 cmake/dependencies.cmake diff --git a/cmake/dependencies.cmake b/cmake/dependencies.cmake deleted file mode 100644 index e806f5dd8a..0000000000 --- a/cmake/dependencies.cmake +++ /dev/null @@ -1,34 +0,0 @@ -# Auto-generated by @//cmake:make_dependencies -# -# This file contains lists of external dependencies based on our Bazel -# config. It should be included from a hand-written CMake file that uses -# them. -# -# Changes to this file will be overwritten based on Bazel definitions. - -if(${CMAKE_VERSION} VERSION_GREATER 3.10 OR ${CMAKE_VERSION} VERSION_EQUAL 3.10) - include_guard() -endif() - -set(abseil-cpp-version "20230802.1") -set(bazel_skylib-version "1.7.0") -set(jsoncpp-version "1.9.5") -set(rules_cc-version "0.0.13") -set(rules_fuzzing-version "0.5.2") -set(rules_java-version "7.11.1") -set(rules_jvm_external-version "6.3") -set(rules_kotlin-version "1.9.6") -set(rules_license-version "1.0.0") -set(rules_pkg-version "1.0.1") -set(rules_python-version "0.28.0") -set(rules_rust-version "0.51.0") -set(platforms-version "0.0.8") -set(zlib-version "1.3.1") -set(bazel_features-version "1.17.0") -set(rules_shell-version "0.2.0") -set(googletest-version "1.14.0") -set(rules_buf-version "0.3.0") -set(rules_testing-version "0.6.0") -set(rules_proto-version "4.0.0") - - diff --git a/hpb_generator/gen_accessors.cc b/hpb_generator/gen_accessors.cc index 2985a4ca24..4e5239da9f 100644 --- a/hpb_generator/gen_accessors.cc +++ b/hpb_generator/gen_accessors.cc @@ -177,17 +177,19 @@ void WriteMapFieldAccessors(const protobuf::Descriptor* desc, resolved_upbc_name); if (val->cpp_type() == protobuf::FieldDescriptor::CPPTYPE_MESSAGE) { - ctx.EmitLegacy( - R"cc( - bool set_$0($1 key, $3 value); - bool set_$0($1 key, $4 value); - bool set_alias_$0($1 key, $3 value); - bool set_alias_$0($1 key, $4 value); - absl::StatusOr<$3> get_$0($1 key); - )cc", - resolved_field_name, CppConstType(key), CppConstType(val), - MessagePtrConstType(val, /* is_const */ true), - MessagePtrConstType(val, /* is_const */ false)); + ctx.Emit({{"field_name", resolved_field_name}, + {"const_key", CppConstType(key)}, + {"const_val", CppConstType(val)}, + {"ConstPtr", MessagePtrConstType(val, true)}, + {"MutPtr", MessagePtrConstType(val, false)}}, + R"cc( + bool set_$field_name$($const_key$ key, $ConstPtr$ value); + bool set_$field_name$($const_key$ key, $MutPtr$ value); + bool set_alias_$field_name$($const_key$ key, $ConstPtr$ value); + bool set_alias_$field_name$($const_key$ key, $MutPtr$ value); + absl::StatusOr<$ConstPtr$> get_$field_name$($const_key$ key); + absl::StatusOr<$MutPtr$> get_mutable_$field_name$($const_key$ key); + )cc"); } else { ctx.EmitLegacy( R"cc( @@ -378,6 +380,31 @@ void WriteMapAccessorDefinitions(const protobuf::Descriptor* message, MessageName(val->message_type()), QualifiedClassName(val->message_type()), optional_conversion_code, converted_key_name, upbc_name); + ctx.Emit( + {{"class_name", class_name}, + {"hpb_field_name", resolved_field_name}, + {"const_key", CppConstType(key)}, + {"PtrMut", MessagePtrConstType(val, false)}, + {"upb_msg_name", MessageName(message)}, + {"return_type", MessageName(val->message_type())}, + {"proto_class", QualifiedClassName(val->message_type())}, + {"optional_conversion_code", optional_conversion_code}, + {"converted_key_name", converted_key_name}, + {"upb_field_name", upbc_name}}, + R"cc( + absl::StatusOr<$PtrMut$> $class_name$::get_mutable_$hpb_field_name$( + $const_key$ key) { + $return_type$* msg_value; + $optional_conversion_code$bool success = + $upb_msg_name$_$upb_field_name$_get(msg_, $converted_key_name$, + &msg_value); + if (success) { + return ::hpb::interop::upb::MakeHandle<$proto_class$>( + UPB_UPCAST(msg_value), arena_); + } + return absl::NotFoundError(""); + } + )cc"); ctx.EmitLegacy( R"cc( void $0::delete_$1($2 key) { $6$4_$8_delete(msg_, $7); } @@ -488,14 +515,15 @@ void WriteUsingAccessorsInHeader(const protobuf::Descriptor* desc, using $0Access::set_$1; )cc", class_name, resolved_field_name); - // only emit set_alias for maps when value is a message + // only emit set_alias and get_mutable for maps when value is a message if (field->message_type()->FindFieldByNumber(2)->cpp_type() == protobuf::FieldDescriptor::CPPTYPE_MESSAGE) { - ctx.EmitLegacy( + ctx.Emit( + {{"class_name", class_name}, {"field_name", resolved_field_name}}, R"cc( - using $0Access::set_alias_$1; - )cc", - class_name, resolved_field_name); + using $class_name$Access::get_mutable_$field_name$; + using $class_name$Access::set_alias_$field_name$; + )cc"); } } } else if (desc->options().map_entry()) { diff --git a/hpb_generator/tests/test_generated.cc b/hpb_generator/tests/test_generated.cc index a7bb538dc1..318c0c19d3 100644 --- a/hpb_generator/tests/test_generated.cc +++ b/hpb_generator/tests/test_generated.cc @@ -654,6 +654,23 @@ TEST(CppGeneratedCode, MessageMapInt32KeyMessageValue) { EXPECT_EQ(false, map_result_after_delete.ok()); } +TEST(CppGeneratedCode, MapMutableValue) { + constexpr int key = 1; + hpb::Arena arena; + auto msg = hpb::CreateMessage(arena); + auto child = hpb::CreateMessage(arena); + child.set_peeps(12); + msg.set_child_map(key, child); + auto const_map_result = msg.get_child_map(key); + EXPECT_EQ(true, const_map_result.ok()); + EXPECT_EQ(12, const_map_result.value()->peeps()); + + auto mut_map_result = msg.get_mutable_child_map(key); + EXPECT_EQ(true, mut_map_result.ok()); + mut_map_result.value()->set_peeps(9001); + EXPECT_EQ(9001, mut_map_result.value()->peeps()); +} + TEST(CppGeneratedCode, MessageMapStringKeyAndStringValue) { ::hpb::Arena arena; auto test_model = ::hpb::CreateMessage(arena);