Skip to content

Commit 68a2efb

Browse files
honglookercopybara-github
authored andcommitted
hpb: introduce hpb::StatusOr<T> parse that takes in hpb::ParseOptions
This CL deprecates the hpb::Parse overload that returns absl::StatusOr<T>. Instead, prefer hpb::Parse -> hpb::StatusOr<T>. String aliasing via upb is allow plumbed through with this CL. PiperOrigin-RevId: 808732870
1 parent 6f9705c commit 68a2efb

File tree

8 files changed

+150
-0
lines changed

8 files changed

+150
-0
lines changed

hpb/BUILD

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ cc_library(
4949
":arena",
5050
":extension",
5151
":multibackend",
52+
":options",
5253
":ptr",
5354
":status",
5455
"//hpb/internal",
@@ -194,3 +195,13 @@ cc_library(
194195
"@abseil-cpp//absl/base:core_headers",
195196
],
196197
)
198+
199+
cc_library(
200+
name = "options",
201+
hdrs = ["options.h"],
202+
visibility = ["//visibility:public"],
203+
deps = [
204+
"//hpb:extension",
205+
"//hpb:multibackend",
206+
],
207+
)

hpb/backend/upb/BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@ cc_library(
1818
":interop",
1919
"//hpb:arena",
2020
"//hpb:extension",
21+
"//hpb:options",
2122
"//hpb:ptr",
2223
"//hpb:status",
24+
"//hpb/backend:types",
2325
"//hpb/internal",
2426
"//hpb/internal:message_lock",
2527
"//hpb/internal:template_help",

hpb/backend/upb/upb.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,13 @@
1313
#include "absl/status/statusor.h"
1414
#include "absl/strings/string_view.h"
1515
#include "hpb/arena.h"
16+
#include "hpb/backend/types.h"
1617
#include "hpb/backend/upb/interop.h"
1718
#include "hpb/extension.h"
1819
#include "hpb/internal/internal.h"
1920
#include "hpb/internal/message_lock.h"
2021
#include "hpb/internal/template_help.h"
22+
#include "hpb/options.h"
2123
#include "hpb/ptr.h"
2224
#include "hpb/status.h"
2325
#include "upb/mem/arena.h"
@@ -106,6 +108,24 @@ absl::StatusOr<T> Parse(absl::string_view bytes,
106108
return MessageDecodeError(status);
107109
}
108110

111+
template <typename T>
112+
hpb::StatusOr<T> Parse(absl::string_view bytes, ParseOptions options) {
113+
int flags = 0;
114+
if (options.alias_string) {
115+
flags |= kUpb_DecodeOption_AliasString;
116+
}
117+
T message;
118+
auto* arena = interop::upb::GetArena(&message);
119+
upb_DecodeStatus status = upb_Decode(
120+
bytes.data(), bytes.size(), interop::upb::GetMessage(&message),
121+
interop::upb::GetMiniTable(&message),
122+
internal::GetUpbExtensions(options.extension_registry), flags, arena);
123+
if (status == kUpb_DecodeStatus_Ok) {
124+
return hpb::StatusOr<T>(std::move(message));
125+
}
126+
return hpb::StatusOr<T>(internal::backend::Error(status));
127+
}
128+
109129
} // namespace hpb::internal::backend::upb
110130

111131
#include "upb/port/undef.inc"

hpb/hpb.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@
1717
#include "hpb/extension.h"
1818
#include "hpb/internal/template_help.h"
1919
#include "hpb/multibackend.h"
20+
#include "hpb/options.h"
2021
#include "hpb/ptr.h"
22+
#include "hpb/status.h"
2123

2224
#if HPB_INTERNAL_BACKEND == HPB_INTERNAL_BACKEND_UPB
2325
#include "hpb/backend/upb/upb.h"
@@ -70,7 +72,16 @@ ABSL_MUST_USE_RESULT bool Parse(internal::PtrOrRaw<T> message,
7072
return backend::Parse(message, bytes, extension_registry);
7173
}
7274

75+
// Note that the default extension registry is the the generated registry.
7376
template <typename T>
77+
hpb::StatusOr<T> Parse(absl::string_view bytes, ParseOptions options) {
78+
return backend::Parse<T>(bytes, options);
79+
}
80+
81+
// Deprecated. Use the overload that returns hpb::StatusOr<T> instead.
82+
// Note that the default extension registry is the empty registry.
83+
template <typename T>
84+
ABSL_DEPRECATED("Prefer the overload that returns hpb::StatusOr<T>")
7485
absl::StatusOr<T> Parse(absl::string_view bytes,
7586
const ExtensionRegistry& extension_registry =
7687
ExtensionRegistry::empty_registry()) {

hpb/options.h

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Protocol Buffers - Google's data interchange format
2+
// Copyright 2025 Google LLC. All rights reserved.
3+
//
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file or at
6+
// https://developers.google.com/open-source/licenses/bsd
7+
8+
#ifndef GOOGLE_PROTOBUF_HPB_OPTIONS_H__
9+
#define GOOGLE_PROTOBUF_HPB_OPTIONS_H__
10+
11+
#include "hpb/extension.h"
12+
#include "hpb/multibackend.h"
13+
14+
namespace hpb {
15+
16+
struct ParseOptions {
17+
// If true, the parsed proto may alias the input string instead of copying.
18+
// Aliased data could include string fields, unknown fields, and possibly
19+
// other data.
20+
//
21+
// REQUIRES: the input string outlives the resulting proto.
22+
bool alias_string = false;
23+
24+
#if HPB_INTERNAL_BACKEND == HPB_INTERNAL_BACKEND_UPB
25+
// For the upb backend, the user can determine which extension registry
26+
// they wish to use. Unless there are compelling reasons to do otherwise,
27+
// we recommend using the generated registry, which uses linker arrays
28+
// and intelligently performs tree shaking when possible.
29+
const ExtensionRegistry& extension_registry =
30+
ExtensionRegistry::generated_registry();
31+
#endif
32+
};
33+
34+
inline ParseOptions ParseOptionsDefault() { return ParseOptions(); }
35+
36+
#if HPB_INTERNAL_BACKEND == HPB_INTERNAL_BACKEND_UPB
37+
// The empty registry is provided here as a convenience for extant users.
38+
// Prefer the generated registry.
39+
inline ParseOptions ParseOptionsWithEmptyRegistry() {
40+
ParseOptions options{
41+
.alias_string = false,
42+
.extension_registry = ExtensionRegistry::empty_registry(),
43+
};
44+
return options;
45+
}
46+
#endif
47+
48+
} // namespace hpb
49+
50+
#endif // GOOGLE_PROTOBUF_HPB_OPTIONS_H__

hpb_generator/tests/BUILD

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,11 @@ cc_test(
163163
"//hpb/backend/upb:interop",
164164
"//hpb:arena",
165165
"//hpb:extension",
166+
"//hpb:options",
166167
"//hpb:requires",
167168
"//hpb:ptr",
168169
"//hpb:repeated_field",
170+
"//hpb:status",
169171
"//upb/mem",
170172
"//upb/mini_table",
171173
],
@@ -180,7 +182,9 @@ cc_test(
180182
"//hpb",
181183
"//hpb:arena",
182184
"//hpb:extension",
185+
"//hpb:options",
183186
"//hpb:requires",
187+
"//hpb:status",
184188
"//hpb/backend/upb:interop",
185189
"//upb/mem",
186190
"@abseil-cpp//absl/status:status_matchers",

hpb_generator/tests/extension_test.cc

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@
2020
#include "hpb/arena.h"
2121
#include "hpb/backend/upb/interop.h"
2222
#include "hpb/hpb.h"
23+
#include "hpb/options.h"
2324
#include "hpb/requires.h"
25+
#include "hpb/status.h"
2426
#include "upb/mem/arena.h"
2527

2628
namespace {
@@ -488,6 +490,30 @@ TEST(CppGeneratedCode, ParseWithExtensionRegistry) {
488490
->ext_name());
489491
}
490492

493+
TEST(CppGeneratedCode, HpbStatusGeneratedRegistry) {
494+
TestModel model;
495+
ThemeExtension extension1;
496+
extension1.set_ext_name("Hello World");
497+
EXPECT_EQ(true, ::hpb::SetExtension(&model, ThemeExtension::theme_extension,
498+
extension1)
499+
.ok());
500+
hpb::Arena arena;
501+
auto bytes = ::hpb::Serialize(&model, arena);
502+
EXPECT_EQ(true, bytes.ok());
503+
504+
// By default, hpb::ParseOptionsDefault uses the generated registry.
505+
hpb::StatusOr<TestModel> parsed_model =
506+
::hpb::Parse<TestModel>(bytes.value(), hpb::ParseOptionsDefault());
507+
EXPECT_EQ(true, parsed_model.ok());
508+
EXPECT_EQ(true, hpb::GetExtension(&parsed_model.value(),
509+
ThemeExtension::theme_extension)
510+
.ok());
511+
EXPECT_EQ("Hello World", hpb::GetExtension(&parsed_model.value(),
512+
ThemeExtension::theme_extension)
513+
.value()
514+
->ext_name());
515+
}
516+
491517
TEST(CppGeneratedCode, ClearSubMessage) {
492518
// Fill model.
493519
TestModel model;

hpb_generator/tests/test_generated.cc

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@
2121
#include "hpb/arena.h"
2222
#include "hpb/backend/upb/interop.h"
2323
#include "hpb/hpb.h"
24+
#include "hpb/options.h"
2425
#include "hpb/ptr.h"
2526
#include "hpb/requires.h"
27+
#include "hpb/status.h"
2628

2729
namespace {
2830

@@ -405,6 +407,30 @@ TEST(CppGeneratedCode, MessageMapStringKeyAndInt32Value) {
405407
EXPECT_EQ(false, result_after_delete.ok());
406408
}
407409

410+
TEST(CppGeneratedCode, HpbStatus) {
411+
TestModel model;
412+
model.set_str1("lightweight status");
413+
hpb::Arena arena;
414+
absl::StatusOr<absl::string_view> bytes = ::hpb::Serialize(&model, arena);
415+
EXPECT_EQ(true, bytes.ok());
416+
417+
hpb::StatusOr<TestModel> parsed_model = ::hpb::Parse<TestModel>(
418+
bytes.value(), hpb::ParseOptionsWithEmptyRegistry());
419+
EXPECT_EQ(true, parsed_model.ok());
420+
EXPECT_EQ("lightweight status", parsed_model.value().str1());
421+
}
422+
423+
TEST(CppGeneratedCode, HpbStatusFail) {
424+
hpb::StatusOr<TestModel> status = ::hpb::Parse<TestModel>(
425+
"definitely not a proto", hpb::ParseOptionsWithEmptyRegistry());
426+
EXPECT_EQ(false, status.ok());
427+
EXPECT_EQ(status.error(), "Wire format was corrupt");
428+
429+
absl::StatusOr<TestModel> to_absl_status = status.ToAbslStatusOr();
430+
EXPECT_EQ(false, to_absl_status.ok());
431+
EXPECT_EQ(to_absl_status.status().message(), "Wire format was corrupt");
432+
}
433+
408434
TEST(CppGeneratedCode, SerializeUsingArena) {
409435
TestModel model;
410436
model.set_str1("Hello World");

0 commit comments

Comments
 (0)