Skip to content

Commit 0a2efd3

Browse files
jckingcopybara-github
authored andcommitted
Mandate passing proto2::DescriptorPool to the type checker
PiperOrigin-RevId: 689498942
1 parent a81711e commit 0a2efd3

15 files changed

+280
-117
lines changed

checker/BUILD

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,14 +91,18 @@ cc_library(
9191
"//common:decl",
9292
"//common:type",
9393
"//internal:status_macros",
94+
"//internal:well_known_types",
9495
"//parser:macro",
9596
"@com_google_absl//absl/base:no_destructor",
97+
"@com_google_absl//absl/base:nullability",
9698
"@com_google_absl//absl/container:flat_hash_map",
9799
"@com_google_absl//absl/container:flat_hash_set",
98100
"@com_google_absl//absl/functional:any_invocable",
101+
"@com_google_absl//absl/log:absl_check",
99102
"@com_google_absl//absl/status",
100103
"@com_google_absl//absl/status:statusor",
101104
"@com_google_absl//absl/strings",
105+
"@com_google_protobuf//:protobuf",
102106
],
103107
)
104108

@@ -112,6 +116,7 @@ cc_test(
112116
"//common:decl",
113117
"//common:type",
114118
"//internal:testing",
119+
"//internal:testing_descriptor_pool",
115120
"@com_google_absl//absl/status",
116121
"@com_google_absl//absl/status:status_matchers",
117122
],
@@ -150,6 +155,7 @@ cc_test(
150155
"//common:decl",
151156
"//common:type",
152157
"//internal:testing",
158+
"//internal:testing_descriptor_pool",
153159
"@com_google_absl//absl/status",
154160
"@com_google_absl//absl/status:status_matchers",
155161
"@com_google_protobuf//:protobuf",
@@ -185,11 +191,9 @@ cc_test(
185191
"//base/ast_internal:ast_impl",
186192
"//base/ast_internal:expr",
187193
"//checker/internal:test_ast_helpers",
188-
"//extensions/protobuf:value",
189194
"//internal:testing",
195+
"//internal:testing_descriptor_pool",
190196
"@com_google_absl//absl/status:status_matchers",
191197
"@com_google_absl//absl/strings",
192-
"@com_google_cel_spec//proto/test/v1/proto3:test_all_types_cc_proto",
193-
"@com_google_protobuf//:protobuf",
194198
],
195199
)

checker/internal/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ cc_test(
167167
"//extensions/protobuf:value",
168168
"//internal:status_macros",
169169
"//internal:testing",
170+
"//internal:testing_descriptor_pool",
170171
"@com_google_absl//absl/base:no_destructor",
171172
"@com_google_absl//absl/base:nullability",
172173
"@com_google_absl//absl/container:flat_hash_set",

checker/internal/type_check_env.cc

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "common/type_introspector.h"
3131
#include "internal/status_macros.h"
3232
#include "google/protobuf/arena.h"
33+
#include "google/protobuf/descriptor.h"
3334

3435
namespace cel::checker_internal {
3536

@@ -59,8 +60,21 @@ absl::Nullable<const FunctionDecl*> TypeCheckEnv::LookupFunction(
5960

6061
absl::StatusOr<absl::optional<Type>> TypeCheckEnv::LookupTypeName(
6162
TypeFactory& type_factory, absl::string_view name) const {
63+
{
64+
// Check the descriptor pool first, then fallback to custom type providers.
65+
absl::Nullable<const google::protobuf::Descriptor*> descriptor =
66+
descriptor_pool_->FindMessageTypeByName(name);
67+
if (descriptor != nullptr) {
68+
return Type::Message(descriptor);
69+
}
70+
absl::Nullable<const google::protobuf::EnumDescriptor*> enum_descriptor =
71+
descriptor_pool_->FindEnumTypeByName(name);
72+
if (enum_descriptor != nullptr) {
73+
return Type::Enum(enum_descriptor);
74+
}
75+
}
6276
const TypeCheckEnv* scope = this;
63-
while (scope != nullptr) {
77+
do {
6478
for (auto iter = type_providers_.rbegin(); iter != type_providers_.rend();
6579
++iter) {
6680
auto type = (*iter)->FindType(type_factory, name);
@@ -69,15 +83,34 @@ absl::StatusOr<absl::optional<Type>> TypeCheckEnv::LookupTypeName(
6983
}
7084
}
7185
scope = scope->parent_;
72-
}
86+
} while ((scope != nullptr));
7387
return absl::nullopt;
7488
}
7589

7690
absl::StatusOr<absl::optional<VariableDecl>> TypeCheckEnv::LookupEnumConstant(
7791
TypeFactory& type_factory, absl::string_view type,
7892
absl::string_view value) const {
93+
{
94+
// Check the descriptor pool first, then fallback to custom type providers.
95+
absl::Nullable<const google::protobuf::EnumDescriptor*> enum_descriptor =
96+
descriptor_pool_->FindEnumTypeByName(type);
97+
if (enum_descriptor != nullptr) {
98+
absl::Nullable<const google::protobuf::EnumValueDescriptor*> enum_value_descriptor =
99+
enum_descriptor->FindValueByName(value);
100+
if (enum_value_descriptor == nullptr) {
101+
return absl::nullopt;
102+
}
103+
auto decl =
104+
MakeVariableDecl(absl::StrCat(enum_descriptor->full_name(), ".",
105+
enum_value_descriptor->name()),
106+
Type::Enum(enum_descriptor));
107+
decl.set_value(
108+
Constant(static_cast<int64_t>(enum_value_descriptor->number())));
109+
return decl;
110+
}
111+
}
79112
const TypeCheckEnv* scope = this;
80-
while (scope != nullptr) {
113+
do {
81114
for (auto iter = type_providers_.rbegin(); iter != type_providers_.rend();
82115
++iter) {
83116
auto enum_constant = (*iter)->FindEnumConstant(type_factory, type, value);
@@ -95,7 +128,7 @@ absl::StatusOr<absl::optional<VariableDecl>> TypeCheckEnv::LookupEnumConstant(
95128
}
96129
}
97130
scope = scope->parent_;
98-
}
131+
} while (scope != nullptr);
99132
return absl::nullopt;
100133
}
101134

@@ -122,8 +155,25 @@ absl::StatusOr<absl::optional<VariableDecl>> TypeCheckEnv::LookupTypeConstant(
122155
absl::StatusOr<absl::optional<StructTypeField>> TypeCheckEnv::LookupStructField(
123156
TypeFactory& type_factory, absl::string_view type_name,
124157
absl::string_view field_name) const {
158+
{
159+
// Check the descriptor pool first, then fallback to custom type providers.
160+
absl::Nullable<const google::protobuf::Descriptor*> descriptor =
161+
descriptor_pool_->FindMessageTypeByName(type_name);
162+
if (descriptor != nullptr) {
163+
absl::Nullable<const google::protobuf::FieldDescriptor*> field_descriptor =
164+
descriptor->FindFieldByName(field_name);
165+
if (field_descriptor == nullptr) {
166+
field_descriptor = descriptor_pool_->FindExtensionByPrintableName(
167+
descriptor, field_name);
168+
if (field_descriptor == nullptr) {
169+
return absl::nullopt;
170+
}
171+
}
172+
return cel::MessageTypeField(field_descriptor);
173+
}
174+
}
125175
const TypeCheckEnv* scope = this;
126-
while (scope != nullptr) {
176+
do {
127177
// Check the type providers in reverse registration order.
128178
// Note: this doesn't allow for shadowing a type with a subset type of the
129179
// same name -- the parent type provider will still be considered when
@@ -137,7 +187,7 @@ absl::StatusOr<absl::optional<StructTypeField>> TypeCheckEnv::LookupStructField(
137187
}
138188
}
139189
scope = scope->parent_;
140-
}
190+
} while (scope != nullptr);
141191
return absl::nullopt;
142192
}
143193

checker/internal/type_check_env.h

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "common/type_factory.h"
3535
#include "common/type_introspector.h"
3636
#include "google/protobuf/arena.h"
37+
#include "google/protobuf/descriptor.h"
3738

3839
namespace cel::checker_internal {
3940

@@ -89,11 +90,12 @@ class TypeCheckEnv {
8990
using FunctionDeclPtr = absl::Nonnull<const FunctionDecl*>;
9091

9192
public:
92-
TypeCheckEnv() : container_(""), parent_(nullptr) {};
93-
94-
explicit TypeCheckEnv(const TypeCheckEnv* parent)
95-
: container_(parent != nullptr ? parent->container() : ""),
96-
parent_(parent) {}
93+
explicit TypeCheckEnv(
94+
absl::Nonnull<std::shared_ptr<const google::protobuf::DescriptorPool>>
95+
descriptor_pool)
96+
: descriptor_pool_(std::move(descriptor_pool)),
97+
container_(""),
98+
parent_(nullptr) {};
9799

98100
// Move-only.
99101
TypeCheckEnv(TypeCheckEnv&&) = default;
@@ -165,14 +167,28 @@ class TypeCheckEnv {
165167
TypeFactory& type_factory, absl::Nonnull<google::protobuf::Arena*> arena,
166168
absl::string_view type_name) const;
167169

168-
TypeCheckEnv MakeExtendedEnvironment() const { return TypeCheckEnv(this); }
169-
VariableScope MakeVariableScope() const { return VariableScope(*this); }
170+
TypeCheckEnv MakeExtendedEnvironment() const ABSL_ATTRIBUTE_LIFETIME_BOUND {
171+
return TypeCheckEnv(this);
172+
}
173+
VariableScope MakeVariableScope() const ABSL_ATTRIBUTE_LIFETIME_BOUND {
174+
return VariableScope(*this);
175+
}
176+
177+
absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool() const {
178+
return descriptor_pool_.get();
179+
}
170180

171181
private:
182+
explicit TypeCheckEnv(absl::Nonnull<const TypeCheckEnv*> parent)
183+
: descriptor_pool_(parent->descriptor_pool_),
184+
container_(parent != nullptr ? parent->container() : ""),
185+
parent_(parent) {}
186+
172187
absl::StatusOr<absl::optional<VariableDecl>> LookupEnumConstant(
173188
TypeFactory& type_factory, absl::string_view type,
174189
absl::string_view value) const;
175190

191+
absl::Nonnull<std::shared_ptr<const google::protobuf::DescriptorPool>> descriptor_pool_;
176192
std::string container_;
177193
absl::Nullable<const TypeCheckEnv*> parent_;
178194

0 commit comments

Comments
 (0)