From 877b02506cc486e113726e43b08ebd549eeafb48 Mon Sep 17 00:00:00 2001 From: Hong Shin Date: Mon, 9 Sep 2024 13:43:56 -0700 Subject: [PATCH] =?UTF-8?q?Introduce=20hpb::interop::upb::GetMessage;=20n?= =?UTF-8?q?=C3=A9e=20protos::internal::GetInternalMsg?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PiperOrigin-RevId: 672659339 --- hpb/backend/upb/BUILD | 1 + hpb/backend/upb/interop.h | 5 ++++ hpb/backend/upb/upb.h | 2 +- hpb/hpb.h | 40 +++++++++++++-------------- hpb/internal/internal.h | 6 ---- hpb_generator/gen_messages.cc | 6 ++-- hpb_generator/tests/test_generated.cc | 14 ++++++---- protos/protos.h | 1 - 8 files changed, 38 insertions(+), 37 deletions(-) diff --git a/hpb/backend/upb/BUILD b/hpb/backend/upb/BUILD index e8397078d4..1bc226dd4c 100644 --- a/hpb/backend/upb/BUILD +++ b/hpb/backend/upb/BUILD @@ -25,6 +25,7 @@ cc_library( hdrs = ["interop.h"], visibility = [ "//hpb:__subpackages__", + "//src/google/protobuf/compiler:__subpackages__", ], deps = [ "//hpb:ptr", diff --git a/hpb/backend/upb/interop.h b/hpb/backend/upb/interop.h index 8b790b42ee..5d3c2fde18 100644 --- a/hpb/backend/upb/interop.h +++ b/hpb/backend/upb/interop.h @@ -28,6 +28,11 @@ const upb_MiniTable* GetMiniTable(Ptr) { return T::minitable(); } +template +auto* GetMessage(T&& message) { + return hpb::internal::PrivateAccess::GetInternalMsg(std::forward(message)); +} + template upb_Arena* GetArena(Ptr message) { return static_cast(message->GetInternalArena()); diff --git a/hpb/backend/upb/upb.h b/hpb/backend/upb/upb.h index 2e2c549b44..3e35f0dd2b 100644 --- a/hpb/backend/upb/upb.h +++ b/hpb/backend/upb/upb.h @@ -19,7 +19,7 @@ template void ClearMessage(hpb::internal::PtrOrRaw message) { auto ptr = Ptr(message); auto minitable = hpb::interop::upb::GetMiniTable(ptr); - upb_Message_Clear(hpb::internal::GetInternalMsg(ptr), minitable); + upb_Message_Clear(hpb::interop::upb::GetMessage(ptr), minitable); } } // namespace hpb::internal::backend::upb diff --git a/hpb/hpb.h b/hpb/hpb.h index f511d8f3ef..7c67a4f320 100644 --- a/hpb/hpb.h +++ b/hpb/hpb.h @@ -109,7 +109,7 @@ ABSL_MUST_USE_RESULT bool HasExtension( Ptr message, const ::hpb::internal::ExtensionIdentifier& id) { return ::hpb::internal::HasExtensionOrUnknown( - ::hpb::internal::GetInternalMsg(message), id.mini_table_ext()); + hpb::interop::upb::GetMessage(message), id.mini_table_ext()); } template message, const ::hpb::internal::ExtensionIdentifier& id) { static_assert(!std::is_const_v, ""); - upb_Message_ClearExtension(hpb::internal::GetInternalMsg(message), + upb_Message_ClearExtension(hpb::interop::upb::GetMessage(message), id.mini_table_ext()); } @@ -147,9 +147,9 @@ absl::Status SetExtension( const Extension& value) { static_assert(!std::is_const_v); auto* message_arena = static_cast(message->GetInternalArena()); - return ::hpb::internal::SetExtension(hpb::internal::GetInternalMsg(message), + return ::hpb::internal::SetExtension(hpb::interop::upb::GetMessage(message), message_arena, id.mini_table_ext(), - hpb::internal::GetInternalMsg(&value)); + hpb::interop::upb::GetMessage(&value)); } template value) { static_assert(!std::is_const_v); auto* message_arena = static_cast(message->GetInternalArena()); - return ::hpb::internal::SetExtension(hpb::internal::GetInternalMsg(message), + return ::hpb::internal::SetExtension(hpb::interop::upb::GetMessage(message), message_arena, id.mini_table_ext(), - hpb::internal::GetInternalMsg(value)); + hpb::interop::upb::GetMessage(value)); } template ); auto* message_arena = static_cast(message->GetInternalArena()); auto* extension_arena = static_cast(ext.GetInternalArena()); - return ::hpb::internal::MoveExtension(hpb::internal::GetInternalMsg(message), + return ::hpb::internal::MoveExtension(hpb::interop::upb::GetMessage(message), message_arena, id.mini_table_ext(), - hpb::internal::GetInternalMsg(&ext), + hpb::interop::upb::GetMessage(&ext), extension_arena); } @@ -215,7 +215,7 @@ absl::StatusOr> GetExtension( // TODO: Fix const correctness issues. upb_MessageValue value; const bool ok = ::hpb::internal::GetOrPromoteExtension( - const_cast(::hpb::internal::GetInternalMsg(message)), + const_cast(hpb::interop::upb::GetMessage(message)), id.mini_table_ext(), hpb::interop::upb::GetArena(message), &value); if (!ok) { return ExtensionNotFoundError( @@ -248,7 +248,7 @@ typename T::Proxy CreateMessage(::hpb::Arena& arena) { template typename T::Proxy CloneMessage(Ptr message, upb_Arena* arena) { return ::hpb::internal::PrivateAccess::Proxy( - ::hpb::internal::DeepClone(::hpb::internal::GetInternalMsg(message), + ::hpb::internal::DeepClone(hpb::interop::upb::GetMessage(message), T::minitable(), arena), arena); } @@ -257,8 +257,8 @@ template void DeepCopy(Ptr source_message, Ptr target_message) { static_assert(!std::is_const_v); ::hpb::internal::DeepCopy( - hpb::internal::GetInternalMsg(target_message), - hpb::internal::GetInternalMsg(source_message), T::minitable(), + hpb::interop::upb::GetMessage(target_message), + hpb::interop::upb::GetMessage(source_message), T::minitable(), static_cast(target_message->GetInternalArena())); } @@ -288,11 +288,11 @@ void ClearMessage(hpb::internal::PtrOrRaw message) { template ABSL_MUST_USE_RESULT bool Parse(Ptr message, absl::string_view bytes) { static_assert(!std::is_const_v); - upb_Message_Clear(::hpb::internal::GetInternalMsg(message), + upb_Message_Clear(hpb::interop::upb::GetMessage(message), ::hpb::interop::upb::GetMiniTable(message)); auto* arena = static_cast(message->GetInternalArena()); return upb_Decode(bytes.data(), bytes.size(), - ::hpb::internal::GetInternalMsg(message), + hpb::interop::upb::GetMessage(message), ::hpb::interop::upb::GetMiniTable(message), /* extreg= */ nullptr, /* options= */ 0, arena) == kUpb_DecodeStatus_Ok; @@ -303,11 +303,11 @@ ABSL_MUST_USE_RESULT bool Parse( Ptr message, absl::string_view bytes, const ::hpb::ExtensionRegistry& extension_registry) { static_assert(!std::is_const_v); - upb_Message_Clear(::hpb::internal::GetInternalMsg(message), + upb_Message_Clear(hpb::interop::upb::GetMessage(message), ::hpb::interop::upb::GetMiniTable(message)); auto* arena = static_cast(message->GetInternalArena()); return upb_Decode(bytes.data(), bytes.size(), - ::hpb::internal::GetInternalMsg(message), + hpb::interop::upb::GetMessage(message), ::hpb::interop::upb::GetMiniTable(message), /* extreg= */ ::hpb::internal::GetUpbExtensions(extension_registry), @@ -325,11 +325,11 @@ ABSL_MUST_USE_RESULT bool Parse( template ABSL_MUST_USE_RESULT bool Parse(T* message, absl::string_view bytes) { static_assert(!std::is_const_v); - upb_Message_Clear(::hpb::internal::GetInternalMsg(message), + upb_Message_Clear(hpb::interop::upb::GetMessage(message), ::hpb::interop::upb::GetMiniTable(message)); auto* arena = static_cast(message->GetInternalArena()); return upb_Decode(bytes.data(), bytes.size(), - ::hpb::internal::GetInternalMsg(message), + hpb::interop::upb::GetMessage(message), ::hpb::interop::upb::GetMiniTable(message), /* extreg= */ nullptr, /* options= */ 0, arena) == kUpb_DecodeStatus_Ok; @@ -369,7 +369,7 @@ absl::StatusOr Parse(absl::string_view bytes, template absl::StatusOr Serialize(const T* message, upb::Arena& arena, int options = 0) { - return ::hpb::internal::Serialize(::hpb::internal::GetInternalMsg(message), + return ::hpb::internal::Serialize(hpb::interop::upb::GetMessage(message), ::hpb::interop::upb::GetMiniTable(message), arena.ptr(), options); } @@ -377,7 +377,7 @@ absl::StatusOr Serialize(const T* message, upb::Arena& arena, template absl::StatusOr Serialize(Ptr message, upb::Arena& arena, int options = 0) { - return ::hpb::internal::Serialize(::hpb::internal::GetInternalMsg(message), + return ::hpb::internal::Serialize(hpb::interop::upb::GetMessage(message), ::hpb::interop::upb::GetMiniTable(message), arena.ptr(), options); } diff --git a/hpb/internal/internal.h b/hpb/internal/internal.h index d42f80af93..319fa87dea 100644 --- a/hpb/internal/internal.h +++ b/hpb/internal/internal.h @@ -2,7 +2,6 @@ #define GOOGLE_PROTOBUF_HPB_INTERNAL_INTERNAL_H__ #include -#include #include "upb/mem/arena.h" #include "upb/message/message.h" @@ -33,11 +32,6 @@ struct PrivateAccess { } }; -template -auto* GetInternalMsg(T&& message) { - return PrivateAccess::GetInternalMsg(std::forward(message)); -} - } // namespace hpb::internal #endif // GOOGLE_PROTOBUF_HPB_INTERNAL_INTERNAL_H__ diff --git a/hpb_generator/gen_messages.cc b/hpb_generator/gen_messages.cc index dba676647c..3ed5f46ca0 100644 --- a/hpb_generator/gen_messages.cc +++ b/hpb_generator/gen_messages.cc @@ -395,12 +395,12 @@ void WriteMessageImplementation( $0::$0(const CProxy& from) : $0Access() { arena_ = owned_arena_.ptr(); msg_ = ($1*)::hpb::internal::DeepClone( - ::hpb::internal::GetInternalMsg(&from), &$2, arena_); + ::hpb::interop::upb::GetMessage(&from), &$2, arena_); } $0::$0(const Proxy& from) : $0(static_cast(from)) {} internal::$0CProxy::$0CProxy($0Proxy m) : $0Access() { arena_ = m.arena_; - msg_ = ($1*)::hpb::internal::GetInternalMsg(&m); + msg_ = ($1*)::hpb::interop::upb::GetMessage(&m); } $0& $0::operator=(const $3& from) { arena_ = owned_arena_.ptr(); @@ -410,7 +410,7 @@ void WriteMessageImplementation( $0& $0::operator=(const CProxy& from) { arena_ = owned_arena_.ptr(); msg_ = ($1*)::hpb::internal::DeepClone( - ::hpb::internal::GetInternalMsg(&from), &$2, arena_); + ::hpb::interop::upb::GetMessage(&from), &$2, arena_); return *this; } )cc", diff --git a/hpb_generator/tests/test_generated.cc b/hpb_generator/tests/test_generated.cc index 1b903859df..292fadb888 100644 --- a/hpb_generator/tests/test_generated.cc +++ b/hpb_generator/tests/test_generated.cc @@ -22,7 +22,9 @@ #include "google/protobuf/compiler/hpb/tests/no_package.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/backend/upb/interop.h" #include "google/protobuf/hpb/hpb.h" +#include "google/protobuf/hpb/ptr.h" #include "google/protobuf/hpb/repeated_field.h" #include "google/protobuf/hpb/requires.h" #include "upb/mem/arena.h" @@ -711,7 +713,7 @@ TEST(CppGeneratedCode, SetExtension) { // Use a nested scope to make sure the arenas are fused correctly. ThemeExtension extension1; extension1.set_ext_name("Hello World"); - prior_message = ::hpb::internal::GetInternalMsg(&extension1); + prior_message = hpb::interop::upb::GetMessage(&extension1); EXPECT_EQ(false, ::hpb::HasExtension(&model, theme)); EXPECT_EQ(true, ::hpb::SetExtension(&model, theme, std::move(extension1)).ok()); @@ -719,7 +721,7 @@ TEST(CppGeneratedCode, SetExtension) { EXPECT_EQ(true, ::hpb::HasExtension(&model, theme)); auto ext = hpb::GetExtension(&model, theme); EXPECT_TRUE(ext.ok()); - EXPECT_EQ(::hpb::internal::GetInternalMsg(*ext), prior_message); + EXPECT_EQ(hpb::interop::upb::GetMessage(*ext), prior_message); } TEST(CppGeneratedCode, SetExtensionWithPtr) { @@ -732,7 +734,7 @@ TEST(CppGeneratedCode, SetExtensionWithPtr) { ::hpb::Ptr extension1 = ::hpb::CreateMessage(arena); extension1->set_ext_name("Hello World"); - prior_message = ::hpb::internal::GetInternalMsg(extension1); + prior_message = hpb::interop::upb::GetMessage(extension1); EXPECT_EQ(false, ::hpb::HasExtension(model, theme)); auto res = ::hpb::SetExtension(model, theme, extension1); EXPECT_EQ(true, res.ok()); @@ -740,7 +742,7 @@ TEST(CppGeneratedCode, SetExtensionWithPtr) { EXPECT_EQ(true, ::hpb::HasExtension(model, theme)); auto ext = hpb::GetExtension(model, theme); EXPECT_TRUE(ext.ok()); - EXPECT_NE(::hpb::internal::GetInternalMsg(*ext), prior_message); + EXPECT_NE(hpb::interop::upb::GetMessage(*ext), prior_message); } #ifndef _MSC_VER @@ -774,7 +776,7 @@ TEST(CppGeneratedCode, SetExtensionWithPtrSameArena) { ::hpb::Ptr extension1 = ::hpb::CreateMessage(arena); extension1->set_ext_name("Hello World"); - prior_message = ::hpb::internal::GetInternalMsg(extension1); + prior_message = hpb::interop::upb::GetMessage(extension1); EXPECT_EQ(false, ::hpb::HasExtension(model, theme)); auto res = ::hpb::SetExtension(model, theme, extension1); EXPECT_EQ(true, res.ok()); @@ -782,7 +784,7 @@ TEST(CppGeneratedCode, SetExtensionWithPtrSameArena) { EXPECT_EQ(true, ::hpb::HasExtension(model, theme)); auto ext = hpb::GetExtension(model, theme); EXPECT_TRUE(ext.ok()); - EXPECT_NE(::hpb::internal::GetInternalMsg(*ext), prior_message); + EXPECT_NE(hpb::interop::upb::GetMessage(*ext), prior_message); } TEST(CppGeneratedCode, SetExtensionFusingFailureShouldCopy) { diff --git a/protos/protos.h b/protos/protos.h index f0932b664b..c05a08f5e1 100644 --- a/protos/protos.h +++ b/protos/protos.h @@ -9,7 +9,6 @@ #include "google/protobuf/hpb/hpb.h" namespace protos { namespace internal { -using hpb::internal::GetInternalMsg; } // namespace internal using hpb::Arena;