Skip to content

Commit c4a927e

Browse files
jnthntatumcopybara-github
authored andcommitted
Add ToBuilder() functions for parser/checker/compiler
Adds support for extending a configured compiler similar to cel-java. PiperOrigin-RevId: 905220235
1 parent 1ca513f commit c4a927e

20 files changed

Lines changed: 335 additions & 32 deletions

checker/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ cc_test(
123123
srcs = ["type_checker_builder_factory_test.cc"],
124124
deps = [
125125
":checker_options",
126+
":optional",
126127
":standard_library",
127128
":type_checker",
128129
":type_checker_builder",

checker/internal/BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ cc_library(
7474
"@com_google_absl//absl/base:core_headers",
7575
"@com_google_absl//absl/base:nullability",
7676
"@com_google_absl//absl/container:flat_hash_map",
77+
"@com_google_absl//absl/log:absl_check",
7778
"@com_google_absl//absl/memory",
7879
"@com_google_absl//absl/status:statusor",
7980
"@com_google_absl//absl/strings",
@@ -175,6 +176,7 @@ cc_test(
175176
":type_checker_impl",
176177
"//checker:checker_options",
177178
"//checker:type_check_issue",
179+
"//checker:type_checker_builder",
178180
"//checker:validation_result",
179181
"//common:ast",
180182
"//common:container",

checker/internal/type_check_env.h

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "absl/base/attributes.h"
2424
#include "absl/base/nullability.h"
2525
#include "absl/container/flat_hash_map.h"
26+
#include "absl/log/absl_check.h"
2627
#include "absl/memory/memory.h"
2728
#include "absl/status/statusor.h"
2829
#include "absl/strings/string_view.h"
@@ -100,7 +101,8 @@ class TypeCheckEnv {
100101
type_providers_.push_back(proto_type_introspector_);
101102
}
102103

103-
// Move-only.
104+
TypeCheckEnv(const TypeCheckEnv&) = default;
105+
TypeCheckEnv& operator=(const TypeCheckEnv&) = default;
104106
TypeCheckEnv(TypeCheckEnv&&) = default;
105107
TypeCheckEnv& operator=(TypeCheckEnv&&) = default;
106108

@@ -193,19 +195,26 @@ class TypeCheckEnv {
193195

194196
// Used to keep an arena alive if one was needed to allocate types.
195197
//
196-
// The TypeCheckEnv does not otherwise use it.
197-
void set_arena(std::shared_ptr<const google::protobuf::Arena> arena) {
198+
// Expected to be called exactly once if at all.
199+
void set_arena(std::shared_ptr<google::protobuf::Arena> arena) {
200+
ABSL_DCHECK(arena_ == nullptr || arena == arena_);
198201
arena_ = std::move(arena);
199202
}
200203

204+
// Returns the arena if one was set, nullptr otherwise.
205+
std::shared_ptr<google::protobuf::Arena> arena() const { return arena_; }
206+
201207
private:
202208
absl::StatusOr<absl::optional<VariableDecl>> LookupEnumConstant(
203209
absl::string_view type, absl::string_view value) const;
204210

205211
absl_nonnull std::shared_ptr<const google::protobuf::DescriptorPool> descriptor_pool_;
206212

207213
// If set, an arena was needed to allocate types in the environment.
208-
absl_nullable std::shared_ptr<const google::protobuf::Arena> arena_;
214+
//
215+
// The TypeCheckEnv does not otherwise use the arena, though it may be used by
216+
// derived TypeCheckerBuilders.
217+
absl_nullable std::shared_ptr<google::protobuf::Arena> arena_;
209218
ExpressionContainer container_;
210219

211220
// Used to resolve fields on message types.

checker/internal/type_checker_builder_impl.cc

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "checker/internal/type_checker_impl.h"
3535
#include "checker/type_checker.h"
3636
#include "checker/type_checker_builder.h"
37+
#include "common/container.h"
3738
#include "common/decl.h"
3839
#include "common/type.h"
3940
#include "common/type_introspector.h"
@@ -342,8 +343,16 @@ absl::Status TypeCheckerBuilderImpl::ApplyConfig(
342343
}
343344

344345
absl::StatusOr<std::unique_ptr<TypeChecker>> TypeCheckerBuilderImpl::Build() {
345-
TypeCheckEnv env(descriptor_pool_);
346-
env.set_container(expression_container_);
346+
TypeCheckEnv env(template_env_);
347+
CEL_RETURN_IF_ERROR(ConfigureTypeCheckEnv(env));
348+
return std::make_unique<checker_internal::TypeCheckerImpl>(std::move(env),
349+
options_);
350+
}
351+
352+
absl::Status TypeCheckerBuilderImpl::ConfigureTypeCheckEnv(TypeCheckEnv& env) {
353+
if (expression_container_.has_value()) {
354+
env.set_container(*expression_container_);
355+
}
347356
if (expected_type_.has_value()) {
348357
env.set_expected_type(*expected_type_);
349358
}
@@ -377,12 +386,10 @@ absl::StatusOr<std::unique_ptr<TypeChecker>> TypeCheckerBuilderImpl::Build() {
377386
/*subset=*/nullptr, env));
378387

379388
CEL_RETURN_IF_ERROR(ApplyConfig(default_config_, /*subset=*/nullptr, env));
380-
// A library may have been the first to initialize the arena, so we need to
381-
// set it as the last step.
382-
env.set_arena(arena_);
383-
auto checker = std::make_unique<checker_internal::TypeCheckerImpl>(
384-
std::move(env), options_);
385-
return checker;
389+
if (type_arena_ != nullptr) {
390+
env.set_arena(type_arena_);
391+
}
392+
return absl::OkStatus();
386393
}
387394

388395
absl::Status TypeCheckerBuilderImpl::AddLibrary(CheckerLibrary library) {
@@ -432,7 +439,7 @@ absl::Status TypeCheckerBuilderImpl::AddOrReplaceVariable(
432439
absl::Status TypeCheckerBuilderImpl::AddContextDeclaration(
433440
absl::string_view type) {
434441
const google::protobuf::Descriptor* desc =
435-
descriptor_pool_->FindMessageTypeByName(type);
442+
template_env_.descriptor_pool()->FindMessageTypeByName(type);
436443
if (desc == nullptr) {
437444
return absl::NotFoundError(
438445
absl::StrCat("context declaration '", type, "' not found"));
@@ -479,7 +486,10 @@ void TypeCheckerBuilderImpl::AddTypeProvider(
479486
}
480487

481488
void TypeCheckerBuilderImpl::set_container(absl::string_view container) {
482-
expression_container_.SetContainer(container).IgnoreError();
489+
if (!expression_container_.has_value()) {
490+
expression_container_.emplace();
491+
}
492+
expression_container_->SetContainer(container).IgnoreError();
483493
}
484494

485495
void TypeCheckerBuilderImpl::SetExpressionContainer(

checker/internal/type_checker_builder_impl.h

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,6 @@
4040

4141
namespace cel::checker_internal {
4242

43-
class TypeCheckerBuilderImpl;
44-
4543
// Builder for TypeChecker instances.
4644
class TypeCheckerBuilderImpl : public TypeCheckerBuilder {
4745
public:
@@ -51,7 +49,18 @@ class TypeCheckerBuilderImpl : public TypeCheckerBuilder {
5149
const CheckerOptions& options)
5250
: options_(options),
5351
target_config_(&default_config_),
54-
descriptor_pool_(std::move(descriptor_pool)) {}
52+
template_env_(std::move(descriptor_pool)) {}
53+
54+
// Constructor for building an extended TypeChecker.
55+
explicit TypeCheckerBuilderImpl(const CheckerOptions& options,
56+
const TypeCheckEnv& template_env)
57+
: options_(options),
58+
target_config_(&default_config_),
59+
template_env_(template_env) {
60+
if (auto arena = template_env_.arena(); arena != nullptr) {
61+
type_arena_ = std::move(arena);
62+
}
63+
}
5564

5665
// Move only.
5766
TypeCheckerBuilderImpl(const TypeCheckerBuilderImpl&) = delete;
@@ -83,14 +92,14 @@ class TypeCheckerBuilderImpl : public TypeCheckerBuilder {
8392
const CheckerOptions& options() const override { return options_; }
8493

8594
google::protobuf::Arena* absl_nonnull arena() override {
86-
if (arena_ == nullptr) {
87-
arena_ = std::make_shared<google::protobuf::Arena>();
95+
if (type_arena_ == nullptr) {
96+
type_arena_ = std::make_shared<google::protobuf::Arena>();
8897
}
89-
return arena_.get();
98+
return type_arena_.get();
9099
}
91100

92101
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool() const override {
93-
return descriptor_pool_.get();
102+
return template_env_.descriptor_pool();
94103
}
95104

96105
private:
@@ -129,19 +138,21 @@ class TypeCheckerBuilderImpl : public TypeCheckerBuilder {
129138
absl::Status ApplyConfig(ConfigRecord config, const TypeCheckerSubset* subset,
130139
TypeCheckEnv& env);
131140

141+
absl::Status ConfigureTypeCheckEnv(TypeCheckEnv& env);
142+
132143
CheckerOptions options_;
133144
// Default target for configuration changes. Used for direct calls to
134145
// AddVariable, AddFunction, etc.
135146
ConfigRecord default_config_;
136147
// Active target for configuration changes.
137148
// This is used to track which library the change is made on behalf of.
138149
ConfigRecord* absl_nonnull target_config_;
139-
std::shared_ptr<const google::protobuf::DescriptorPool> descriptor_pool_;
140-
std::shared_ptr<google::protobuf::Arena> arena_;
150+
TypeCheckEnv template_env_;
151+
std::shared_ptr<google::protobuf::Arena> type_arena_;
141152
std::vector<CheckerLibrary> libraries_;
142153
absl::flat_hash_map<std::string, TypeCheckerSubset> subsets_;
143154
absl::flat_hash_set<std::string> library_ids_;
144-
ExpressionContainer expression_container_;
155+
absl::optional<ExpressionContainer> expression_container_;
145156
absl::optional<Type> expected_type_;
146157
};
147158

checker/internal/type_checker_impl.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,10 @@
3737
#include "checker/internal/format_type_name.h"
3838
#include "checker/internal/namespace_generator.h"
3939
#include "checker/internal/type_check_env.h"
40+
#include "checker/internal/type_checker_builder_impl.h"
4041
#include "checker/internal/type_inference_context.h"
4142
#include "checker/type_check_issue.h"
43+
#include "checker/type_checker_builder.h"
4244
#include "checker/validation_result.h"
4345
#include "common/ast.h"
4446
#include "common/ast_rewrite.h"
@@ -1326,4 +1328,8 @@ absl::StatusOr<ValidationResult> TypeCheckerImpl::Check(
13261328
return ValidationResult(std::move(ast), std::move(issues));
13271329
}
13281330

1331+
std::unique_ptr<TypeCheckerBuilder> TypeCheckerImpl::ToBuilder() const {
1332+
return std::make_unique<TypeCheckerBuilderImpl>(options_, env_);
1333+
}
1334+
13291335
} // namespace cel::checker_internal

checker/internal/type_checker_impl.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "checker/checker_options.h"
2323
#include "checker/internal/type_check_env.h"
2424
#include "checker/type_checker.h"
25+
#include "checker/type_checker_builder.h"
2526
#include "checker/validation_result.h"
2627
#include "common/ast.h"
2728
#include "google/protobuf/arena.h"
@@ -44,6 +45,8 @@ class TypeCheckerImpl : public TypeChecker {
4445
absl::StatusOr<ValidationResult> Check(
4546
std::unique_ptr<Ast> ast) const override;
4647

48+
std::unique_ptr<TypeCheckerBuilder> ToBuilder() const override;
49+
4750
private:
4851
TypeCheckEnv env_;
4952
google::protobuf::Arena type_arena_;

checker/internal/type_checker_impl_test.cc

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include "checker/internal/test_ast_helpers.h"
3434
#include "checker/internal/type_check_env.h"
3535
#include "checker/type_check_issue.h"
36+
#include "checker/type_checker_builder.h"
3637
#include "checker/validation_result.h"
3738
#include "common/ast.h"
3839
#include "common/container.h"
@@ -1413,6 +1414,44 @@ TEST(TypeCheckerImplTest, ExpectedTypeDoesntMatch) {
14131414
"expected type 'map(string, string)' but found 'map(string, int)'")));
14141415
}
14151416

1417+
TEST(TypeCheckerImplTest, ToBuilder) {
1418+
TypeCheckEnv env(GetSharedTestingDescriptorPool());
1419+
TypeCheckerImpl impl(std::move(env));
1420+
auto builder = impl.ToBuilder();
1421+
ASSERT_THAT(builder->AddVariable(MakeVariableDecl("x", IntType())), IsOk());
1422+
ASSERT_OK_AND_ASSIGN(auto new_checker, builder->Build());
1423+
1424+
ASSERT_OK_AND_ASSIGN(auto ast, MakeTestParsedAst("x"));
1425+
ASSERT_OK_AND_ASSIGN(ValidationResult result,
1426+
new_checker->Check(std::move(ast)));
1427+
EXPECT_TRUE(result.IsValid());
1428+
}
1429+
1430+
TEST(TypeCheckerImplTest, ToBuilderPropagatesArena) {
1431+
auto arena = std::make_shared<google::protobuf::Arena>();
1432+
1433+
TypeCheckEnv env(GetSharedTestingDescriptorPool());
1434+
env.set_arena(arena);
1435+
1436+
Type list_type = ListType(arena.get(), IntType());
1437+
ASSERT_TRUE(
1438+
env.InsertVariableIfAbsent(MakeVariableDecl("my_list", list_type)));
1439+
1440+
auto base_checker = std::make_unique<TypeCheckerImpl>(std::move(env));
1441+
1442+
std::unique_ptr<TypeCheckerBuilder> builder = base_checker->ToBuilder();
1443+
1444+
base_checker.reset();
1445+
arena.reset();
1446+
1447+
ASSERT_OK_AND_ASSIGN(auto derived_checker, builder->Build());
1448+
1449+
ASSERT_OK_AND_ASSIGN(auto ast, MakeTestParsedAst("my_list"));
1450+
ASSERT_OK_AND_ASSIGN(ValidationResult result,
1451+
derived_checker->Check(std::move(ast)));
1452+
EXPECT_TRUE(result.IsValid());
1453+
}
1454+
14161455
TEST(TypeCheckerImplTest, BadSourcePosition) {
14171456
TypeCheckEnv env(GetSharedTestingDescriptorPool());
14181457

checker/type_checker.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323

2424
namespace cel {
2525

26+
class TypeCheckerBuilder;
27+
2628
// TypeChecker interface.
2729
//
2830
// Checks references and type agreement for a parsed CEL expression.
@@ -43,6 +45,9 @@ class TypeChecker {
4345
virtual absl::StatusOr<ValidationResult> Check(
4446
std::unique_ptr<Ast> ast) const = 0;
4547

48+
// Returns a builder initialized with the configuration of this type checker.
49+
virtual std::unique_ptr<TypeCheckerBuilder> ToBuilder() const = 0;
50+
4651
// TODO(uncreated-issue/73): add overload for cref AST.
4752
};
4853

checker/type_checker_builder.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
namespace cel {
3636

3737
class TypeCheckerBuilder;
38-
class TypeCheckerBuilderImpl;
3938

4039
// Functional implementation to apply the library features to a
4140
// TypeCheckerBuilder.

0 commit comments

Comments
 (0)