Skip to content

Commit 26b3a5a

Browse files
jnthntatumcopybara-github
jnthntatum
authored and
copybara-github
committed
Add support for subsetting libraries in CompilerBuilder.
PiperOrigin-RevId: 747978669
1 parent 91a9c92 commit 26b3a5a

18 files changed

+643
-118
lines changed

‎checker/internal/type_checker_builder_impl.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
14+
1415
#include "checker/internal/type_checker_builder_impl.h"
1516

1617
#include <cstddef>
@@ -124,8 +125,7 @@ absl::optional<FunctionDecl> FilterDecl(FunctionDecl decl,
124125
std::string name = decl.release_name();
125126
std::vector<OverloadDecl> overloads = decl.release_overloads();
126127
for (const auto& ovl : overloads) {
127-
if (subset.predicate(name, ovl.id()) ==
128-
TypeCheckerSubset::MatchResult::kInclude) {
128+
if (subset.should_include_overload(name, ovl.id())) {
129129
absl::Status s = filtered.AddOverload(std::move(ovl));
130130
if (!s.ok()) {
131131
// Should not be possible to construct the original decl in a way that

‎checker/type_checker_builder.h

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -50,30 +50,18 @@ struct CheckerLibrary {
5050

5151
// Represents a declaration to only use a subset of a library.
5252
struct TypeCheckerSubset {
53-
// Semantic for how to apply the subsetting predicate.
54-
//
55-
// For functions, the predicate is applied to each overload. If no overload
56-
// for a function is determined to be in the subset, the function is
57-
// excluded. Otherwise, a filtered version of the function is added to the
58-
// type checker.
59-
enum class MatchResult {
60-
// Include only the declarations that match the predicate, exclude by
61-
// default.
62-
kInclude,
63-
// Exclude only the declarations that match the predicate, include by
64-
// default.
65-
kExclude
66-
};
67-
68-
using FunctionPredicate = absl::AnyInvocable<MatchResult(
53+
using FunctionPredicate = absl::AnyInvocable<bool(
6954
absl::string_view function, absl::string_view overload_id) const>;
7055

7156
// The id of the library to subset. Only one subset can be applied per
7257
// library id.
7358
//
7459
// Must be non-empty.
7560
std::string library_id;
76-
FunctionPredicate predicate;
61+
// Predicate to apply to function overloads. If true, the overload will be
62+
// included in the subset. If no overload for a function is included, the
63+
// entire function is excluded.
64+
FunctionPredicate should_include_overload;
7765
};
7866

7967
// Interface for TypeCheckerBuilders.

‎checker/type_checker_builder_factory_test.cc

Lines changed: 19 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -231,9 +231,7 @@ TEST(TypeCheckerBuilderTest, AddLibraryIncludeSubset) {
231231
builder->AddLibrarySubset(
232232
{"testlib",
233233
[](absl::string_view /*function*/, absl::string_view overload_id) {
234-
return (overload_id == "add_int" || overload_id == "sub_int")
235-
? TypeCheckerSubset::MatchResult::kInclude
236-
: TypeCheckerSubset::MatchResult::kExclude;
234+
return (overload_id == "add_int" || overload_id == "sub_int");
237235
}}),
238236
IsOk());
239237
ASSERT_OK_AND_ASSIGN(auto checker, builder->Build());
@@ -272,9 +270,7 @@ TEST(TypeCheckerBuilderTest, AddLibraryExcludeSubset) {
272270
builder->AddLibrarySubset(
273271
{"testlib",
274272
[](absl::string_view /*function*/, absl::string_view overload_id) {
275-
return (overload_id == "add_int" || overload_id == "sub_int")
276-
? TypeCheckerSubset::MatchResult::kExclude
277-
: TypeCheckerSubset::MatchResult::kInclude;
273+
return (overload_id != "add_int" && overload_id != "sub_int");
278274
;
279275
}}),
280276
IsOk());
@@ -310,15 +306,12 @@ TEST(TypeCheckerBuilderTest, AddLibrarySubsetRemoveAllOvl) {
310306
CreateTypeCheckerBuilder(GetSharedTestingDescriptorPool()));
311307

312308
ASSERT_THAT(builder->AddLibrary(SubsetTestlib()), IsOk());
313-
ASSERT_THAT(
314-
builder->AddLibrarySubset(
315-
{"testlib",
316-
[](absl::string_view function, absl::string_view /*overload_id*/) {
317-
return function == "add"
318-
? TypeCheckerSubset::MatchResult::kExclude
319-
: TypeCheckerSubset::MatchResult::kInclude;
320-
}}),
321-
IsOk());
309+
ASSERT_THAT(builder->AddLibrarySubset({"testlib",
310+
[](absl::string_view function,
311+
absl::string_view /*overload_id*/) {
312+
return function != "add";
313+
}}),
314+
IsOk());
322315
ASSERT_OK_AND_ASSIGN(auto checker, builder->Build());
323316

324317
std::vector<ValidationResult> results;
@@ -353,35 +346,28 @@ TEST(TypeCheckerBuilderTest, AddLibraryOneSubsetPerLibraryId) {
353346
ASSERT_THAT(builder->AddLibrary(SubsetTestlib()), IsOk());
354347
ASSERT_THAT(
355348
builder->AddLibrarySubset(
356-
{"testlib",
357-
[](absl::string_view function, absl::string_view /*overload_id*/) {
358-
return TypeCheckerSubset::MatchResult::kInclude;
359-
}}),
349+
{"testlib", [](absl::string_view function,
350+
absl::string_view /*overload_id*/) { return true; }}),
360351
IsOk());
361352
EXPECT_THAT(
362353
builder->AddLibrarySubset(
363-
{"testlib",
364-
[](absl::string_view function, absl::string_view /*overload_id*/) {
365-
return TypeCheckerSubset::MatchResult::kInclude;
366-
}}),
354+
{"testlib", [](absl::string_view function,
355+
absl::string_view /*overload_id*/) { return true; }}),
367356
StatusIs(absl::StatusCode::kAlreadyExists));
368357
}
369358

370-
TEST(TypeCheckerBuilderTest, AddLibrarySubsetLibraryIdRequiredds) {
359+
TEST(TypeCheckerBuilderTest, AddLibrarySubsetLibraryIdRequireds) {
371360
ASSERT_OK_AND_ASSIGN(
372361
std::unique_ptr<TypeCheckerBuilder> builder,
373362
CreateTypeCheckerBuilder(GetSharedTestingDescriptorPool()));
374363

375364
ASSERT_THAT(builder->AddLibrary(SubsetTestlib()), IsOk());
376-
EXPECT_THAT(
377-
builder->AddLibrarySubset(
378-
{"",
379-
[](absl::string_view function, absl::string_view /*overload_id*/) {
380-
return function == "add"
381-
? TypeCheckerSubset::MatchResult::kExclude
382-
: TypeCheckerSubset::MatchResult::kInclude;
383-
}}),
384-
StatusIs(absl::StatusCode::kInvalidArgument));
365+
EXPECT_THAT(builder->AddLibrarySubset({"",
366+
[](absl::string_view function,
367+
absl::string_view /*overload_id*/) {
368+
return function == "add";
369+
}}),
370+
StatusIs(absl::StatusCode::kInvalidArgument));
385371
}
386372

387373
TEST(TypeCheckerBuilderTest, AddContextDeclaration) {

‎checker/type_checker_subset_factory.cc

Lines changed: 14 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -24,55 +24,31 @@
2424

2525
namespace cel {
2626

27-
TypeCheckerSubset TypeCheckerSubsetFactory::StdlibIncludeList(
27+
TypeCheckerSubset::FunctionPredicate IncludeOverloadsByIdPredicate(
2828
absl::flat_hash_set<std::string> overload_ids) {
29-
return TypeCheckerSubset{
30-
.library_id = "stdlib",
31-
.predicate =
32-
[overload_ids = std::move(overload_ids)](
33-
absl::string_view /*function*/, absl::string_view overload_id) {
34-
return overload_ids.contains(overload_id)
35-
? TypeCheckerSubset::MatchResult::kInclude
36-
: TypeCheckerSubset::MatchResult::kExclude;
37-
},
29+
return [overload_ids = std::move(overload_ids)](
30+
absl::string_view /*function*/, absl::string_view overload_id) {
31+
return overload_ids.contains(overload_id);
3832
};
3933
}
4034

41-
TypeCheckerSubset TypeCheckerSubsetFactory::StdlibIncludeList(
42-
absl::Span<const std::string> overload_ids) {
43-
return StdlibIncludeList(absl::flat_hash_set<std::string>(
35+
TypeCheckerSubset::FunctionPredicate IncludeOverloadsByIdPredicate(
36+
absl::Span<const absl::string_view> overload_ids) {
37+
return IncludeOverloadsByIdPredicate(absl::flat_hash_set<std::string>(
4438
overload_ids.begin(), overload_ids.end()));
4539
}
4640

47-
TypeCheckerSubset TypeCheckerSubsetFactory::StdlibIncludeList(
48-
absl::Span<absl::string_view> overload_ids) {
49-
return StdlibIncludeList(absl::flat_hash_set<std::string>(
50-
overload_ids.begin(), overload_ids.end()));
51-
}
52-
53-
TypeCheckerSubset TypeCheckerSubsetFactory::StdlibExcludeList(
41+
TypeCheckerSubset::FunctionPredicate ExcludeOverloadsByIdPredicate(
5442
absl::flat_hash_set<std::string> overload_ids) {
55-
return TypeCheckerSubset{
56-
.library_id = "stdlib",
57-
.predicate =
58-
[overload_ids = std::move(overload_ids)](
59-
absl::string_view /*function*/, absl::string_view overload_id) {
60-
return overload_ids.contains(overload_id)
61-
? TypeCheckerSubset::MatchResult::kExclude
62-
: TypeCheckerSubset::MatchResult::kInclude;
63-
},
43+
return [overload_ids = std::move(overload_ids)](
44+
absl::string_view /*function*/, absl::string_view overload_id) {
45+
return !overload_ids.contains(overload_id);
6446
};
6547
}
6648

67-
TypeCheckerSubset TypeCheckerSubsetFactory::StdlibExcludeList(
68-
absl::Span<const std::string> overload_ids) {
69-
return StdlibExcludeList(absl::flat_hash_set<std::string>(
70-
overload_ids.begin(), overload_ids.end()));
71-
}
72-
73-
TypeCheckerSubset TypeCheckerSubsetFactory::StdlibExcludeList(
74-
absl::Span<absl::string_view> overload_ids) {
75-
return StdlibExcludeList(absl::flat_hash_set<std::string>(
49+
TypeCheckerSubset::FunctionPredicate ExcludeOverloadsByIdPredicate(
50+
absl::Span<const absl::string_view> overload_ids) {
51+
return ExcludeOverloadsByIdPredicate(absl::flat_hash_set<std::string>(
7652
overload_ids.begin(), overload_ids.end()));
7753
}
7854

‎checker/type_checker_subset_factory.h

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
14+
//
15+
// Factory functions for creating typical type checker library subsets.
1416

1517
#ifndef THIRD_PARTY_CEL_CPP_CHECKER_TYPE_CHECKER_SUBSET_FACTORY_H_
1618
#define THIRD_PARTY_CEL_CPP_CHECKER_TYPE_CHECKER_SUBSET_FACTORY_H_
@@ -24,28 +26,19 @@
2426

2527
namespace cel {
2628

27-
// Factory for creating typical type checker subsets.
28-
struct TypeCheckerSubsetFactory {
29-
// Subsets the standard library to only include the given overload ids.
30-
static TypeCheckerSubset StdlibIncludeList(
31-
absl::flat_hash_set<std::string> overload_ids);
32-
33-
static TypeCheckerSubset StdlibIncludeList(
34-
absl::Span<const std::string> overload_ids);
35-
36-
static TypeCheckerSubset StdlibIncludeList(
37-
absl::Span<absl::string_view> overload_ids);
29+
// Subsets a type checker library to only include the given overload ids.
30+
TypeCheckerSubset::FunctionPredicate IncludeOverloadsByIdPredicate(
31+
absl::flat_hash_set<std::string> overload_ids);
3832

39-
// Subsets the standard library to exclude the given overload ids.
40-
static TypeCheckerSubset StdlibExcludeList(
41-
absl::flat_hash_set<std::string> overload_ids);
33+
TypeCheckerSubset::FunctionPredicate IncludeOverloadsByIdPredicate(
34+
absl::Span<const absl::string_view> overload_ids);
4235

43-
static TypeCheckerSubset StdlibExcludeList(
44-
absl::Span<const std::string> overload_ids);
36+
// Subsets a type checker library to exclude the given overload ids.
37+
TypeCheckerSubset::FunctionPredicate ExcludeOverloadsByIdPredicate(
38+
absl::flat_hash_set<std::string> overload_ids);
4539

46-
static TypeCheckerSubset StdlibExcludeList(
47-
absl::Span<absl::string_view> overload_ids);
48-
};
40+
TypeCheckerSubset::FunctionPredicate ExcludeOverloadsByIdPredicate(
41+
absl::Span<const absl::string_view> overload_ids);
4942

5043
} // namespace cel
5144

‎checker/type_checker_subset_factory_test.cc

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ using ::absl_testing::IsOk;
3131
namespace cel {
3232
namespace {
3333

34-
TEST(TypeCheckerSubsetFactoryTest, StdlibIncludeList) {
34+
TEST(TypeCheckerSubsetFactoryTest, IncludeOverloadsByIdPredicate) {
3535
ASSERT_OK_AND_ASSIGN(
3636
std::unique_ptr<CompilerBuilder> builder,
3737
NewCompilerBuilder(internal::GetSharedTestingDescriptorPool()));
@@ -45,8 +45,10 @@ TEST(TypeCheckerSubsetFactoryTest, StdlibIncludeList) {
4545
StandardOverloadIds::kNotStrictlyFalse,
4646
};
4747
ASSERT_THAT(builder->AddLibrary(StandardCompilerLibrary()), IsOk());
48-
ASSERT_THAT(builder->GetCheckerBuilder().AddLibrarySubset(
49-
TypeCheckerSubsetFactory::StdlibIncludeList(allowlist)),
48+
ASSERT_THAT(builder->GetCheckerBuilder().AddLibrarySubset({
49+
"stdlib",
50+
IncludeOverloadsByIdPredicate(allowlist),
51+
}),
5052
IsOk());
5153

5254
ASSERT_OK_AND_ASSIGN(std::unique_ptr<Compiler> compiler, builder->Build());
@@ -74,7 +76,7 @@ TEST(TypeCheckerSubsetFactoryTest, StdlibIncludeList) {
7476
EXPECT_FALSE(r.IsValid());
7577
}
7678

77-
TEST(TypeCheckerSubsetFactoryTest, StdlibExcludeList) {
79+
TEST(TypeCheckerSubsetFactoryTest, ExcludeOverloadsByIdPredicate) {
7880
ASSERT_OK_AND_ASSIGN(
7981
std::unique_ptr<CompilerBuilder> builder,
8082
NewCompilerBuilder(internal::GetSharedTestingDescriptorPool()));
@@ -83,8 +85,10 @@ TEST(TypeCheckerSubsetFactoryTest, StdlibExcludeList) {
8385
StandardOverloadIds::kMatchesMember,
8486
};
8587
ASSERT_THAT(builder->AddLibrary(StandardCompilerLibrary()), IsOk());
86-
ASSERT_THAT(builder->GetCheckerBuilder().AddLibrarySubset(
87-
TypeCheckerSubsetFactory::StdlibExcludeList(exclude_list)),
88+
ASSERT_THAT(builder->GetCheckerBuilder().AddLibrarySubset({
89+
"stdlib",
90+
ExcludeOverloadsByIdPredicate(exclude_list),
91+
}),
8892
IsOk());
8993

9094
ASSERT_OK_AND_ASSIGN(std::unique_ptr<Compiler> compiler, builder->Build());

‎compiler/BUILD

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ cc_library(
2424
"//checker:validation_result",
2525
"//parser:options",
2626
"//parser:parser_interface",
27-
"@com_google_absl//absl/functional:any_invocable",
2827
"@com_google_absl//absl/status",
2928
"@com_google_absl//absl/status:statusor",
3029
"@com_google_absl//absl/strings:string_view",
@@ -42,7 +41,6 @@ cc_library(
4241
"//checker:type_checker_builder_factory",
4342
"//checker:validation_result",
4443
"//common:source",
45-
"//common:type",
4644
"//internal:noop_delete",
4745
"//internal:status_macros",
4846
"//parser",
@@ -133,3 +131,37 @@ cc_library(
133131
"@com_google_absl//absl/status",
134132
],
135133
)
134+
135+
cc_library(
136+
name = "compiler_library_subset_factory",
137+
srcs = ["compiler_library_subset_factory.cc"],
138+
hdrs = ["compiler_library_subset_factory.h"],
139+
deps = [
140+
":compiler",
141+
"//checker:type_checker_subset_factory",
142+
"//parser:parser_subset_factory",
143+
"@com_google_absl//absl/container:flat_hash_set",
144+
"@com_google_absl//absl/strings:string_view",
145+
"@com_google_absl//absl/types:span",
146+
],
147+
)
148+
149+
cc_test(
150+
name = "compiler_library_subset_factory_test",
151+
srcs = ["compiler_library_subset_factory_test.cc"],
152+
deps = [
153+
":compiler",
154+
":compiler_factory",
155+
":compiler_library_subset_factory",
156+
":standard_library",
157+
"//checker:validation_result",
158+
"//common:standard_definitions",
159+
"//internal:testing",
160+
"//internal:testing_descriptor_pool",
161+
"@com_google_absl//absl/container:flat_hash_set",
162+
"@com_google_absl//absl/status:status_matchers",
163+
"@com_google_absl//absl/status:statusor",
164+
"@com_google_absl//absl/strings:string_view",
165+
"@com_google_absl//absl/types:span",
166+
],
167+
)

‎compiler/compiler.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,18 @@ struct CompilerLibrary {
7878
configure_checker(std::move(checker_library.configure)) {}
7979
};
8080

81+
struct CompilerLibrarySubset {
82+
// The id of the library to subset. Only one subset can be applied per
83+
// library id.
84+
//
85+
// Must be non-empty.
86+
std::string library_id;
87+
ParserLibrarySubset::MacroPredicate should_include_macro;
88+
TypeCheckerSubset::FunctionPredicate should_include_overload;
89+
// TODO: to faithfully report the subset back, we need to track
90+
// the default (include or exclude) behavior for each of the predicates.
91+
};
92+
8193
// General options for configuring the underlying parser and checker.
8294
struct CompilerOptions {
8395
ParserOptions parser_options;
@@ -92,7 +104,8 @@ class CompilerBuilder {
92104
public:
93105
virtual ~CompilerBuilder() = default;
94106

95-
virtual absl::Status AddLibrary(cel::CompilerLibrary library) = 0;
107+
virtual absl::Status AddLibrary(CompilerLibrary library) = 0;
108+
virtual absl::Status AddLibrarySubset(CompilerLibrarySubset subset) = 0;
96109

97110
virtual TypeCheckerBuilder& GetCheckerBuilder() = 0;
98111
virtual ParserBuilder& GetParserBuilder() = 0;

0 commit comments

Comments
 (0)