From ab6c2563fb3b3691d76a18622cff58e7e4c373ac Mon Sep 17 00:00:00 2001 From: Hong Shin Date: Tue, 28 Oct 2025 10:01:26 -0700 Subject: [PATCH] hpb/multi: properly split up internal.h based on backend Before this delta, internal.h looked like it's backend-agnostic. It's not; it relied too heavily on upb internals. This CL sunders that illusion. The extant internal.h has been moved to hpb/backend/upb, and a dummy c++ corollary has been created in hpb/backend/cpp. PiperOrigin-RevId: 825088652 --- hpb/BUILD | 11 +++++-- hpb/backend/BUILD | 2 ++ hpb/backend/cpp/BUILD | 6 ++++ hpb/backend/cpp/internal.h | 15 +++++++++ hpb/backend/types.h | 2 ++ hpb/backend/upb/BUILD | 11 +++++++ hpb/backend/upb/internal.h | 65 ++++++++++++++++++++++++++++++++++++++ hpb/extension.h | 9 ++++-- hpb/internal/BUILD | 12 +++++-- hpb/internal/internal.h | 62 +++++------------------------------- 10 files changed, 133 insertions(+), 62 deletions(-) create mode 100644 hpb/backend/cpp/internal.h create mode 100644 hpb/backend/upb/internal.h diff --git a/hpb/BUILD b/hpb/BUILD index ad97ba856258f..d29e10fce78b6 100644 --- a/hpb/BUILD +++ b/hpb/BUILD @@ -187,14 +187,19 @@ cc_library( ":arena", ":multibackend", ":ptr", - "//hpb/backend/upb:extension", - "//hpb/backend/upb:interop", "//hpb/internal:message_lock", "//hpb/internal:template_help", "//upb/message", "//upb/mini_table", "@abseil-cpp//absl/base:core_headers", - ], + ] + select({ + "//hpb:hpb_backend_cpp": [ + ], + "//hpb:hpb_backend_upb": [ + "//hpb/backend/upb:extension", + "//hpb/backend/upb:interop", + ], + }), ) cc_library( diff --git a/hpb/backend/BUILD b/hpb/backend/BUILD index fc615dd4dc355..e29c8f9db56ca 100644 --- a/hpb/backend/BUILD +++ b/hpb/backend/BUILD @@ -18,10 +18,12 @@ cc_library( ] + select({ "//hpb:hpb_backend_cpp": [ "//hpb/backend/cpp:error", + "//hpb/backend/cpp:internal", "//src/google/protobuf:arena", ], "//hpb:hpb_backend_upb": [ "//hpb/backend/upb:error", + "//hpb/backend/upb:internal", "//upb/mem", ], }), diff --git a/hpb/backend/cpp/BUILD b/hpb/backend/cpp/BUILD index 35a94e61166e2..565fd30399078 100644 --- a/hpb/backend/cpp/BUILD +++ b/hpb/backend/cpp/BUILD @@ -44,6 +44,12 @@ cc_library( visibility = ["//hpb:__subpackages__"], ) +cc_library( + name = "internal", + hdrs = ["internal.h"], + visibility = ["//hpb:__subpackages__"], +) + cc_test( name = "cpp_test", srcs = ["cpp_test.cc"], diff --git a/hpb/backend/cpp/internal.h b/hpb/backend/cpp/internal.h new file mode 100644 index 0000000000000..88703ba9748ae --- /dev/null +++ b/hpb/backend/cpp/internal.h @@ -0,0 +1,15 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2025 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 GOOGLE_PROTOBUF_HPB_BACKEND_CPP_INTERNAL_H__ +#define GOOGLE_PROTOBUF_HPB_BACKEND_CPP_INTERNAL_H__ + +namespace hpb::internal { +struct PrivateAccess {}; +} // namespace hpb::internal + +#endif // GOOGLE_PROTOBUF_HPB_BACKEND_CPP_INTERNAL_H__ diff --git a/hpb/backend/types.h b/hpb/backend/types.h index 1bae6a88833ff..707e18602b158 100644 --- a/hpb/backend/types.h +++ b/hpb/backend/types.h @@ -11,10 +11,12 @@ #include "hpb/multibackend.h" #if HPB_INTERNAL_BACKEND == HPB_INTERNAL_BACKEND_UPB #include "hpb/backend/upb/error.h" +#include "hpb/backend/upb/internal.h" #include "upb/mem/arena.hpp" #elif HPB_INTERNAL_BACKEND == HPB_INTERNAL_BACKEND_CPP #include "google/protobuf/arena.h" #include "hpb/backend/cpp/error.h" +#include "hpb/backend/cpp/internal.h" #endif namespace hpb { diff --git a/hpb/backend/upb/BUILD b/hpb/backend/upb/BUILD index ee981eb159c37..555c360201890 100644 --- a/hpb/backend/upb/BUILD +++ b/hpb/backend/upb/BUILD @@ -43,6 +43,7 @@ cc_library( ], deps = [ "//hpb:ptr", + "//hpb/backend:types", "//hpb/internal", "//upb/base", "//upb/mem", @@ -53,6 +54,16 @@ cc_library( ], ) +cc_library( + name = "internal", + hdrs = ["internal.h"], + visibility = ["//hpb:__subpackages__"], + deps = [ + "//upb/mem", + "//upb/message", + ], +) + cc_test( name = "interop_test", srcs = ["interop_test.cc"], diff --git a/hpb/backend/upb/internal.h b/hpb/backend/upb/internal.h new file mode 100644 index 0000000000000..bacde22b0aa94 --- /dev/null +++ b/hpb/backend/upb/internal.h @@ -0,0 +1,65 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2025 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 GOOGLE_PROTOBUF_HPB_BACKEND_UPB_INTERNAL_H__ +#define GOOGLE_PROTOBUF_HPB_BACKEND_UPB_INTERNAL_H__ + +#include +#include +#include "upb/mem/arena.h" +#include "upb/message/message.h" + +namespace hpb::internal { + +struct PrivateAccess { + template + static auto* GetInternalMsg(T&& message) { + return message->msg(); + } + template + static auto* GetInternalArena(T&& message) { + return message->arena(); + } + template + static auto* GetInternalUPBArena(T&& arena) { + return arena.arena_.ptr(); + } + template + static auto Proxy(upb_Message* p, upb_Arena* arena) { + return typename T::Proxy(p, arena); + } + template + static auto CProxy(const upb_Message* p, upb_Arena* arena) { + return typename T::CProxy(p, arena); + } + template + static auto CreateMessage(upb_Arena* arena) { + return typename T::Proxy(upb_Message_New(T::minitable(), arena), arena); + } + + template + static constexpr auto InvokeConstructor(Args&&... args) { + return T(std::forward(args)...); + } + + template + static constexpr uint32_t GetExtensionNumber(const ExtensionId& id) { + return id.number(); + } + + template + static decltype(auto) GetDefaultValue(const ExtensionId& id) { + return id.default_value(); + } +}; + +template +struct AssociatedUpbTypes; + +} // namespace hpb::internal + +#endif // GOOGLE_PROTOBUF_HPB_BACKEND_UPB_INTERNAL_H__ diff --git a/hpb/extension.h b/hpb/extension.h index af1920d8a021f..675a8d4d34499 100644 --- a/hpb/extension.h +++ b/hpb/extension.h @@ -13,14 +13,17 @@ #include "absl/base/attributes.h" #include "hpb/arena.h" -#include "hpb/backend/upb/extension.h" -#include "hpb/backend/upb/interop.h" #include "hpb/internal/message_lock.h" #include "hpb/internal/template_help.h" #include "hpb/multibackend.h" #include "hpb/ptr.h" + +#if HPB_INTERNAL_BACKEND == HPB_INTERNAL_BACKEND_UPB +#include "hpb/backend/upb/extension.h" +#include "hpb/backend/upb/interop.h" #include "upb/message/accessors.h" #include "upb/mini_table/extension_registry.h" +#endif namespace hpb { // upb has a notion of an ExtensionRegistry. We expect most callers to use @@ -87,6 +90,7 @@ class ExtensionRegistry { explicit ExtensionRegistry() = default; }; +#if HPB_INTERNAL_BACKEND == HPB_INTERNAL_BACKEND_UPB template > ABSL_MUST_USE_RESULT bool HasExtension( @@ -186,6 +190,7 @@ constexpr uint32_t ExtensionNumber( const internal::ExtensionIdentifier& id) { return internal::PrivateAccess::GetExtensionNumber(id); } +#endif } // namespace hpb diff --git a/hpb/internal/BUILD b/hpb/internal/BUILD index 2c2336d161967..92645b3ca8d05 100644 --- a/hpb/internal/BUILD +++ b/hpb/internal/BUILD @@ -49,9 +49,15 @@ cc_library( "//hpb:__subpackages__", ], deps = [ - "//upb/mem", - "//upb/message", - ], + "//hpb:multibackend", + ] + select({ + "//hpb:hpb_backend_upb": [ + "//hpb/backend/upb:internal", + ], + "//hpb:hpb_backend_cpp": [ + "//hpb/backend/cpp:internal", + ], + }), ) cc_library( diff --git a/hpb/internal/internal.h b/hpb/internal/internal.h index f4be198ed4bd8..1bd620e88be57 100644 --- a/hpb/internal/internal.h +++ b/hpb/internal/internal.h @@ -8,59 +8,13 @@ #ifndef GOOGLE_PROTOBUF_HPB_INTERNAL_INTERNAL_H__ #define GOOGLE_PROTOBUF_HPB_INTERNAL_INTERNAL_H__ -#include -#include - -#include "upb/mem/arena.h" -#include "upb/message/message.h" - -namespace hpb::internal { - -struct PrivateAccess { - template - static auto* GetInternalMsg(T&& message) { - return message->msg(); - } - template - static auto* GetInternalArena(T&& message) { - return message->arena(); - } - template - static auto* GetInternalUPBArena(T&& arena) { - return arena.arena_.ptr(); - } - template - static auto Proxy(upb_Message* p, upb_Arena* arena) { - return typename T::Proxy(p, arena); - } - template - static auto CProxy(const upb_Message* p, upb_Arena* arena) { - return typename T::CProxy(p, arena); - } - template - static auto CreateMessage(upb_Arena* arena) { - return typename T::Proxy(upb_Message_New(T::minitable(), arena), arena); - } - - template - static constexpr auto InvokeConstructor(Args&&... args) { - return T(std::forward(args)...); - } - - template - static constexpr uint32_t GetExtensionNumber(const ExtensionId& id) { - return id.number(); - } - - template - static decltype(auto) GetDefaultValue(const ExtensionId& id) { - return id.default_value(); - } -}; - -template -struct AssociatedUpbTypes; - -} // namespace hpb::internal +#include "hpb/multibackend.h" +#if HPB_INTERNAL_BACKEND == HPB_INTERNAL_BACKEND_UPB +#include "hpb/backend/upb/internal.h" +#elif HPB_INTERNAL_BACKEND == HPB_INTERNAL_BACKEND_CPP +#include "hpb/backend/cpp/internal.h" +#else +#error "Unsupported hpb backend" +#endif #endif // GOOGLE_PROTOBUF_HPB_INTERNAL_INTERNAL_H__