From 1db4fdc2364ada358867d890e9e2b38d1cc197d9 Mon Sep 17 00:00:00 2001 From: Hong Shin Date: Tue, 1 Oct 2024 14:37:32 -0700 Subject: [PATCH] Introduce set_alias to hpb In certain cases, it is useful to share submessages across multiple parent messages. proto2::cpp has a mechanism for this, so we add the hpb equivalent. For this initial impl, we stipulate that the arenas must be exactly the same. We may explore broadening the constraint to allow for all fused arenas. PiperOrigin-RevId: 681169537 --- hpb/bazel/hpb_proto_library.bzl | 1 + hpb_generator/gen_accessors.cc | 18 +++++++++++++++- hpb_generator/protoc-gen-upb-protos.cc | 1 + hpb_generator/tests/set_alias.proto | 19 +++++++++++++++++ hpb_generator/tests/test_generated.cc | 29 ++++++++++++++++++++++++++ 5 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 hpb_generator/tests/set_alias.proto diff --git a/hpb/bazel/hpb_proto_library.bzl b/hpb/bazel/hpb_proto_library.bzl index 56b1a99a6e..eb26365c04 100644 --- a/hpb/bazel/hpb_proto_library.bzl +++ b/hpb/bazel/hpb_proto_library.bzl @@ -229,6 +229,7 @@ _upb_cc_proto_library_aspect = aspect( # TODO: Add dependencies for cc runtime (absl/string etc..) "//upb:generated_cpp_support__only_for_generated_code_do_not_use__i_give_permission_to_break_me", "//hpb:generated_hpb_support", + "@com_google_absl//absl/log:absl_check", "@com_google_absl//absl/strings", "@com_google_absl//absl/status:statusor", "//hpb:repeated_field", diff --git a/hpb_generator/gen_accessors.cc b/hpb_generator/gen_accessors.cc index 9422df8104..ef3f327a62 100644 --- a/hpb_generator/gen_accessors.cc +++ b/hpb_generator/gen_accessors.cc @@ -111,6 +111,13 @@ void WriteFieldAccessorsInHeader(const protobuf::Descriptor* desc, output(R"cc( $1 $2() const; $0 mutable_$2(); + /** + * Re-points submessage to the given target. + * + * REQUIRES: + * - both messages must be in the same arena. + */ + void set_alias_$2($0 target); )cc", MessagePtrConstType(field, /* const */ false), MessagePtrConstType(field, /* const */ true), @@ -262,11 +269,18 @@ void WriteAccessorsInSource(const protobuf::Descriptor* desc, Output& output) { return hpb::interop::upb::MakeHandle<$4>( (upb_Message*)($3_mutable_$5(msg_, $6)), $6); } + void $0::set_alias_$2($1 target) { + ABSL_CHECK_EQ(arena_, hpb::interop::upb::GetArena(target)); + upb_Message_SetBaseFieldMessage( + UPB_UPCAST(msg_), + upb_MiniTable_GetFieldByIndex($7::minitable(), $8), + hpb::interop::upb::GetMessage(target)); + } )cc", class_name, MessagePtrConstType(field, /* is_const */ false), resolved_field_name, MessageName(desc), MessageBaseType(field, /* maybe_const */ false), resolved_upbc_name, - arena_expression); + arena_expression, ClassName(desc), field->index()); } } } @@ -460,6 +474,8 @@ void WriteUsingAccessorsInHeader(const protobuf::Descriptor* desc, if (!read_only) { output("using $0Access::mutable_$1;\n", class_name, resolved_field_name); + output("using $0Access::set_alias_$1;\n", class_name, + resolved_field_name); } } else { output("using $0Access::$1;\n", class_name, resolved_field_name); diff --git a/hpb_generator/protoc-gen-upb-protos.cc b/hpb_generator/protoc-gen-upb-protos.cc index 1f0642aeb0..201bbee2df 100644 --- a/hpb_generator/protoc-gen-upb-protos.cc +++ b/hpb_generator/protoc-gen-upb-protos.cc @@ -198,6 +198,7 @@ void WriteSource(const protobuf::FileDescriptor* file, Output& output, output( R"cc( #include +#include "absl/log/absl_check.h" #include "absl/strings/string_view.h" #include "$0" )cc", diff --git a/hpb_generator/tests/set_alias.proto b/hpb_generator/tests/set_alias.proto new file mode 100644 index 0000000000..b55923fa60 --- /dev/null +++ b/hpb_generator/tests/set_alias.proto @@ -0,0 +1,19 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2024 Google LLC. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd + +syntax = "proto2"; + +package hpb_unittest; + +message Child { + optional int32 peeps = 1; +} + +message Parent { + optional int32 x = 1; + optional Child child = 2; +} diff --git a/hpb_generator/tests/test_generated.cc b/hpb_generator/tests/test_generated.cc index 292fadb888..4619b887a2 100644 --- a/hpb_generator/tests/test_generated.cc +++ b/hpb_generator/tests/test_generated.cc @@ -20,8 +20,10 @@ #include "absl/strings/string_view.h" #include "google/protobuf/compiler/hpb/tests/child_model.upb.proto.h" #include "google/protobuf/compiler/hpb/tests/no_package.upb.proto.h" +#include "google/protobuf/compiler/hpb/tests/set_alias.upb.proto.h" #include "google/protobuf/compiler/hpb/tests/test_extension.upb.proto.h" #include "google/protobuf/compiler/hpb/tests/test_model.upb.proto.h" +#include "google/protobuf/hpb/arena.h" #include "google/protobuf/hpb/backend/upb/interop.h" #include "google/protobuf/hpb/hpb.h" #include "google/protobuf/hpb/ptr.h" @@ -33,10 +35,12 @@ namespace { using ::hpb::internal::Requires; +using ::hpb_unittest::protos::Child; using ::hpb_unittest::protos::ChildModel1; using ::hpb_unittest::protos::container_ext; using ::hpb_unittest::protos::ContainerExtension; using ::hpb_unittest::protos::other_ext; +using ::hpb_unittest::protos::Parent; using ::hpb_unittest::protos::RED; using ::hpb_unittest::protos::TestEnum; using ::hpb_unittest::protos::TestModel; @@ -1244,4 +1248,29 @@ TEST(CppGeneratedCode, ClearConstMessageShouldFailForConstChild) { EXPECT_TRUE(CanCallClearMessage()); } +TEST(CppGeneratedCode, SetAlias) { + hpb::Arena arena; + auto child = hpb::CreateMessage(arena); + child.set_peeps(12); + auto parent1 = hpb::CreateMessage(arena); + auto parent2 = hpb::CreateMessage(arena); + parent1.set_alias_child(child); + parent2.set_alias_child(child); + + ASSERT_EQ(parent1.child()->peeps(), parent2.child()->peeps()); + ASSERT_EQ(hpb::interop::upb::GetMessage(parent1.child()), + hpb::interop::upb::GetMessage(parent2.child())); + auto childPtr = hpb::Ptr(child); + ASSERT_EQ(hpb::interop::upb::GetMessage(childPtr), + hpb::interop::upb::GetMessage(parent1.child())); +} + +TEST(CppGeneratedCode, SetAliasFailsForDifferentArena) { + hpb::Arena arena; + auto child = hpb::CreateMessage(arena); + hpb::Arena different_arena; + auto parent = hpb::CreateMessage(different_arena); + EXPECT_DEATH(parent.set_alias_child(child), "hpb::interop::upb::GetArena"); +} + } // namespace