Skip to content

Commit feaecdb

Browse files
jckingcopybara-github
jcking
authored and
copybara-github
committed
Remove TypeFactory from TypeIntrospector
PiperOrigin-RevId: 690748051
1 parent 29818b8 commit feaecdb

25 files changed

+83
-203
lines changed

‎checker/internal/BUILD‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,9 @@ cc_library(
127127
"//common:constant",
128128
"//common:decl",
129129
"//common:expr",
130-
"//common:memory",
131130
"//common:source",
132131
"//common:type",
133132
"//common:type_kind",
134-
"//extensions/protobuf:memory_manager",
135133
"//internal:status_macros",
136134
"@com_google_absl//absl/base:nullability",
137135
"@com_google_absl//absl/container:flat_hash_map",

‎checker/internal/type_check_env.cc‎

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
#include "common/constant.h"
2727
#include "common/decl.h"
2828
#include "common/type.h"
29-
#include "common/type_factory.h"
3029
#include "common/type_introspector.h"
3130
#include "internal/status_macros.h"
3231
#include "google/protobuf/arena.h"
@@ -59,7 +58,7 @@ absl::Nullable<const FunctionDecl*> TypeCheckEnv::LookupFunction(
5958
}
6059

6160
absl::StatusOr<absl::optional<Type>> TypeCheckEnv::LookupTypeName(
62-
TypeFactory& type_factory, absl::string_view name) const {
61+
absl::string_view name) const {
6362
{
6463
// Check the descriptor pool first, then fallback to custom type providers.
6564
absl::Nullable<const google::protobuf::Descriptor*> descriptor =
@@ -77,7 +76,7 @@ absl::StatusOr<absl::optional<Type>> TypeCheckEnv::LookupTypeName(
7776
do {
7877
for (auto iter = type_providers_.rbegin(); iter != type_providers_.rend();
7978
++iter) {
80-
auto type = (*iter)->FindType(type_factory, name);
79+
auto type = (*iter)->FindType(name);
8180
if (!type.ok() || type->has_value()) {
8281
return type;
8382
}
@@ -88,8 +87,7 @@ absl::StatusOr<absl::optional<Type>> TypeCheckEnv::LookupTypeName(
8887
}
8988

9089
absl::StatusOr<absl::optional<VariableDecl>> TypeCheckEnv::LookupEnumConstant(
91-
TypeFactory& type_factory, absl::string_view type,
92-
absl::string_view value) const {
90+
absl::string_view type, absl::string_view value) const {
9391
{
9492
// Check the descriptor pool first, then fallback to custom type providers.
9593
absl::Nullable<const google::protobuf::EnumDescriptor*> enum_descriptor =
@@ -113,7 +111,7 @@ absl::StatusOr<absl::optional<VariableDecl>> TypeCheckEnv::LookupEnumConstant(
113111
do {
114112
for (auto iter = type_providers_.rbegin(); iter != type_providers_.rend();
115113
++iter) {
116-
auto enum_constant = (*iter)->FindEnumConstant(type_factory, type, value);
114+
auto enum_constant = (*iter)->FindEnumConstant(type, value);
117115
if (!enum_constant.ok()) {
118116
return enum_constant.status();
119117
}
@@ -133,10 +131,8 @@ absl::StatusOr<absl::optional<VariableDecl>> TypeCheckEnv::LookupEnumConstant(
133131
}
134132

135133
absl::StatusOr<absl::optional<VariableDecl>> TypeCheckEnv::LookupTypeConstant(
136-
TypeFactory& type_factory, absl::Nonnull<google::protobuf::Arena*> arena,
137-
absl::string_view name) const {
138-
CEL_ASSIGN_OR_RETURN(absl::optional<Type> type,
139-
LookupTypeName(type_factory, name));
134+
absl::Nonnull<google::protobuf::Arena*> arena, absl::string_view name) const {
135+
CEL_ASSIGN_OR_RETURN(absl::optional<Type> type, LookupTypeName(name));
140136
if (type.has_value()) {
141137
return MakeVariableDecl(std::string(type->name()), TypeType(arena, *type));
142138
}
@@ -145,16 +141,14 @@ absl::StatusOr<absl::optional<VariableDecl>> TypeCheckEnv::LookupTypeConstant(
145141
size_t last_dot = name.rfind('.');
146142
absl::string_view enum_name_candidate = name.substr(0, last_dot);
147143
absl::string_view value_name_candidate = name.substr(last_dot + 1);
148-
return LookupEnumConstant(type_factory, enum_name_candidate,
149-
value_name_candidate);
144+
return LookupEnumConstant(enum_name_candidate, value_name_candidate);
150145
}
151146

152147
return absl::nullopt;
153148
}
154149

155150
absl::StatusOr<absl::optional<StructTypeField>> TypeCheckEnv::LookupStructField(
156-
TypeFactory& type_factory, absl::string_view type_name,
157-
absl::string_view field_name) const {
151+
absl::string_view type_name, absl::string_view field_name) const {
158152
{
159153
// Check the descriptor pool first, then fallback to custom type providers.
160154
absl::Nullable<const google::protobuf::Descriptor*> descriptor =
@@ -180,8 +174,8 @@ absl::StatusOr<absl::optional<StructTypeField>> TypeCheckEnv::LookupStructField(
180174
// checking field accesses.
181175
for (auto iter = type_providers_.rbegin(); iter != type_providers_.rend();
182176
++iter) {
183-
auto field_info = (*iter)->FindStructTypeFieldByName(
184-
type_factory, type_name, field_name);
177+
auto field_info =
178+
(*iter)->FindStructTypeFieldByName(type_name, field_name);
185179
if (!field_info.ok() || field_info->has_value()) {
186180
return field_info;
187181
}

‎checker/internal/type_check_env.h‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
#include "common/constant.h"
3232
#include "common/decl.h"
3333
#include "common/type.h"
34-
#include "common/type_factory.h"
3534
#include "common/type_introspector.h"
3635
#include "google/protobuf/arena.h"
3736
#include "google/protobuf/descriptor.h"
@@ -161,15 +160,13 @@ class TypeCheckEnv {
161160
absl::string_view name) const;
162161

163162
absl::StatusOr<absl::optional<Type>> LookupTypeName(
164-
TypeFactory& type_factory, absl::string_view name) const;
163+
absl::string_view name) const;
165164

166165
absl::StatusOr<absl::optional<StructTypeField>> LookupStructField(
167-
TypeFactory& type_factory, absl::string_view type_name,
168-
absl::string_view field_name) const;
166+
absl::string_view type_name, absl::string_view field_name) const;
169167

170168
absl::StatusOr<absl::optional<VariableDecl>> LookupTypeConstant(
171-
TypeFactory& type_factory, absl::Nonnull<google::protobuf::Arena*> arena,
172-
absl::string_view type_name) const;
169+
absl::Nonnull<google::protobuf::Arena*> arena, absl::string_view type_name) const;
173170

174171
TypeCheckEnv MakeExtendedEnvironment() const ABSL_ATTRIBUTE_LIFETIME_BOUND {
175172
return TypeCheckEnv(this);
@@ -189,8 +186,7 @@ class TypeCheckEnv {
189186
parent_(parent) {}
190187

191188
absl::StatusOr<absl::optional<VariableDecl>> LookupEnumConstant(
192-
TypeFactory& type_factory, absl::string_view type,
193-
absl::string_view value) const;
189+
absl::string_view type, absl::string_view value) const;
194190

195191
absl::Nonnull<std::shared_ptr<const google::protobuf::DescriptorPool>> descriptor_pool_;
196192
std::string container_;

‎checker/internal/type_checker_impl.cc‎

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,9 @@
4848
#include "common/constant.h"
4949
#include "common/decl.h"
5050
#include "common/expr.h"
51-
#include "common/memory.h"
5251
#include "common/source.h"
5352
#include "common/type.h"
54-
#include "common/type_factory.h"
5553
#include "common/type_kind.h"
56-
#include "extensions/protobuf/memory_manager.h"
5754
#include "internal/status_macros.h"
5855
#include "google/protobuf/arena.h"
5956

@@ -67,19 +64,6 @@ using Severity = TypeCheckIssue::Severity;
6764

6865
constexpr const char kOptionalSelect[] = "_?._";
6966

70-
class TrivialTypeFactory : public TypeFactory {
71-
public:
72-
explicit TrivialTypeFactory(absl::Nonnull<google::protobuf::Arena*> arena)
73-
: arena_(arena) {}
74-
75-
MemoryManagerRef GetMemoryManager() const override {
76-
return extensions::ProtoMemoryManagerRef(arena_);
77-
}
78-
79-
private:
80-
absl::Nonnull<google::protobuf::Arena*> arena_;
81-
};
82-
8367
std::string FormatCandidate(absl::Span<const std::string> qualifiers) {
8468
return absl::StrJoin(qualifiers, ".");
8569
}
@@ -253,7 +237,7 @@ class ResolveVisitor : public AstVisitorBase {
253237
const TypeCheckEnv& env, const AstImpl& ast,
254238
TypeInferenceContext& inference_context,
255239
std::vector<TypeCheckIssue>& issues,
256-
absl::Nonnull<google::protobuf::Arena*> arena, TypeFactory& type_factory)
240+
absl::Nonnull<google::protobuf::Arena*> arena)
257241
: container_(container),
258242
namespace_generator_(std::move(namespace_generator)),
259243
env_(&env),
@@ -262,7 +246,6 @@ class ResolveVisitor : public AstVisitorBase {
262246
ast_(&ast),
263247
root_scope_(env.MakeVariableScope()),
264248
arena_(arena),
265-
type_factory_(&type_factory),
266249
current_scope_(&root_scope_) {}
267250

268251
void PreVisitExpr(const Expr& expr) override { expr_stack_.push_back(&expr); }
@@ -408,7 +391,7 @@ class ResolveVisitor : public AstVisitorBase {
408391
// Lookup message type by name to support WellKnownType creation.
409392
CEL_ASSIGN_OR_RETURN(
410393
absl::optional<StructTypeField> field_info,
411-
env_->LookupStructField(*type_factory_, resolved_name, field.name()));
394+
env_->LookupStructField(resolved_name, field.name()));
412395
if (!field_info.has_value()) {
413396
ReportUndefinedField(field.id(), field.name(), resolved_name);
414397
continue;
@@ -455,7 +438,6 @@ class ResolveVisitor : public AstVisitorBase {
455438
absl::Nonnull<const ast_internal::AstImpl*> ast_;
456439
VariableScope root_scope_;
457440
absl::Nonnull<google::protobuf::Arena*> arena_;
458-
absl::Nonnull<TypeFactory*> type_factory_;
459441

460442
// state tracking for the traversal.
461443
const VariableScope* current_scope_;
@@ -669,7 +651,7 @@ void ResolveVisitor::PostVisitStruct(const Expr& expr,
669651
Type resolved_type;
670652
namespace_generator_.GenerateCandidates(
671653
create_struct.name(), [&](const absl::string_view name) {
672-
auto type = env_->LookupTypeName(*type_factory_, name);
654+
auto type = env_->LookupTypeName(name);
673655
if (!type.ok()) {
674656
status.Update(type.status());
675657
return false;
@@ -938,7 +920,7 @@ absl::Nullable<const VariableDecl*> ResolveVisitor::LookupIdentifier(
938920
return decl;
939921
}
940922
absl::StatusOr<absl::optional<VariableDecl>> constant =
941-
env_->LookupTypeConstant(*type_factory_, arena_, name);
923+
env_->LookupTypeConstant(arena_, name);
942924

943925
if (!constant.ok()) {
944926
status_.Update(constant.status());
@@ -1032,8 +1014,7 @@ absl::optional<Type> ResolveVisitor::CheckFieldType(int64_t id,
10321014
switch (operand_type.kind()) {
10331015
case TypeKind::kStruct: {
10341016
StructType struct_type = operand_type.GetStruct();
1035-
auto field_info =
1036-
env_->LookupStructField(*type_factory_, struct_type.name(), field);
1017+
auto field_info = env_->LookupStructField(struct_type.name(), field);
10371018
if (!field_info.ok()) {
10381019
status_.Update(field_info.status());
10391020
return absl::nullopt;
@@ -1228,10 +1209,8 @@ absl::StatusOr<ValidationResult> TypeCheckerImpl::Check(
12281209

12291210
TypeInferenceContext type_inference_context(
12301211
&type_arena, options_.enable_legacy_null_assignment);
1231-
TrivialTypeFactory type_factory(&type_arena);
12321212
ResolveVisitor visitor(env_.container(), std::move(generator), env_, ast_impl,
1233-
type_inference_context, issues, &type_arena,
1234-
type_factory);
1213+
type_inference_context, issues, &type_arena);
12351214

12361215
TraversalOptions opts;
12371216
opts.use_comprehension_callbacks = true;

‎common/type_factory.h‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,27 +15,14 @@
1515
#ifndef THIRD_PARTY_CEL_CPP_COMMON_TYPE_FACTORY_H_
1616
#define THIRD_PARTY_CEL_CPP_COMMON_TYPE_FACTORY_H_
1717

18-
#include "common/memory.h"
19-
2018
namespace cel {
2119

22-
namespace common_internal {
23-
class PiecewiseValueManager;
24-
}
25-
2620
// `TypeFactory` is the preferred way for constructing compound types such as
2721
// lists, maps, structs, and opaques. It caches types and avoids constructing
2822
// them multiple times.
2923
class TypeFactory {
3024
public:
3125
virtual ~TypeFactory() = default;
32-
33-
// Returns a `MemoryManagerRef` which is used to manage memory for internal
34-
// data structures as well as created types.
35-
virtual MemoryManagerRef GetMemoryManager() const = 0;
36-
37-
protected:
38-
friend class common_internal::PiecewiseValueManager;
3926
};
4027

4128
} // namespace cel

‎common/type_introspector.cc‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -214,49 +214,47 @@ const WellKnownTypesMap& GetWellKnownTypesMap() {
214214
} // namespace
215215

216216
absl::StatusOr<absl::optional<Type>> TypeIntrospector::FindType(
217-
TypeFactory& type_factory, absl::string_view name) const {
217+
absl::string_view name) const {
218218
const auto& well_known_types = GetWellKnownTypesMap();
219219
if (auto it = well_known_types.find(name); it != well_known_types.end()) {
220220
return it->second.type;
221221
}
222-
return FindTypeImpl(type_factory, name);
222+
return FindTypeImpl(name);
223223
}
224224

225225
absl::StatusOr<absl::optional<TypeIntrospector::EnumConstant>>
226-
TypeIntrospector::FindEnumConstant(TypeFactory& type_factory,
227-
absl::string_view type,
226+
TypeIntrospector::FindEnumConstant(absl::string_view type,
228227
absl::string_view value) const {
229228
if (type == "google.protobuf.NullValue" && value == "NULL_VALUE") {
230229
return EnumConstant{NullType{}, "google.protobuf.NullValue", "NULL_VALUE",
231230
0};
232231
}
233-
return FindEnumConstantImpl(type_factory, type, value);
232+
return FindEnumConstantImpl(type, value);
234233
}
235234

236235
absl::StatusOr<absl::optional<StructTypeField>>
237-
TypeIntrospector::FindStructTypeFieldByName(TypeFactory& type_factory,
238-
absl::string_view type,
236+
TypeIntrospector::FindStructTypeFieldByName(absl::string_view type,
239237
absl::string_view name) const {
240238
const auto& well_known_types = GetWellKnownTypesMap();
241239
if (auto it = well_known_types.find(type); it != well_known_types.end()) {
242240
return it->second.FieldByName(name);
243241
}
244-
return FindStructTypeFieldByNameImpl(type_factory, type, name);
242+
return FindStructTypeFieldByNameImpl(type, name);
245243
}
246244

247245
absl::StatusOr<absl::optional<Type>> TypeIntrospector::FindTypeImpl(
248-
TypeFactory&, absl::string_view) const {
246+
absl::string_view) const {
249247
return absl::nullopt;
250248
}
251249

252250
absl::StatusOr<absl::optional<TypeIntrospector::EnumConstant>>
253-
TypeIntrospector::FindEnumConstantImpl(TypeFactory&, absl::string_view,
251+
TypeIntrospector::FindEnumConstantImpl(absl::string_view,
254252
absl::string_view) const {
255253
return absl::nullopt;
256254
}
257255

258256
absl::StatusOr<absl::optional<StructTypeField>>
259-
TypeIntrospector::FindStructTypeFieldByNameImpl(TypeFactory&, absl::string_view,
257+
TypeIntrospector::FindStructTypeFieldByNameImpl(absl::string_view,
260258
absl::string_view) const {
261259
return absl::nullopt;
262260
}

‎common/type_introspector.h‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -46,40 +46,34 @@ class TypeIntrospector {
4646
virtual ~TypeIntrospector() = default;
4747

4848
// `FindType` find the type corresponding to name `name`.
49-
absl::StatusOr<absl::optional<Type>> FindType(TypeFactory& type_factory,
50-
absl::string_view name) const;
49+
absl::StatusOr<absl::optional<Type>> FindType(absl::string_view name) const;
5150

5251
// `FindEnumConstant` find a fully qualified enumerator name `name` in enum
5352
// type `type`.
5453
absl::StatusOr<absl::optional<EnumConstant>> FindEnumConstant(
55-
TypeFactory& type_factory, absl::string_view type,
56-
absl::string_view value) const;
54+
absl::string_view type, absl::string_view value) const;
5755

5856
// `FindStructTypeFieldByName` find the name, number, and type of the field
5957
// `name` in type `type`.
6058
absl::StatusOr<absl::optional<StructTypeField>> FindStructTypeFieldByName(
61-
TypeFactory& type_factory, absl::string_view type,
62-
absl::string_view name) const;
59+
absl::string_view type, absl::string_view name) const;
6360

6461
// `FindStructTypeFieldByName` find the name, number, and type of the field
6562
// `name` in struct type `type`.
6663
absl::StatusOr<absl::optional<StructTypeField>> FindStructTypeFieldByName(
67-
TypeFactory& type_factory, const StructType& type,
68-
absl::string_view name) const {
69-
return FindStructTypeFieldByName(type_factory, type.name(), name);
64+
const StructType& type, absl::string_view name) const {
65+
return FindStructTypeFieldByName(type.name(), name);
7066
}
7167

7268
protected:
7369
virtual absl::StatusOr<absl::optional<Type>> FindTypeImpl(
74-
TypeFactory& type_factory, absl::string_view name) const;
70+
absl::string_view name) const;
7571

7672
virtual absl::StatusOr<absl::optional<EnumConstant>> FindEnumConstantImpl(
77-
TypeFactory& type_factory, absl::string_view type,
78-
absl::string_view value) const;
73+
absl::string_view type, absl::string_view value) const;
7974

8075
virtual absl::StatusOr<absl::optional<StructTypeField>>
81-
FindStructTypeFieldByNameImpl(TypeFactory& type_factory,
82-
absl::string_view type,
76+
FindStructTypeFieldByNameImpl(absl::string_view type,
8377
absl::string_view name) const;
8478
};
8579

‎common/type_manager.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ Shared<TypeManager> NewThreadCompatibleTypeManager(
2727
Shared<TypeIntrospector> type_introspector) {
2828
return memory_manager
2929
.MakeShared<common_internal::ThreadCompatibleTypeManager>(
30-
memory_manager, std::move(type_introspector));
30+
std::move(type_introspector));
3131
}
3232

3333
} // namespace cel

0 commit comments

Comments
 (0)