Skip to content

Commit 75045e0

Browse files
CEL Dev Teamcopybara-github
authored andcommitted
Create independent build target for string.format()
`string.format()` pulls in extra dependencies like ICU4C for locale support. Not all users need formatting. If everything is bundled in the strings extensions, all users need to pay for the extra dependency. By putting it in its own build target, users can opt in. PiperOrigin-RevId: 719338589
1 parent 3eea687 commit 75045e0

6 files changed

Lines changed: 31 additions & 50 deletions

File tree

extensions/BUILD

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -439,22 +439,15 @@ cc_test(
439439

440440
cc_library(
441441
name = "strings",
442-
srcs = [
443-
"formatting.cc",
444-
"strings.cc",
445-
],
446-
hdrs = [
447-
"formatting.h",
448-
"strings.h",
449-
],
442+
srcs = ["strings.cc"],
443+
hdrs = ["strings.h"],
450444
deps = [
451445
"//checker:type_checker_builder",
452446
"//checker/internal:builtins_arena",
453447
"//common:casting",
454448
"//common:decl",
455449
"//common:type",
456450
"//common:value",
457-
"//common:value_kind",
458451
"//eval/public:cel_function_registry",
459452
"//eval/public:cel_options",
460453
"//internal:status_macros",
@@ -463,19 +456,12 @@ cc_library(
463456
"//runtime:function_registry",
464457
"//runtime:runtime_options",
465458
"//runtime/internal:errors",
466-
"@com_google_absl//absl/base:core_headers",
467459
"@com_google_absl//absl/base:no_destructor",
468-
"@com_google_absl//absl/container:btree",
469-
"@com_google_absl//absl/memory",
470-
"@com_google_absl//absl/numeric:bits",
471460
"@com_google_absl//absl/status",
472461
"@com_google_absl//absl/status:statusor",
473462
"@com_google_absl//absl/strings",
474463
"@com_google_absl//absl/strings:cord",
475-
"@com_google_absl//absl/strings:str_format",
476464
"@com_google_absl//absl/strings:string_view",
477-
"@com_google_absl//absl/time",
478-
"@icu4c",
479465
],
480466
)
481467

@@ -593,11 +579,36 @@ cc_test(
593579
],
594580
)
595581

582+
cc_library(
583+
name = "formatting",
584+
srcs = ["formatting.cc"],
585+
hdrs = ["formatting.h"],
586+
deps = [
587+
"//common:value",
588+
"//common:value_kind",
589+
"//internal:status_macros",
590+
"//runtime:function_adapter",
591+
"//runtime:function_registry",
592+
"//runtime:runtime_options",
593+
"@com_google_absl//absl/base:core_headers",
594+
"@com_google_absl//absl/container:btree",
595+
"@com_google_absl//absl/memory",
596+
"@com_google_absl//absl/numeric:bits",
597+
"@com_google_absl//absl/status",
598+
"@com_google_absl//absl/status:statusor",
599+
"@com_google_absl//absl/strings",
600+
"@com_google_absl//absl/strings:str_format",
601+
"@com_google_absl//absl/strings:string_view",
602+
"@com_google_absl//absl/time",
603+
"@icu4c",
604+
],
605+
)
606+
596607
cc_test(
597608
name = "formatting_test",
598609
srcs = ["formatting_test.cc"],
599610
deps = [
600-
":strings",
611+
":formatting",
601612
"//common:allocator",
602613
"//common:value",
603614
"//extensions/protobuf:runtime_adapter",

extensions/formatting.cc

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -582,8 +582,6 @@ absl::StatusOr<Value> Format(ValueManager& value_manager,
582582

583583
} // namespace
584584

585-
namespace strings_internal {
586-
587585
absl::Status RegisterStringFormattingFunctions(FunctionRegistry& registry,
588586
const RuntimeOptions& options) {
589587
auto locale = icu::Locale::createCanonical(options.locale.c_str());
@@ -603,5 +601,4 @@ absl::Status RegisterStringFormattingFunctions(FunctionRegistry& registry,
603601
return absl::OkStatus();
604602
}
605603

606-
} // namespace strings_internal
607604
} // namespace cel::extensions

extensions/formatting.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,12 @@
1919
#include "runtime/function_registry.h"
2020
#include "runtime/runtime_options.h"
2121

22-
namespace cel::extensions::strings_internal {
22+
namespace cel::extensions {
2323

2424
// Register extension functions for string formatting.
2525
absl::Status RegisterStringFormattingFunctions(FunctionRegistry& registry,
2626
const RuntimeOptions& options);
2727

28-
} // namespace cel::extensions::strings_internal
28+
} // namespace cel::extensions
2929

3030
#endif // THIRD_PARTY_CEL_CPP_EXTENSIONS_FORMATTING_H_

extensions/formatting_test.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,7 @@ TEST_P(StringFormatTest, TestStringFormatting) {
8989
CreateStandardRuntimeBuilder(
9090
internal::GetTestingDescriptorPool(), options));
9191
auto registration_status =
92-
strings_internal::RegisterStringFormattingFunctions(
93-
builder.function_registry(), options);
92+
RegisterStringFormattingFunctions(builder.function_registry(), options);
9493
if (test_case.error.has_value() && !registration_status.ok()) {
9594
EXPECT_THAT(registration_status.message(), HasSubstr(*test_case.error));
9695
return;

extensions/strings.cc

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
#include "common/value_manager.h"
3737
#include "eval/public/cel_function_registry.h"
3838
#include "eval/public/cel_options.h"
39-
#include "extensions/formatting.h"
4039
#include "internal/status_macros.h"
4140
#include "internal/utf8.h"
4241
#include "runtime/function_adapter.h"
@@ -432,8 +431,6 @@ absl::Status RegisterStringsFunctions(FunctionRegistry& registry,
432431
int64_t>::CreateDescriptor("replace", /*receiver_style=*/true),
433432
VariadicFunctionAdapter<absl::StatusOr<Value>, StringValue, StringValue,
434433
StringValue, int64_t>::WrapFunction(Replace2)));
435-
CEL_RETURN_IF_ERROR(
436-
strings_internal::RegisterStringFormattingFunctions(registry, options));
437434
return absl::OkStatus();
438435
}
439436

extensions/strings_test.cc

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -228,29 +228,6 @@ TEST(Strings, UpperAscii) {
228228
EXPECT_TRUE(result.GetBool().NativeValue());
229229
}
230230

231-
TEST(Strings, Format) {
232-
google::protobuf::Arena arena;
233-
const auto options = RuntimeOptions{};
234-
ASSERT_OK_AND_ASSIGN(auto builder,
235-
CreateStandardRuntimeBuilder(
236-
internal::GetTestingDescriptorPool(), options));
237-
EXPECT_THAT(RegisterStringsFunctions(builder.function_registry(), options),
238-
IsOk());
239-
240-
ASSERT_OK_AND_ASSIGN(auto runtime, std::move(builder).Build());
241-
242-
ASSERT_OK_AND_ASSIGN(ParsedExpr expr, Parse("'abc %d'.format([2]) == 'abc 2'",
243-
"<input>", ParserOptions{}));
244-
245-
ASSERT_OK_AND_ASSIGN(std::unique_ptr<Program> program,
246-
ProtobufRuntimeAdapter::CreateProgram(*runtime, expr));
247-
248-
Activation activation;
249-
ASSERT_OK_AND_ASSIGN(Value result, program->Evaluate(&arena, activation));
250-
ASSERT_TRUE(result.Is<BoolValue>());
251-
EXPECT_TRUE(result.GetBool().NativeValue());
252-
}
253-
254231
TEST(StringsCheckerLibrary, SmokeTest) {
255232
google::protobuf::Arena arena;
256233
ASSERT_OK_AND_ASSIGN(

0 commit comments

Comments
 (0)