Skip to content

Commit 9996e99

Browse files
jnthntatumcopybara-github
authored andcommitted
Remove downcasting from cel::Ast to AstImpl.
Fix code that was assuming that a cel::Ast is always a cel::AstImpl. Will just be using Ast going forward. PiperOrigin-RevId: 800211146
1 parent 7aba412 commit 9996e99

30 files changed

Lines changed: 194 additions & 328 deletions

checker/internal/BUILD

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,6 @@ cc_test(
176176
"//common:expr",
177177
"//common:source",
178178
"//common:type",
179-
"//common/ast:ast_impl",
180179
"//common/ast:expr",
181180
"//internal:status_macros",
182181
"//internal:testing",

checker/internal/test_ast_helpers_test.cc

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,16 @@
1919
#include "absl/status/status.h"
2020
#include "absl/status/status_matchers.h"
2121
#include "common/ast.h"
22-
#include "common/ast/ast_impl.h"
2322
#include "internal/testing.h"
2423

2524
namespace cel::checker_internal {
2625
namespace {
2726

2827
using ::absl_testing::StatusIs;
29-
using ::cel::ast_internal::AstImpl;
3028

3129
TEST(MakeTestParsedAstTest, Works) {
3230
ASSERT_OK_AND_ASSIGN(std::unique_ptr<Ast> ast, MakeTestParsedAst("123"));
33-
AstImpl& impl = AstImpl::CastFromPublicAst(*ast);
34-
EXPECT_TRUE(impl.root_expr().has_const_expr());
31+
EXPECT_TRUE(ast->root_expr().has_const_expr());
3532
}
3633

3734
TEST(MakeTestParsedAstTest, ForwardsParseError) {

checker/internal/type_checker_builder_impl_test.cc

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ namespace {
4141

4242
using ::absl_testing::IsOk;
4343
using ::absl_testing::StatusIs;
44-
using ::cel::ast_internal::AstImpl;
4544

4645
using AstType = cel::ast_internal::Type;
4746

@@ -67,9 +66,7 @@ TEST_P(ContextDeclsFieldsDefinedTest, ContextDeclsFieldsDefined) {
6766

6867
ASSERT_TRUE(result.IsValid());
6968

70-
const auto& ast_impl = AstImpl::CastFromPublicAst(*result.GetAst());
71-
72-
EXPECT_EQ(ast_impl.GetReturnType(), GetParam().expected_type);
69+
EXPECT_EQ(result.GetAst()->GetReturnType(), GetParam().expected_type);
7370
}
7471

7572
INSTANTIATE_TEST_SUITE_P(
@@ -324,9 +321,9 @@ TEST(TypeCheckerBuilderImplTest, ReplaceVariable) {
324321

325322
ASSERT_TRUE(result.IsValid());
326323

327-
const auto& ast_impl = AstImpl::CastFromPublicAst(*result.GetAst());
324+
const auto& checked_ast = *result.GetAst();
328325

329-
EXPECT_EQ(ast_impl.GetReturnType(),
326+
EXPECT_EQ(checked_ast.GetReturnType(),
330327
AstType(ast_internal::PrimitiveType::kString));
331328
}
332329

checker/internal/type_checker_impl.cc

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,6 @@
5858
namespace cel::checker_internal {
5959
namespace {
6060

61-
using cel::ast_internal::AstImpl;
62-
6361
using AstType = cel::ast_internal::Type;
6462
using Severity = TypeCheckIssue::Severity;
6563

@@ -69,7 +67,7 @@ std::string FormatCandidate(absl::Span<const std::string> qualifiers) {
6967
return absl::StrJoin(qualifiers, ".");
7068
}
7169

72-
SourceLocation ComputeSourceLocation(const AstImpl& ast, int64_t expr_id) {
70+
SourceLocation ComputeSourceLocation(const Ast& ast, int64_t expr_id) {
7371
const auto& source_info = ast.source_info();
7472
auto iter = source_info.positions().find(expr_id);
7573
if (iter == source_info.positions().end()) {
@@ -248,7 +246,7 @@ class ResolveVisitor : public AstVisitorBase {
248246

249247
ResolveVisitor(absl::string_view container,
250248
NamespaceGenerator namespace_generator,
251-
const TypeCheckEnv& env, const AstImpl& ast,
249+
const TypeCheckEnv& env, const Ast& ast,
252250
TypeInferenceContext& inference_context,
253251
std::vector<TypeCheckIssue>& issues,
254252
google::protobuf::Arena* absl_nonnull arena)
@@ -468,7 +466,7 @@ class ResolveVisitor : public AstVisitorBase {
468466
const TypeCheckEnv* absl_nonnull env_;
469467
TypeInferenceContext* absl_nonnull inference_context_;
470468
std::vector<TypeCheckIssue>* absl_nonnull issues_;
471-
const ast_internal::AstImpl* absl_nonnull ast_;
469+
const Ast* absl_nonnull ast_;
472470
VariableScope root_scope_;
473471
google::protobuf::Arena* absl_nonnull arena_;
474472

@@ -1198,8 +1196,7 @@ class ResolveRewriter : public AstRewriterBase {
11981196
explicit ResolveRewriter(const ResolveVisitor& visitor,
11991197
const TypeInferenceContext& inference_context,
12001198
const CheckerOptions& options,
1201-
AstImpl::ReferenceMap& references,
1202-
AstImpl::TypeMap& types)
1199+
Ast::ReferenceMap& references, Ast::TypeMap& types)
12031200
: visitor_(visitor),
12041201
inference_context_(inference_context),
12051202
reference_map_(references),
@@ -1264,16 +1261,15 @@ class ResolveRewriter : public AstRewriterBase {
12641261
absl::Status status_;
12651262
const ResolveVisitor& visitor_;
12661263
const TypeInferenceContext& inference_context_;
1267-
AstImpl::ReferenceMap& reference_map_;
1268-
AstImpl::TypeMap& type_map_;
1264+
Ast::ReferenceMap& reference_map_;
1265+
Ast::TypeMap& type_map_;
12691266
const CheckerOptions& options_;
12701267
};
12711268

12721269
} // namespace
12731270

12741271
absl::StatusOr<ValidationResult> TypeCheckerImpl::Check(
12751272
std::unique_ptr<Ast> ast) const {
1276-
auto& ast_impl = AstImpl::CastFromPublicAst(*ast);
12771273
google::protobuf::Arena type_arena;
12781274

12791275
std::vector<TypeCheckIssue> issues;
@@ -1282,13 +1278,13 @@ absl::StatusOr<ValidationResult> TypeCheckerImpl::Check(
12821278

12831279
TypeInferenceContext type_inference_context(
12841280
&type_arena, options_.enable_legacy_null_assignment);
1285-
ResolveVisitor visitor(env_.container(), std::move(generator), env_, ast_impl,
1281+
ResolveVisitor visitor(env_.container(), std::move(generator), env_, *ast,
12861282
type_inference_context, issues, &type_arena);
12871283

12881284
TraversalOptions opts;
12891285
opts.use_comprehension_callbacks = true;
12901286
bool error_limit_reached = false;
1291-
auto traversal = AstTraversal::Create(ast_impl.root_expr(), opts);
1287+
auto traversal = AstTraversal::Create(ast->root_expr(), opts);
12921288

12931289
for (int step = 0; step < options_.max_expression_node_count * 2; ++step) {
12941290
bool has_next = traversal.Step(visitor);
@@ -1315,7 +1311,7 @@ absl::StatusOr<ValidationResult> TypeCheckerImpl::Check(
13151311
{}, absl::StrCat("maximum number of ERROR issues exceeded: ",
13161312
options_.max_error_issues)));
13171313
} else if (env_.expected_type().has_value()) {
1318-
visitor.AssertExpectedType(ast_impl.root_expr(), *env_.expected_type());
1314+
visitor.AssertExpectedType(ast->root_expr(), *env_.expected_type());
13191315
}
13201316

13211317
// If any issues are errors, return without an AST.
@@ -1329,13 +1325,13 @@ absl::StatusOr<ValidationResult> TypeCheckerImpl::Check(
13291325
// Happens in a second pass to simplify validating that pointers haven't
13301326
// been invalidated by other updates.
13311327
ResolveRewriter rewriter(visitor, type_inference_context, options_,
1332-
ast_impl.mutable_reference_map(),
1333-
ast_impl.mutable_type_map());
1334-
AstRewrite(ast_impl.mutable_root_expr(), rewriter);
1328+
ast->mutable_reference_map(),
1329+
ast->mutable_type_map());
1330+
AstRewrite(ast->mutable_root_expr(), rewriter);
13351331

13361332
CEL_RETURN_IF_ERROR(rewriter.status());
13371333

1338-
ast_impl.set_is_checked(true);
1334+
ast->set_is_checked(true);
13391335

13401336
return ValidationResult(std::move(ast), std::move(issues));
13411337
}

0 commit comments

Comments
 (0)