Skip to content

Commit c8887b6

Browse files
jnthntatumcopybara-github
authored andcommitted
Add an optional protobuf Arena tied to TypeCheckerBuilder instances.
Intended to be used to make it easier to guarantee lifetime of types needed by the checker. PiperOrigin-RevId: 733896891
1 parent accb9a6 commit c8887b6

File tree

8 files changed

+56
-3
lines changed

8 files changed

+56
-3
lines changed

checker/BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,12 @@ cc_library(
8888
":type_checker",
8989
"//common:decl",
9090
"//common:type",
91+
"@com_google_absl//absl/base:nullability",
9192
"@com_google_absl//absl/functional:any_invocable",
9293
"@com_google_absl//absl/status",
9394
"@com_google_absl//absl/status:statusor",
9495
"@com_google_absl//absl/strings",
96+
"@com_google_protobuf//:protobuf",
9597
],
9698
)
9799

checker/internal/type_check_env.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,17 @@ class TypeCheckEnv {
179179
return descriptor_pool_.get();
180180
}
181181

182+
// Return an arena that can be used to allocate memory for types that will be
183+
// used by the TypeChecker being built.
184+
//
185+
// This is only intended to be used for configuration.
186+
absl::Nonnull<google::protobuf::Arena*> arena() {
187+
if (arena_ == nullptr) {
188+
arena_ = std::make_unique<google::protobuf::Arena>();
189+
}
190+
return arena_.get();
191+
}
192+
182193
private:
183194
explicit TypeCheckEnv(absl::Nonnull<const TypeCheckEnv*> parent)
184195
: descriptor_pool_(parent->descriptor_pool_),
@@ -189,6 +200,7 @@ class TypeCheckEnv {
189200
absl::string_view type, absl::string_view value) const;
190201

191202
absl::Nonnull<std::shared_ptr<const google::protobuf::DescriptorPool>> descriptor_pool_;
203+
absl::Nullable<std::unique_ptr<google::protobuf::Arena>> arena_;
192204
std::string container_;
193205
absl::Nullable<const TypeCheckEnv*> parent_;
194206

checker/internal/type_checker_builder_impl.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include "common/decl.h"
3333
#include "common/type.h"
3434
#include "common/type_introspector.h"
35+
#include "google/protobuf/arena.h"
3536
#include "google/protobuf/descriptor.h"
3637

3738
namespace cel::checker_internal {
@@ -71,6 +72,13 @@ class TypeCheckerBuilderImpl : public TypeCheckerBuilder {
7172

7273
const CheckerOptions& options() const override { return options_; }
7374

75+
absl::Nonnull<google::protobuf::Arena*> arena() override { return env_.arena(); }
76+
77+
absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool()
78+
const override {
79+
return env_.descriptor_pool();
80+
}
81+
7482
private:
7583
absl::Status AddContextDeclarationVariables(
7684
absl::Nonnull<const google::protobuf::Descriptor*> descriptor);

checker/type_checker.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ namespace cel {
2727
//
2828
// Checks references and type agreement for a parsed CEL expression.
2929
//
30-
// TODO: see Compiler for bundled parse and type check from a
31-
// source expression string.
30+
// See Compiler for bundled parse and type check from a source expression
31+
// string.
3232
class TypeChecker {
3333
public:
3434
virtual ~TypeChecker() = default;

checker/type_checker_builder.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <memory>
1919
#include <string>
2020

21+
#include "absl/base/nullability.h"
2122
#include "absl/functional/any_invocable.h"
2223
#include "absl/status/status.h"
2324
#include "absl/status/statusor.h"
@@ -27,6 +28,8 @@
2728
#include "common/decl.h"
2829
#include "common/type.h"
2930
#include "common/type_introspector.h"
31+
#include "google/protobuf/arena.h"
32+
#include "google/protobuf/descriptor.h"
3033

3134
namespace cel {
3235

@@ -106,6 +109,16 @@ class TypeCheckerBuilder {
106109
// This operation is destructive: the builder instance should not be used
107110
// after this method is called.
108111
virtual absl::StatusOr<std::unique_ptr<TypeChecker>> Build() && = 0;
112+
113+
// Returns a pointer to an arena that can be used to allocate memory for types
114+
// that will be used by the TypeChecker being built.
115+
//
116+
// On Build(), the arena is transferred to the TypeChecker being built.
117+
virtual absl::Nonnull<google::protobuf::Arena*> arena() = 0;
118+
119+
// The configured descriptor pool.
120+
virtual absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool()
121+
const = 0;
109122
};
110123

111124
} // namespace cel

checker/type_checker_builder_factory.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
21
// Copyright 2024 Google LLC
32
//
43
// Licensed under the Apache License, Version 2.0 (the "License");

checker/type_checker_builder_factory.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ namespace cel {
2626

2727
// Creates a new `TypeCheckerBuilder`.
2828
//
29+
// The builder implementation is thread-hostile and should only be used from a
30+
// single thread, but the resulting `TypeChecker` instance is thread-safe.
31+
//
2932
// When passing a raw pointer to a descriptor pool, the descriptor pool must
3033
// outlive the type checker builder and the type checker builder it creates.
3134
//

checker/type_checker_builder_factory_test.cc

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,22 @@ TEST(TypeCheckerBuilderTest, AddVariable) {
4949
EXPECT_TRUE(result.IsValid());
5050
}
5151

52+
TEST(TypeCheckerBuilderTest, AddComplexType) {
53+
ASSERT_OK_AND_ASSIGN(
54+
std::unique_ptr<TypeCheckerBuilder> builder,
55+
CreateTypeCheckerBuilder(GetSharedTestingDescriptorPool()));
56+
57+
MapType map_type(builder->arena(), StringType(), IntType());
58+
59+
ASSERT_THAT(builder->AddVariable(MakeVariableDecl("m", map_type)), IsOk());
60+
61+
ASSERT_OK_AND_ASSIGN(auto checker, std::move(*builder).Build());
62+
builder.reset();
63+
ASSERT_OK_AND_ASSIGN(auto ast, MakeTestParsedAst("m.foo"));
64+
ASSERT_OK_AND_ASSIGN(ValidationResult result, checker->Check(std::move(ast)));
65+
EXPECT_TRUE(result.IsValid());
66+
}
67+
5268
TEST(TypeCheckerBuilderTest, AddVariableRedeclaredError) {
5369
ASSERT_OK_AND_ASSIGN(
5470
std::unique_ptr<TypeCheckerBuilder> builder,

0 commit comments

Comments
 (0)