Skip to content

Commit 2a8b2c8

Browse files
jnthntatumcopybara-github
authored andcommitted
Add CompilerLibraries for lists and strings extensions.
Add specific overloads for sortable list types. PiperOrigin-RevId: 778177024
1 parent 4326d18 commit 2a8b2c8

File tree

6 files changed

+154
-53
lines changed

6 files changed

+154
-53
lines changed

extensions/BUILD

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,11 +355,13 @@ cc_library(
355355
"//common:type",
356356
"//common:value",
357357
"//common:value_kind",
358+
"//compiler",
358359
"//internal:status_macros",
359360
"//parser:macro",
360361
"//parser:macro_expr_factory",
361362
"//parser:macro_registry",
362363
"//parser:options",
364+
"//parser:parser_interface",
363365
"//runtime:function_adapter",
364366
"//runtime:function_registry",
365367
"//runtime:runtime_options",
@@ -369,6 +371,7 @@ cc_library(
369371
"@com_google_absl//absl/container:flat_hash_set",
370372
"@com_google_absl//absl/status",
371373
"@com_google_absl//absl/status:statusor",
374+
"@com_google_absl//absl/strings",
372375
"@com_google_absl//absl/strings:str_format",
373376
"@com_google_absl//absl/strings:string_view",
374377
"@com_google_absl//absl/types:optional",
@@ -382,13 +385,13 @@ cc_test(
382385
srcs = ["lists_functions_test.cc"],
383386
deps = [
384387
":lists_functions",
385-
"//checker:standard_library",
386388
"//checker:validation_result",
387389
"//common:source",
388390
"//common:value",
389391
"//common:value_testing",
390392
"//compiler",
391393
"//compiler:compiler_factory",
394+
"//compiler:standard_library",
392395
"//extensions/protobuf:runtime_adapter",
393396
"//internal:testing",
394397
"//internal:testing_descriptor_pool",
@@ -404,6 +407,7 @@ cc_test(
404407
"//runtime:standard_runtime_builder_factory",
405408
"@com_google_absl//absl/status",
406409
"@com_google_absl//absl/status:status_matchers",
410+
"@com_google_absl//absl/strings:string_view",
407411
"@com_google_cel_spec//proto/cel/expr:syntax_cc_proto",
408412
"@com_google_protobuf//:protobuf",
409413
],
@@ -494,6 +498,7 @@ cc_library(
494498
"//common:decl",
495499
"//common:type",
496500
"//common:value",
501+
"//compiler",
497502
"//eval/public:cel_function_registry",
498503
"//eval/public:cel_options",
499504
"//internal:status_macros",
@@ -524,6 +529,7 @@ cc_test(
524529
"//common:decl",
525530
"//common:value",
526531
"//compiler:compiler_factory",
532+
"//compiler:standard_library",
527533
"//extensions/protobuf:runtime_adapter",
528534
"//internal:testing",
529535
"//internal:testing_descriptor_pool",

extensions/lists_functions.cc

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <cstddef>
1818
#include <cstdint>
1919
#include <numeric>
20+
#include <string>
2021
#include <utility>
2122
#include <vector>
2223

@@ -26,6 +27,7 @@
2627
#include "absl/container/flat_hash_set.h"
2728
#include "absl/status/status.h"
2829
#include "absl/status/statusor.h"
30+
#include "absl/strings/str_cat.h"
2931
#include "absl/strings/str_format.h"
3032
#include "absl/strings/string_view.h"
3133
#include "absl/types/optional.h"
@@ -38,11 +40,13 @@
3840
#include "common/type.h"
3941
#include "common/value.h"
4042
#include "common/value_kind.h"
43+
#include "compiler/compiler.h"
4144
#include "internal/status_macros.h"
4245
#include "parser/macro.h"
4346
#include "parser/macro_expr_factory.h"
4447
#include "parser/macro_registry.h"
4548
#include "parser/options.h"
49+
#include "parser/parser_interface.h"
4650
#include "runtime/function_adapter.h"
4751
#include "runtime/function_registry.h"
4852
#include "runtime/runtime_options.h"
@@ -55,6 +59,15 @@ namespace {
5559

5660
using ::cel::checker_internal::BuiltinsArena;
5761

62+
absl::Span<const cel::Type> SortableTypes() {
63+
static const Type kTypes[]{cel::IntType(), cel::UintType(),
64+
cel::DoubleType(), cel::BoolType(),
65+
cel::DurationType(), cel::TimestampType(),
66+
cel::StringType(), cel::BytesType()};
67+
68+
return kTypes;
69+
}
70+
5871
// Slow distinct() implementation that uses Equal() to compare values in O(n^2).
5972
absl::Status ListDistinctHeterogeneousImpl(
6073
const ListValue& list,
@@ -577,21 +590,50 @@ absl::Status RegisterListsCheckerDecls(TypeCheckerBuilder& builder) {
577590
"slice",
578591
MakeMemberOverloadDecl("list_slice", ListTypeParamType(),
579592
ListTypeParamType(), IntType(), IntType())));
580-
// TODO(uncreated-issue/83): Update to specific decls for sortable types.
581-
CEL_ASSIGN_OR_RETURN(
582-
FunctionDecl sort_decl,
583-
MakeFunctionDecl("sort",
584-
MakeMemberOverloadDecl("list_sort", ListTypeParamType(),
585-
ListTypeParamType())));
586593

594+
static const absl::NoDestructor<std::vector<Type>> kSortableListTypes([] {
595+
std::vector<Type> instance;
596+
instance.reserve(SortableTypes().size());
597+
for (const Type& type : SortableTypes()) {
598+
instance.push_back(ListType(BuiltinsArena(), type));
599+
}
600+
return instance;
601+
}());
602+
603+
FunctionDecl sort_decl;
604+
sort_decl.set_name("sort");
605+
FunctionDecl sort_by_key_decl;
606+
sort_by_key_decl.set_name("@sortByAssociatedKeys");
607+
608+
for (const Type& list_type : *kSortableListTypes) {
609+
std::string elem_type_name(list_type.AsList()->GetElement().name());
610+
611+
CEL_RETURN_IF_ERROR(sort_decl.AddOverload(MakeMemberOverloadDecl(
612+
absl::StrCat("list_", elem_type_name, "_sort"), list_type, list_type)));
613+
CEL_RETURN_IF_ERROR(sort_by_key_decl.AddOverload(MakeMemberOverloadDecl(
614+
absl::StrCat("list_", elem_type_name, "_sortByAssociatedKeys"),
615+
ListTypeParamType(), ListTypeParamType(), list_type)));
616+
}
617+
618+
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(sort_decl)));
619+
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(sort_by_key_decl)));
587620
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(distinct_decl)));
588621
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(flatten_decl)));
589622
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(range_decl)));
590623
// MergeFunction is used to combine with the reverse function
591624
// defined in strings extension.
592625
CEL_RETURN_IF_ERROR(builder.MergeFunction(std::move(reverse_decl)));
593626
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(slice_decl)));
594-
return builder.AddFunction(std::move(sort_decl));
627+
return absl::OkStatus();
628+
}
629+
630+
std::vector<Macro> lists_macros() { return {ListSortByMacro()}; }
631+
632+
absl::Status ConfigureParser(ParserBuilder& builder) {
633+
for (const Macro& macro : lists_macros()) {
634+
CEL_RETURN_IF_ERROR(builder.AddMacro(macro));
635+
}
636+
return absl::OkStatus();
595637
}
596638

597639
} // namespace
@@ -607,8 +649,6 @@ absl::Status RegisterListsFunctions(FunctionRegistry& registry,
607649
return absl::OkStatus();
608650
}
609651

610-
std::vector<Macro> lists_macros() { return {ListSortByMacro()}; }
611-
612652
absl::Status RegisterListsMacros(MacroRegistry& registry,
613653
const ParserOptions&) {
614654
return registry.RegisterMacros(lists_macros());
@@ -618,4 +658,10 @@ CheckerLibrary ListsCheckerLibrary() {
618658
return {.id = "cel.lib.ext.lists", .configure = RegisterListsCheckerDecls};
619659
}
620660

661+
CompilerLibrary ListsCompilerLibrary() {
662+
auto lib = CompilerLibrary::FromCheckerLibrary(ListsCheckerLibrary());
663+
lib.configure_parser = ConfigureParser;
664+
return lib;
665+
}
666+
621667
} // namespace cel::extensions

extensions/lists_functions.h

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
#include "absl/status/status.h"
1919
#include "checker/type_checker_builder.h"
20+
#include "compiler/compiler.h"
2021
#include "parser/macro_registry.h"
2122
#include "parser/options.h"
2223
#include "runtime/function_registry.h"
@@ -59,11 +60,31 @@ absl::Status RegisterListsMacros(MacroRegistry& registry,
5960
//
6061
// <list(T)>.reverse() -> list(T)
6162
//
62-
// <list(T)>.sort() -> list(T)
63+
// <list(T_)>.sort() -> list(T_) where T_ is partially orderable
6364
//
6465
// <list(T)>.slice(start: int, end: int) -> list(T)
6566
CheckerLibrary ListsCheckerLibrary();
6667

68+
// Provides decls for the following functions:
69+
//
70+
// lists.range(n: int) -> list(int)
71+
//
72+
// <list(T)>.distinct() -> list(T)
73+
//
74+
// <list(dyn)>.flatten() -> list(dyn)
75+
// <list(dyn)>.flatten(limit: int) -> list(dyn)
76+
//
77+
// <list(T)>.reverse() -> list(T)
78+
//
79+
// <list(T_)>.sort() -> list(T_) where T_ is partially orderable
80+
//
81+
// <list(T)>.slice(start: int, end: int) -> list(T)
82+
//
83+
// and the following macros:
84+
//
85+
// <list(T)>.sortBy(<element name>, <element key expression>)
86+
CompilerLibrary ListsCompilerLibrary();
87+
6788
} // namespace cel::extensions
6889

6990
#endif // THIRD_PARTY_CEL_CPP_EXTENSIONS_SETS_FUNCTIONS_H_

extensions/lists_functions_test.cc

Lines changed: 62 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,14 @@
2222
#include "cel/expr/syntax.pb.h"
2323
#include "absl/status/status.h"
2424
#include "absl/status/status_matchers.h"
25-
#include "checker/standard_library.h"
25+
#include "absl/strings/string_view.h"
2626
#include "checker/validation_result.h"
2727
#include "common/source.h"
2828
#include "common/value.h"
2929
#include "common/value_testing.h"
3030
#include "compiler/compiler.h"
3131
#include "compiler/compiler_factory.h"
32+
#include "compiler/standard_library.h"
3233
#include "extensions/protobuf/runtime_adapter.h"
3334
#include "internal/testing.h"
3435
#include "internal/testing_descriptor_pool.h"
@@ -43,7 +44,6 @@
4344
#include "runtime/runtime_options.h"
4445
#include "runtime/standard_runtime_builder_factory.h"
4546
#include "google/protobuf/arena.h"
46-
#include "google/protobuf/descriptor.h"
4747

4848
namespace cel::extensions {
4949
namespace {
@@ -281,8 +281,8 @@ TEST(ListsFunctionsTest, ListSortByMacroParseError) {
281281
}
282282

283283
struct ListCheckerTestCase {
284-
const std::string expr;
285-
bool is_valid;
284+
std::string expr;
285+
std::string error_substr;
286286
};
287287

288288
class ListsCheckerLibraryTest
@@ -291,64 +291,86 @@ class ListsCheckerLibraryTest
291291
void SetUp() override {
292292
// Arrange: Configure the compiler.
293293
// Add the lists checker library to the compiler builder.
294-
ASSERT_OK_AND_ASSIGN(std::unique_ptr<CompilerBuilder> compiler_builder,
295-
NewCompilerBuilder(descriptor_pool_));
296-
ASSERT_THAT(compiler_builder->AddLibrary(StandardCheckerLibrary()), IsOk());
297-
ASSERT_THAT(compiler_builder->AddLibrary(ListsCheckerLibrary()), IsOk());
294+
ASSERT_OK_AND_ASSIGN(
295+
std::unique_ptr<CompilerBuilder> compiler_builder,
296+
NewCompilerBuilder(internal::GetTestingDescriptorPool()));
297+
ASSERT_THAT(compiler_builder->AddLibrary(StandardCompilerLibrary()),
298+
IsOk());
299+
ASSERT_THAT(compiler_builder->AddLibrary(ListsCompilerLibrary()), IsOk());
300+
compiler_builder->GetCheckerBuilder().set_container(
301+
"cel.expr.conformance.proto3");
298302
ASSERT_OK_AND_ASSIGN(compiler_, std::move(*compiler_builder).Build());
299303
}
300304

301-
const google::protobuf::DescriptorPool* descriptor_pool_ =
302-
internal::GetTestingDescriptorPool();
303305
std::unique_ptr<Compiler> compiler_;
304306
};
305307

306308
TEST_P(ListsCheckerLibraryTest, ListsFunctionsTypeCheckerSuccess) {
307309
// Act & Assert: Compile the expression and validate the result.
308310
ASSERT_OK_AND_ASSIGN(ValidationResult result,
309311
compiler_->Compile(GetParam().expr));
310-
EXPECT_EQ(result.IsValid(), GetParam().is_valid);
312+
absl::string_view error_substr = GetParam().error_substr;
313+
EXPECT_EQ(result.IsValid(), error_substr.empty());
314+
315+
if (!error_substr.empty()) {
316+
EXPECT_THAT(result.FormatError(), HasSubstr(error_substr));
317+
}
311318
}
312319

313320
// Returns a vector of test cases for the ListsCheckerLibraryTest.
314321
// Returns both positive and negative test cases for the lists functions.
315322
std::vector<ListCheckerTestCase> createListsCheckerParams() {
316323
return {
317324
// lists.distinct()
318-
{R"([1,2,3,4,4].distinct() == [1,2,3,4])", true},
319-
{R"('abc'.distinct() == [1,2,3,4])", false},
320-
{R"([1,2,3,4,4].distinct() == 'abc')", false},
321-
{R"([1,2,3,4,4].distinct(1) == [1,2,3,4])", false},
325+
{R"([1,2,3,4,4].distinct() == [1,2,3,4])"},
326+
{R"('abc'.distinct() == [1,2,3,4])",
327+
"no matching overload for 'distinct'"},
328+
{R"([1,2,3,4,4].distinct() == 'abc')", "no matching overload for '_==_'"},
329+
{R"([1,2,3,4,4].distinct(1) == [1,2,3,4])", "undeclared reference"},
322330
// lists.flatten()
323-
{R"([1,2,3,4].flatten() == [1,2,3,4])", true},
324-
{R"([1,2,3,4].flatten(1) == [1,2,3,4])", true},
325-
{R"('abc'.flatten() == [1,2,3,4])", false},
326-
{R"([1,2,3,4].flatten() == 'abc')", false},
327-
{R"('abc'.flatten(1) == [1,2,3,4])", false},
328-
{R"([1,2,3,4].flatten('abc') == [1,2,3,4])", false},
329-
{R"([1,2,3,4].flatten(1) == 'abc')", false},
331+
{R"([1,2,3,4].flatten() == [1,2,3,4])"},
332+
{R"([1,2,3,4].flatten(1) == [1,2,3,4])"},
333+
{R"('abc'.flatten() == [1,2,3,4])", "no matching overload for 'flatten'"},
334+
{R"([1,2,3,4].flatten() == 'abc')", "no matching overload for '_==_'"},
335+
{R"('abc'.flatten(1) == [1,2,3,4])",
336+
"no matching overload for 'flatten'"},
337+
{R"([1,2,3,4].flatten('abc') == [1,2,3,4])",
338+
"no matching overload for 'flatten'"},
339+
{R"([1,2,3,4].flatten(1) == 'abc')", "no matching overload"},
330340
// lists.range()
331-
{R"(lists.range(4) == [0,1,2,3])", true},
332-
{R"(lists.range('abc') == [])", false},
333-
{R"(lists.range(4) == 'abc')", false},
334-
{R"(lists.range(4, 4) == [0,1,2,3])", false},
341+
{R"(lists.range(4) == [0,1,2,3])"},
342+
{R"(lists.range('abc') == [])", "no matching overload for 'lists.range'"},
343+
{R"(lists.range(4) == 'abc')", "no matching overload for '_==_'"},
344+
{R"(lists.range(4, 4) == [0,1,2,3])", "undeclared reference"},
335345
// lists.reverse()
336-
{R"([1,2,3,4].reverse() == [4,3,2,1])", true},
337-
{R"('abc'.reverse() == [])", false},
338-
{R"([1,2,3,4].reverse() == 'abc')", false},
339-
{R"([1,2,3,4].reverse(1) == [4,3,2,1])", false},
346+
{R"([1,2,3,4].reverse() == [4,3,2,1])"},
347+
{R"('abc'.reverse() == [])", "no matching overload for 'reverse'"},
348+
{R"([1,2,3,4].reverse() == 'abc')", "no matching overload for '_==_'"},
349+
{R"([1,2,3,4].reverse(1) == [4,3,2,1])", "undeclared reference"},
340350
// lists.slice()
341-
{R"([1,2,3,4].slice(0, 4) == [1,2,3,4])", true},
342-
{R"('abc'.slice(0, 4) == [1,2,3,4])", false},
343-
{R"([1,2,3,4].slice('abc', 4) == [1,2,3,4])", false},
344-
{R"([1,2,3,4].slice(0, 'abc') == [1,2,3,4])", false},
345-
{R"([1,2,3,4].slice(0, 4) == 'abc')", false},
346-
{R"([1,2,3,4].slice(0, 2, 3) == [1,2,3,4])", false},
351+
{R"([1,2,3,4].slice(0, 4) == [1,2,3,4])"},
352+
{R"('abc'.slice(0, 4) == [1,2,3,4])", "no matching overload for 'slice'"},
353+
{R"([1,2,3,4].slice('abc', 4) == [1,2,3,4])",
354+
"no matching overload for 'slice'"},
355+
{R"([1,2,3,4].slice(0, 'abc') == [1,2,3,4])",
356+
"no matching overload for 'slice'"},
357+
{R"([1,2,3,4].slice(0, 4) == 'abc')", "no matching overload for '_==_'"},
358+
{R"([1,2,3,4].slice(0, 2, 3) == [1,2,3,4])", "undeclared reference"},
347359
// lists.sort()
348-
{R"([1,2,3,4].sort() == [1,2,3,4])", true},
349-
{R"('abc'.sort() == [])", false},
350-
{R"([1,2,3,4].sort() == 'abc')", false},
351-
{R"([1,2,3,4].sort(2) == [1,2,3,4])", false},
360+
{R"([1,2,3,4].sort() == [1,2,3,4])"},
361+
{R"([TestAllTypes{}, TestAllTypes{}].sort() == [])",
362+
"no matching overload for 'sort'"},
363+
{R"('abc'.sort() == [])", "no matching overload for 'sort'"},
364+
{R"([1,2,3,4].sort() == 'abc')", "no matching overload for '_==_'"},
365+
{R"([1,2,3,4].sort(2) == [1,2,3,4])", "undeclared reference"},
366+
// sortBy macro
367+
{R"([1,2,3,4].sortBy(x, -x) == [4,3,2,1])"},
368+
{R"([TestAllTypes{}, TestAllTypes{}].sortBy(x, x) == [])",
369+
"no matching overload for '@sortByAssociatedKeys'"},
370+
{R"(
371+
[TestAllTypes{single_int64: 2}, TestAllTypes{single_int64: 1}]
372+
.sortBy(x, x.single_int64) ==
373+
[TestAllTypes{single_int64: 1}, TestAllTypes{single_int64: 2}])"},
352374
};
353375
}
354376

0 commit comments

Comments
 (0)