From 69dbe6fd6f667a2288d21ecba71fcd3e03eeff32 Mon Sep 17 00:00:00 2001 From: Hong Shin Date: Tue, 10 Sep 2024 14:51:45 -0700 Subject: [PATCH] hpb: Migrate MoveMessage out of internal to interop PiperOrigin-RevId: 673115936 --- hpb/BUILD | 34 ------------------- hpb/backend/upb/BUILD | 20 +++++++++++ hpb/backend/upb/interop.h | 14 ++++++++ .../upb/interop_test.cc} | 10 +++--- hpb/internal.h | 25 -------------- hpb_generator/gen_messages.cc | 3 +- hpb_generator/protoc-gen-upb-protos.cc | 1 - 7 files changed, 41 insertions(+), 66 deletions(-) rename hpb/{internal_test.cc => backend/upb/interop_test.cc} (79%) delete mode 100644 hpb/internal.h diff --git a/hpb/BUILD b/hpb/BUILD index 23b6d3a7c8..30429894e0 100644 --- a/hpb/BUILD +++ b/hpb/BUILD @@ -82,53 +82,19 @@ cc_library( copts = UPB_DEFAULT_CPPOPTS, ) -cc_library( - name = "internal", - hdrs = ["internal.h"], - copts = UPB_DEFAULT_CPPOPTS, - visibility = ["//upb:friends"], - deps = [ - ":hpb", - "//upb:mem", - "//upb:message", - "//upb:mini_table", - "@com_google_absl//absl/status", - "@com_google_absl//absl/status:statusor", - "@com_google_absl//absl/strings:str_format", - ], -) - # Common support code for C++ generated code. cc_library( name = "generated_hpb_support", - hdrs = [ - "internal.h", - ], copts = UPB_DEFAULT_CPPOPTS, visibility = ["//hpb/bazel:__pkg__"], deps = [ ":hpb", - ":internal", ":repeated_field", "//upb:mem", "//upb:message", ], ) -cc_test( - name = "internal_test", - srcs = ["internal_test.cc"], - copts = UPB_DEFAULT_CPPOPTS, - deps = [ - ":internal", - "//src/google/protobuf/compiler/hpb/tests:test_model_upb_cc_proto", - "//src/google/protobuf/compiler/hpb/tests:test_model_upb_proto", - "//upb:mem", - "@com_google_googletest//:gtest", - "@com_google_googletest//:gtest_main", - ], -) - upb_cc_proto_library_copts( name = "upb_cc_proto_library_copts", copts = UPB_DEFAULT_CPPOPTS, diff --git a/hpb/backend/upb/BUILD b/hpb/backend/upb/BUILD index 1bc226dd4c..79c54c6185 100644 --- a/hpb/backend/upb/BUILD +++ b/hpb/backend/upb/BUILD @@ -5,6 +5,11 @@ # license that can be found in the LICENSE file or at # https://developers.google.com/open-source/licenses/bsd +load( + "//upb/bazel:build_defs.bzl", + "UPB_DEFAULT_CPPOPTS", +) + package(default_applicable_licenses = ["//:license"]) cc_library( @@ -35,3 +40,18 @@ cc_library( "//upb:mini_table", ], ) + +cc_test( + name = "interop_test", + srcs = ["interop_test.cc"], + copts = UPB_DEFAULT_CPPOPTS, + deps = [ + ":interop", + "//src/google/protobuf/compiler/hpb/tests:test_model_upb_cc_proto", + "//src/google/protobuf/compiler/hpb/tests:test_model_upb_proto", + "//upb:mem", + "//upb:message", + "@com_google_googletest//:gtest", + "@com_google_googletest//:gtest_main", + ], +) diff --git a/hpb/backend/upb/interop.h b/hpb/backend/upb/interop.h index 5d3c2fde18..0ef7b554c8 100644 --- a/hpb/backend/upb/interop.h +++ b/hpb/backend/upb/interop.h @@ -18,6 +18,20 @@ namespace hpb::interop::upb { +/** + * Moves ownership of a message created in a source arena. + * + * Utility function to provide a way to move ownership across languages or VMs. + * + * Warning: any minitable skew will incur arbitrary memory access. Ensuring + * minitable compatibility is the responsibility of the caller. + */ +// TODO: b/365824801 - consider rename to OwnMessage +template +T MoveMessage(upb_Message* msg, upb_Arena* arena) { + return T(msg, arena); +} + template const upb_MiniTable* GetMiniTable(const T*) { return T::minitable(); diff --git a/hpb/internal_test.cc b/hpb/backend/upb/interop_test.cc similarity index 79% rename from hpb/internal_test.cc rename to hpb/backend/upb/interop_test.cc index fbfdd0be25..9aa15c8257 100644 --- a/hpb/internal_test.cc +++ b/hpb/backend/upb/interop_test.cc @@ -5,19 +5,19 @@ // license that can be found in the LICENSE file or at // https://developers.google.com/open-source/licenses/bsd -#include "google/protobuf/hpb/internal.h" +#include "google/protobuf/hpb/backend/upb/interop.h" -#include #include #include "google/protobuf/compiler/hpb/tests/test_model.upb.h" #include "google/protobuf/compiler/hpb/tests/test_model.upb.proto.h" #include "upb/mem/arena.h" +#include "upb/message/message.h" namespace hpb::testing { namespace { using ::hpb_unittest::protos::TestModel; -TEST(CppGeneratedCode, InternalMoveMessage) { +TEST(CppGeneratedCode, InteropMoveMessage) { // Generate message (simulating message created in another VM/language) upb_Arena* source_arena = upb_Arena_New(); hpb_unittest_TestModel* message = hpb_unittest_TestModel_new(source_arena); @@ -25,8 +25,8 @@ TEST(CppGeneratedCode, InternalMoveMessage) { hpb_unittest_TestModel_set_int_value_with_default(message, 123); // Move ownership. - TestModel model = hpb::internal::MoveMessage((upb_Message*)message, - source_arena); + TestModel model = hpb::interop::upb::MoveMessage( + (upb_Message*)message, source_arena); // Now that we have moved ownership, free original arena. upb_Arena_Free(source_arena); EXPECT_EQ(model.int_value_with_default(), 123); diff --git a/hpb/internal.h b/hpb/internal.h deleted file mode 100644 index 5ff49cb5eb..0000000000 --- a/hpb/internal.h +++ /dev/null @@ -1,25 +0,0 @@ -// Protocol Buffers - Google's data interchange format -// Copyright 2023 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 - -#ifndef PROTOBUF_HPB_INTERNAL_H_ -#define PROTOBUF_HPB_INTERNAL_H_ - -#include "upb/mem/arena.h" -#include "upb/message/message.h" - -namespace hpb::internal { - -// Moves ownership of a message created in a source arena. -// -// Utility function to provide a way to move ownership across languages or VMs. -template -T MoveMessage(upb_Message* msg, upb_Arena* arena) { - return T(msg, arena); -} - -} // namespace hpb::internal -#endif diff --git a/hpb_generator/gen_messages.cc b/hpb_generator/gen_messages.cc index 3ed5f46ca0..1a367449ef 100644 --- a/hpb_generator/gen_messages.cc +++ b/hpb_generator/gen_messages.cc @@ -252,7 +252,8 @@ void WriteModelPublicDeclaration( const ::hpb::ExtensionRegistry& extension_registry, int options)); friend upb_Arena* hpb::interop::upb::GetArena<$0>($0* message); friend upb_Arena* hpb::interop::upb::GetArena<$0>(::hpb::Ptr<$0> message); - friend $0(::hpb::internal::MoveMessage<$0>(upb_Message* msg, upb_Arena* arena)); + friend $0(hpb::interop::upb::MoveMessage<$0>(upb_Message* msg, + upb_Arena* arena)); )cc", ClassName(descriptor), MessageName(descriptor), QualifiedClassName(descriptor)); diff --git a/hpb_generator/protoc-gen-upb-protos.cc b/hpb_generator/protoc-gen-upb-protos.cc index c18676bae7..3e525c1e49 100644 --- a/hpb_generator/protoc-gen-upb-protos.cc +++ b/hpb_generator/protoc-gen-upb-protos.cc @@ -134,7 +134,6 @@ void WriteHeader(const protobuf::FileDescriptor* file, Output& output, #ifndef $0_HPB_PROTO_H_ #define $0_HPB_PROTO_H_ -#include "google/protobuf/hpb/internal.h" #include "google/protobuf/hpb/repeated_field.h" #include "protos/protos.h"