Skip to content

Commit 07b5609

Browse files
jnthntatumcopybara-github
authored andcommitted
Prototype: Refactor scope handling to allow wiring in the runtime representation of annotations.
PiperOrigin-RevId: 723628841
1 parent 148c393 commit 07b5609

28 files changed

+1520
-188
lines changed

checker/checker_options.h

+8
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
namespace cel {
1919

20+
enum class CheckerAnnotationSupport { kStrip, kRetain, kCheck };
21+
2022
// Options for enabling core type checker features.
2123
struct CheckerOptions {
2224
// Enable overloads for numeric comparisons across types.
@@ -55,6 +57,12 @@ struct CheckerOptions {
5557
// If exceeded, the checker will stop processing the ast and return
5658
// the current set of issues.
5759
int max_error_issues = 20;
60+
61+
// Annotation support level.
62+
//
63+
// Default behavior is to strip annotations.
64+
CheckerAnnotationSupport annotation_support =
65+
CheckerAnnotationSupport::kStrip;
5866
};
5967

6068
} // namespace cel

checker/internal/BUILD

+2
Original file line numberDiff line numberDiff line change
@@ -137,13 +137,15 @@ cc_library(
137137
"//common:source",
138138
"//common:type",
139139
"//common:type_kind",
140+
"//internal:annotations",
140141
"//internal:status_macros",
141142
"//parser:macro",
142143
"@com_google_absl//absl/algorithm:container",
143144
"@com_google_absl//absl/base:no_destructor",
144145
"@com_google_absl//absl/base:nullability",
145146
"@com_google_absl//absl/container:flat_hash_map",
146147
"@com_google_absl//absl/container:flat_hash_set",
148+
"@com_google_absl//absl/log:absl_log",
147149
"@com_google_absl//absl/status",
148150
"@com_google_absl//absl/status:statusor",
149151
"@com_google_absl//absl/strings",

checker/internal/type_check_env.cc

+13
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,19 @@ absl::Nullable<const VariableDecl*> TypeCheckEnv::LookupVariable(
4545
return nullptr;
4646
}
4747

48+
absl::Nullable<const AnnotationDecl*> TypeCheckEnv::LookupAnnotation(
49+
absl::string_view name) const {
50+
const TypeCheckEnv* scope = this;
51+
while (scope != nullptr) {
52+
if (auto it = scope->annotations_.find(name);
53+
it != scope->annotations_.end()) {
54+
return &it->second;
55+
}
56+
scope = scope->parent_;
57+
}
58+
return nullptr;
59+
}
60+
4861
absl::Nullable<const FunctionDecl*> TypeCheckEnv::LookupFunction(
4962
absl::string_view name) const {
5063
const TypeCheckEnv* scope = this;

checker/internal/type_check_env.h

+7-4
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,6 @@ class VariableScope {
8484
//
8585
// This class is thread-compatible.
8686
class TypeCheckEnv {
87-
private:
88-
using VariableDeclPtr = absl::Nonnull<const VariableDecl*>;
89-
using FunctionDeclPtr = absl::Nonnull<const FunctionDecl*>;
90-
9187
public:
9288
explicit TypeCheckEnv(
9389
absl::Nonnull<std::shared_ptr<const google::protobuf::DescriptorPool>>
@@ -130,6 +126,10 @@ class TypeCheckEnv {
130126
return variables_.insert({decl.name(), std::move(decl)}).second;
131127
}
132128

129+
bool InsertAnnotationIfAbsent(AnnotationDecl decl) {
130+
return annotations_.insert({decl.name(), std::move(decl)}).second;
131+
}
132+
133133
const absl::flat_hash_map<std::string, FunctionDecl>& functions() const {
134134
return functions_;
135135
}
@@ -158,6 +158,8 @@ class TypeCheckEnv {
158158
absl::string_view name) const;
159159
absl::Nullable<const FunctionDecl*> LookupFunction(
160160
absl::string_view name) const;
161+
absl::Nullable<const AnnotationDecl*> LookupAnnotation(
162+
absl::string_view name) const;
161163

162164
absl::StatusOr<absl::optional<Type>> LookupTypeName(
163165
absl::string_view name) const;
@@ -195,6 +197,7 @@ class TypeCheckEnv {
195197
// Maps fully qualified names to declarations.
196198
absl::flat_hash_map<std::string, VariableDecl> variables_;
197199
absl::flat_hash_map<std::string, FunctionDecl> functions_;
200+
absl::flat_hash_map<std::string, AnnotationDecl> annotations_;
198201

199202
// Type providers for custom types.
200203
std::vector<std::unique_ptr<TypeIntrospector>> type_providers_;

checker/internal/type_checker_builder_impl.cc

+11
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,17 @@ absl::Status TypeCheckerBuilderImpl::MergeFunction(const FunctionDecl& decl) {
205205
return absl::OkStatus();
206206
}
207207

208+
absl::Status TypeCheckerBuilderImpl::AddAnnotation(const AnnotationDecl& decl) {
209+
if (decl.name().empty()) {
210+
return absl::InvalidArgumentError("annotation name must not be empty");
211+
}
212+
if (!env_.InsertAnnotationIfAbsent(decl)) {
213+
return absl::AlreadyExistsError(
214+
absl::StrCat("annotation '", decl.name(), "' already exists"));
215+
}
216+
return absl::OkStatus();
217+
}
218+
208219
void TypeCheckerBuilderImpl::AddTypeProvider(
209220
std::unique_ptr<TypeIntrospector> provider) {
210221
env_.AddTypeProvider(std::move(provider));

checker/internal/type_checker_builder_impl.h

+1
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ class TypeCheckerBuilderImpl : public TypeCheckerBuilder {
6060
absl::Status AddVariable(const VariableDecl& decl) override;
6161
absl::Status AddContextDeclaration(absl::string_view type) override;
6262
absl::Status AddFunction(const FunctionDecl& decl) override;
63+
absl::Status AddAnnotation(const AnnotationDecl& decl) override;
6364

6465
void SetExpectedType(const Type& type) override;
6566

checker/internal/type_checker_impl.cc

+145-7
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
#include "common/source.h"
5252
#include "common/type.h"
5353
#include "common/type_kind.h"
54+
#include "internal/annotations.h"
5455
#include "internal/status_macros.h"
5556
#include "google/protobuf/arena.h"
5657

@@ -68,6 +69,10 @@ std::string FormatCandidate(absl::Span<const std::string> qualifiers) {
6869
return absl::StrJoin(qualifiers, ".");
6970
}
7071

72+
static const TraversalOptions kTraversalOptions = {
73+
/*.use_comprehension_callbacks=*/true,
74+
};
75+
7176
SourceLocation ComputeSourceLocation(const AstImpl& ast, int64_t expr_id) {
7277
const auto& source_info = ast.source_info();
7378
auto iter = source_info.positions().find(expr_id);
@@ -235,6 +240,8 @@ absl::StatusOr<AstType> FlattenType(const Type& type) {
235240
}
236241
}
237242

243+
using AnnotationMap = internal::AnnotationMap;
244+
238245
class ResolveVisitor : public AstVisitorBase {
239246
public:
240247
struct FunctionResolution {
@@ -247,13 +254,17 @@ class ResolveVisitor : public AstVisitorBase {
247254
const TypeCheckEnv& env, const AstImpl& ast,
248255
TypeInferenceContext& inference_context,
249256
std::vector<TypeCheckIssue>& issues,
257+
AnnotationMap& annotations,
258+
cel::CheckerAnnotationSupport annotation_support,
250259
absl::Nonnull<google::protobuf::Arena*> arena)
251260
: container_(container),
261+
annotation_support_(annotation_support),
252262
namespace_generator_(std::move(namespace_generator)),
253263
env_(&env),
254264
inference_context_(&inference_context),
255265
issues_(&issues),
256266
ast_(&ast),
267+
annotations_(&annotations),
257268
root_scope_(env.MakeVariableScope()),
258269
arena_(arena),
259270
current_scope_(&root_scope_) {}
@@ -265,6 +276,43 @@ class ResolveVisitor : public AstVisitorBase {
265276
return;
266277
}
267278
expr_stack_.pop_back();
279+
if (!status_.ok()) {
280+
return;
281+
}
282+
if (annotation_support_ != CheckerAnnotationSupport::kCheck) {
283+
return;
284+
}
285+
auto annotations = annotations_->find(expr.id());
286+
if (annotations == annotations_->end()) {
287+
return;
288+
}
289+
if (annotation_context_.has_value()) {
290+
issues_->push_back(
291+
TypeCheckIssue::CreateError(ComputeSourceLocation(*ast_, expr.id()),
292+
"Nested annotations are not supported."));
293+
return;
294+
}
295+
auto annotation_scope = current_scope_->MakeNestedScope();
296+
VariableScope* annotation_scope_ptr = annotation_scope.get();
297+
// bit of a misuse, but annotation scope is largely the same as for
298+
// comprehensions.
299+
comprehension_vars_.push_back(std::move(annotation_scope));
300+
301+
annotation_context_ = {current_scope_};
302+
current_scope_ = annotation_scope_ptr;
303+
Type annotated_expr_type = GetDeducedType(&expr);
304+
annotation_scope_ptr->InsertVariableIfAbsent(
305+
MakeVariableDecl("cel.annotated_value", annotated_expr_type));
306+
307+
// Note: this does not need to happen now during the main traversal, but
308+
// it's a easier to reason about for me. It's equally valid to just record
309+
// the relevant annotations and do a separate check pass later.
310+
for (const auto& annotation : annotations->second) {
311+
CheckAnnotation(annotation, expr, annotated_expr_type);
312+
}
313+
314+
current_scope_ = annotation_context_->parent;
315+
annotation_context_.reset();
268316
}
269317

270318
void PostVisitConst(const Expr& expr, const Constant& constant) override;
@@ -341,6 +389,10 @@ class ResolveVisitor : public AstVisitorBase {
341389
const FunctionDecl* decl;
342390
};
343391

392+
struct AnnotationContext {
393+
const VariableScope* parent;
394+
};
395+
344396
void ResolveSimpleIdentifier(const Expr& expr, absl::string_view name);
345397

346398
void ResolveQualifiedIdentifier(const Expr& expr,
@@ -459,12 +511,17 @@ class ResolveVisitor : public AstVisitorBase {
459511
return DynType();
460512
}
461513

514+
void CheckAnnotation(const internal::AnnotationRep& annotation_expr,
515+
const Expr& annotated_expr, const Type& annotated_type);
516+
462517
absl::string_view container_;
518+
CheckerAnnotationSupport annotation_support_;
463519
NamespaceGenerator namespace_generator_;
464520
absl::Nonnull<const TypeCheckEnv*> env_;
465521
absl::Nonnull<TypeInferenceContext*> inference_context_;
466522
absl::Nonnull<std::vector<TypeCheckIssue>*> issues_;
467523
absl::Nonnull<const ast_internal::AstImpl*> ast_;
524+
absl::Nonnull<AnnotationMap*> annotations_;
468525
VariableScope root_scope_;
469526
absl::Nonnull<google::protobuf::Arena*> arena_;
470527

@@ -479,6 +536,7 @@ class ResolveVisitor : public AstVisitorBase {
479536
absl::flat_hash_set<const Expr*> deferred_select_operations_;
480537
std::vector<std::unique_ptr<VariableScope>> comprehension_vars_;
481538
std::vector<ComprehensionScope> comprehension_scopes_;
539+
absl::optional<AnnotationContext> annotation_context_;
482540
absl::Status status_;
483541
int error_count_ = 0;
484542

@@ -535,6 +593,64 @@ void ResolveVisitor::PostVisitIdent(const Expr& expr, const IdentExpr& ident) {
535593
}
536594
}
537595

596+
void ResolveVisitor::CheckAnnotation(const internal::AnnotationRep& annotation,
597+
const Expr& annotated_expr,
598+
const Type& annotated_type) {
599+
const auto* annotation_decl = env_->LookupAnnotation(annotation.name);
600+
if (annotation_decl == nullptr) {
601+
ReportIssue(TypeCheckIssue::CreateError(
602+
ComputeSourceLocation(*ast_, annotated_expr.id()),
603+
absl::StrCat("undefined annotation '", annotation.name, "'")));
604+
return;
605+
}
606+
607+
// Checking if assignable to Dyn may influence the type inference so skip
608+
// here.
609+
if (!annotation_decl->applicable_type().IsDyn()) {
610+
if (!inference_context_->IsAssignable(annotated_type,
611+
annotation_decl->applicable_type())) {
612+
ReportIssue(TypeCheckIssue::CreateError(
613+
ComputeSourceLocation(*ast_, annotated_expr.id()),
614+
absl::StrCat(
615+
"annotation '", annotation.name, "' is not applicable to type '",
616+
inference_context_->FinalizeType(annotated_type).DebugString(),
617+
"'")));
618+
return;
619+
}
620+
}
621+
622+
if (annotation.inspect_only) {
623+
// Nothing to do -- the value expression is not intended to be evaluated.
624+
// Examples are for things like a pointer to another file if the
625+
// subexpression is inlined from somewhere else.
626+
return;
627+
}
628+
629+
// TODO - re-entrant traversal bypasses the complexity limits.
630+
AstTraverse(*annotation.value_expr, *this, kTraversalOptions);
631+
632+
if (!status_.ok()) {
633+
return;
634+
}
635+
636+
Type value_expression_type = GetDeducedType(annotation.value_expr);
637+
638+
if (!annotation_decl->expected_type().IsDyn()) {
639+
if (!inference_context_->IsAssignable(value_expression_type,
640+
annotation_decl->expected_type())) {
641+
ReportIssue(TypeCheckIssue::CreateError(
642+
ComputeSourceLocation(*ast_, annotated_expr.id()),
643+
absl::StrCat("annotation '", annotation.name,
644+
"' value expression type '",
645+
inference_context_->FinalizeType(value_expression_type)
646+
.DebugString(),
647+
"' is not assignable to '",
648+
annotation_decl->expected_type().DebugString(), "'")));
649+
return;
650+
}
651+
}
652+
}
653+
538654
void ResolveVisitor::PostVisitConst(const Expr& expr,
539655
const Constant& constant) {
540656
switch (constant.kind().index()) {
@@ -1276,13 +1392,28 @@ absl::StatusOr<ValidationResult> TypeCheckerImpl::Check(
12761392

12771393
TypeInferenceContext type_inference_context(
12781394
&type_arena, options_.enable_legacy_null_assignment);
1395+
1396+
internal::AnnotationMap annotation_exprs;
1397+
Expr* root = &ast_impl.root_expr();
1398+
if (ast_impl.root_expr().has_call_expr() &&
1399+
ast_impl.root_expr().call_expr().function() == "cel.@annotated" &&
1400+
ast_impl.root_expr().call_expr().args().size() == 2) {
1401+
if (options_.annotation_support == CheckerAnnotationSupport::kStrip) {
1402+
ast_impl.root_expr() =
1403+
std::move(ast_impl.root_expr().mutable_call_expr().mutable_args()[0]);
1404+
root = &ast_impl.root_expr();
1405+
} else {
1406+
annotation_exprs = internal::BuildAnnotationMap(ast_impl);
1407+
root = &ast_impl.root_expr().mutable_call_expr().mutable_args()[0];
1408+
}
1409+
}
1410+
12791411
ResolveVisitor visitor(env_.container(), std::move(generator), env_, ast_impl,
1280-
type_inference_context, issues, &type_arena);
1412+
type_inference_context, issues, annotation_exprs,
1413+
options_.annotation_support, &type_arena);
12811414

1282-
TraversalOptions opts;
1283-
opts.use_comprehension_callbacks = true;
12841415
bool error_limit_reached = false;
1285-
auto traversal = AstTraversal::Create(ast_impl.root_expr(), opts);
1416+
auto traversal = AstTraversal::Create(*root, kTraversalOptions);
12861417

12871418
for (int step = 0; step < options_.max_expression_node_count * 2; ++step) {
12881419
bool has_next = traversal.Step(visitor);
@@ -1300,7 +1431,7 @@ absl::StatusOr<ValidationResult> TypeCheckerImpl::Check(
13001431

13011432
if (!traversal.IsDone() && !error_limit_reached) {
13021433
return absl::InvalidArgumentError(
1303-
absl::StrCat("Maximum expression node count exceeded: ",
1434+
absl::StrCat("maximum expression node count exceeded: ",
13041435
options_.max_expression_node_count));
13051436
}
13061437

@@ -1309,7 +1440,7 @@ absl::StatusOr<ValidationResult> TypeCheckerImpl::Check(
13091440
{}, absl::StrCat("maximum number of ERROR issues exceeded: ",
13101441
options_.max_error_issues)));
13111442
} else if (env_.expected_type().has_value()) {
1312-
visitor.AssertExpectedType(ast_impl.root_expr(), *env_.expected_type());
1443+
visitor.AssertExpectedType(*root, *env_.expected_type());
13131444
}
13141445

13151446
// If any issues are errors, return without an AST.
@@ -1324,7 +1455,14 @@ absl::StatusOr<ValidationResult> TypeCheckerImpl::Check(
13241455
// been invalidated by other updates.
13251456
ResolveRewriter rewriter(visitor, type_inference_context, options_,
13261457
ast_impl.reference_map(), ast_impl.type_map());
1327-
AstRewrite(ast_impl.root_expr(), rewriter);
1458+
AstRewrite(*root, rewriter);
1459+
if (options_.annotation_support == CheckerAnnotationSupport::kCheck) {
1460+
for (auto& annotations : annotation_exprs) {
1461+
for (auto& annotation : annotations.second) {
1462+
AstRewrite(*annotation.value_expr, rewriter);
1463+
}
1464+
}
1465+
}
13281466

13291467
CEL_RETURN_IF_ERROR(rewriter.status());
13301468

0 commit comments

Comments
 (0)