From 3db277e36ccb5a317787dd0c9f595e64fa8eb2f2 Mon Sep 17 00:00:00 2001 From: Jonathan Tatum Date: Fri, 22 Aug 2025 17:18:08 -0700 Subject: [PATCH] Make AST fields visible on public AST interface. PiperOrigin-RevId: 798397862 --- common/BUILD | 6 ++ common/{ast/ast_impl.cc => ast.cc} | 21 +++-- common/ast.h | 134 +++++++++++++++++++++++++---- common/ast/BUILD | 4 - common/ast/ast_impl.h | 109 ++--------------------- common/ast/metadata.h | 2 +- 6 files changed, 144 insertions(+), 132 deletions(-) rename common/{ast/ast_impl.cc => ast.cc} (69%) diff --git a/common/BUILD b/common/BUILD index ff36da33f..b4b727086 100644 --- a/common/BUILD +++ b/common/BUILD @@ -21,9 +21,15 @@ licenses(["notice"]) cc_library( name = "ast", + srcs = ["ast.cc"], hdrs = ["ast.h"], deps = [ ":expr", + "//common/ast:metadata", + "@com_google_absl//absl/base:no_destructor", + "@com_google_absl//absl/base:nullability", + "@com_google_absl//absl/container:flat_hash_map", + "@com_google_absl//absl/strings:string_view", ], ) diff --git a/common/ast/ast_impl.cc b/common/ast.cc similarity index 69% rename from common/ast/ast_impl.cc rename to common/ast.cc index 9177f53d3..aea153197 100644 --- a/common/ast/ast_impl.cc +++ b/common/ast.cc @@ -12,26 +12,25 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "common/ast/ast_impl.h" +#include "common/ast.h" #include +#include "absl/base/no_destructor.h" #include "absl/base/nullability.h" -#include "absl/container/flat_hash_map.h" -#include "common/ast/expr.h" #include "common/ast/metadata.h" -namespace cel::ast_internal { +namespace cel { namespace { -const Type& DynSingleton() { - static auto* singleton = new TypeSpec(TypeKind(DynamicType())); +const TypeSpec& DynSingleton() { + static absl::NoDestructor singleton{TypeSpecKind(DynTypeSpec())}; return *singleton; } } // namespace -const TypeSpec* absl_nullable AstImpl::GetType(int64_t expr_id) const { +const TypeSpec* absl_nullable Ast::GetType(int64_t expr_id) const { auto iter = type_map_.find(expr_id); if (iter == type_map_.end()) { return nullptr; @@ -39,18 +38,18 @@ const TypeSpec* absl_nullable AstImpl::GetType(int64_t expr_id) const { return &iter->second; } -const TypeSpec& AstImpl::GetTypeOrDyn(int64_t expr_id) const { +const TypeSpec& Ast::GetTypeOrDyn(int64_t expr_id) const { if (const TypeSpec* type = GetType(expr_id); type != nullptr) { return *type; } return DynSingleton(); } -const TypeSpec& AstImpl::GetReturnType() const { +const TypeSpec& Ast::GetReturnType() const { return GetTypeOrDyn(root_expr().id()); } -const Reference* AstImpl::GetReference(int64_t expr_id) const { +const Reference* absl_nullable Ast::GetReference(int64_t expr_id) const { auto iter = reference_map_.find(expr_id); if (iter == reference_map_.end()) { return nullptr; @@ -58,4 +57,4 @@ const Reference* AstImpl::GetReference(int64_t expr_id) const { return &iter->second; } -} // namespace cel::ast_internal +} // namespace cel diff --git a/common/ast.h b/common/ast.h index 9d3d2a234..81a477ec4 100644 --- a/common/ast.h +++ b/common/ast.h @@ -15,38 +15,142 @@ #ifndef THIRD_PARTY_CEL_CPP_COMMON_AST_H_ #define THIRD_PARTY_CEL_CPP_COMMON_AST_H_ +#include +#include +#include + +#include "absl/base/nullability.h" +#include "absl/container/flat_hash_map.h" +#include "absl/strings/string_view.h" +#include "common/ast/metadata.h" // IWYU pragma: export #include "common/expr.h" namespace cel { namespace ast_internal { -// Forward declare supported implementations. class AstImpl; } // namespace ast_internal -// Runtime representation of a CEL expression's Abstract Syntax Tree. +// In memory representation of a CEL abstract syntax tree. +// +// If AST inspection or manipulation is needed, prefer to use an existing tool +// or traverse the protobuf representation rather than directly manipulating +// through this class. See `cel::NavigableAst` and `cel::AstTraverse`. // -// This class provides public APIs for CEL users and allows for clients to -// manage lifecycle. +// Type and reference maps are only populated if the AST is checked. Any changes +// to the AST are not automatically reflected in the type or reference maps. // -// Implementations are intentionally opaque to prevent dependencies on the -// details of the runtime representation. To create a new instance, from a -// protobuf representation, use the conversion utilities in -// `extensions/protobuf/ast_converters.h`. +// To create a new instance from a protobuf representation, use the conversion +// utilities in `common/ast_proto.h`. class Ast { public: + using ReferenceMap = absl::flat_hash_map; + using TypeMap = absl::flat_hash_map; + virtual ~Ast() = default; - // Whether the AST includes type check information. - // If false, the runtime assumes all types are dyn, and that qualified names - // have not been resolved. - virtual bool IsChecked() const = 0; + Ast() : is_checked_(false) {} + + Ast(Expr expr, SourceInfo source_info) + : root_expr_(std::move(expr)), + source_info_(std::move(source_info)), + is_checked_(false) {} + + Ast(Expr expr, SourceInfo source_info, ReferenceMap reference_map, + TypeMap type_map, std::string expr_version) + : root_expr_(std::move(expr)), + source_info_(std::move(source_info)), + reference_map_(std::move(reference_map)), + type_map_(std::move(type_map)), + expr_version_(std::move(expr_version)), + is_checked_(true) {} + + // Move-only + Ast(const Ast& other) = delete; + Ast& operator=(const Ast& other) = delete; + Ast(Ast&& other) = default; + Ast& operator=(Ast&& other) = default; + + // Deprecated. Use `is_checked()` instead. + bool IsChecked() const { return is_checked_; } + + bool is_checked() const { return is_checked_; } + void set_is_checked(bool is_checked) { is_checked_ = is_checked; } + + // The root expression of the AST. + // + // This is the entry point for evaluation and determines the overall result + // of the expression given a context. + const Expr& root_expr() const { return root_expr_; } + Expr& mutable_root_expr() { return root_expr_; } + + // Metadata about the source expression. + const SourceInfo& source_info() const { return source_info_; } + SourceInfo& mutable_source_info() { return source_info_; } + + // Returns the type of the expression with the given `expr_id`. + // + // Returns `nullptr` if the expression node is not found or has dynamic type. + const TypeSpec* absl_nullable GetType(int64_t expr_id) const; + const TypeSpec& GetTypeOrDyn(int64_t expr_id) const; + const TypeSpec& GetReturnType() const; + + // Returns the resolved reference for the expression with the given `expr_id`. + // + // Returns `nullptr` if the expression node is not found or no reference was + // resolved. + const Reference* absl_nullable GetReference(int64_t expr_id) const; + + // A map from expression ids to resolved references. + // + // The following entries are in this table: + // + // - An Ident or Select expression is represented here if it resolves to a + // declaration. For instance, if `a.b.c` is represented by + // `select(select(id(a), b), c)`, and `a.b` resolves to a declaration, + // while `c` is a field selection, then the reference is attached to the + // nested select expression (but not to the id or or the outer select). + // In turn, if `a` resolves to a declaration and `b.c` are field selections, + // the reference is attached to the ident expression. + // - Every Call expression has an entry here, identifying the function being + // called. + // - Every CreateStruct expression for a message has an entry, identifying + // the message. + // + // Unpopulated if the AST is not checked. + const ReferenceMap& reference_map() const { return reference_map_; } + ReferenceMap& mutable_reference_map() { return reference_map_; } + + // A map from expression ids to types. + // + // Every expression node which has a type different than DYN has a mapping + // here. If an expression has type DYN, it is omitted from this map to save + // space. + // + // Unpopulated if the AST is not checked. + const TypeMap& type_map() const { return type_map_; } + TypeMap& mutable_type_map() { return type_map_; } + + // The expr version indicates the major / minor version number of the `expr` + // representation. + // + // The most common reason for a version change will be to indicate to the CEL + // runtimes that transformations have been performed on the expr during static + // analysis. + absl::string_view expr_version() const { return expr_version_; } + void set_expr_version(absl::string_view expr_version) { + expr_version_ = expr_version; + } private: - // This interface should only be implemented by friend-visibility allowed - // subclasses. - Ast() = default; friend class ast_internal::AstImpl; + + Expr root_expr_; + SourceInfo source_info_; + ReferenceMap reference_map_; + TypeMap type_map_; + std::string expr_version_; + bool is_checked_; }; } // namespace cel diff --git a/common/ast/BUILD b/common/ast/BUILD index 19d00dde6..a71d7ba5a 100644 --- a/common/ast/BUILD +++ b/common/ast/BUILD @@ -75,7 +75,6 @@ cc_test( cc_library( name = "ast_impl", - srcs = ["ast_impl.cc"], hdrs = ["ast_impl.h"], deps = [ ":expr", @@ -83,9 +82,6 @@ cc_library( "//common:ast", "//common:expr", "//internal:casts", - "@com_google_absl//absl/base:nullability", - "@com_google_absl//absl/container:flat_hash_map", - "@com_google_absl//absl/strings:string_view", ], ) diff --git a/common/ast/ast_impl.h b/common/ast/ast_impl.h index 3c5dc7bae..9f424bc7e 100644 --- a/common/ast/ast_impl.h +++ b/common/ast/ast_impl.h @@ -15,12 +15,9 @@ #ifndef THIRD_PARTY_CEL_CPP_BASE_AST_INTERNAL_AST_IMPL_H_ #define THIRD_PARTY_CEL_CPP_BASE_AST_INTERNAL_AST_IMPL_H_ -#include #include #include -#include "absl/container/flat_hash_map.h" -#include "absl/strings/string_view.h" #include "common/ast.h" #include "common/ast/expr.h" #include "common/ast/metadata.h" // IWYU pragma: export @@ -29,18 +26,13 @@ namespace cel::ast_internal { -// In memory representation of a CEL abstract syntax tree. +// Trivial subclass of the public Ast. // -// If AST inspection or manipulation is needed, prefer to use an existing tool -// or traverse the protobuf representation rather than directly manipulating -// through this class. See `cel::NavigableAst` and `cel::AstTraverse`. -// -// Type and reference maps are only populated if the AST is checked. Any changes -// to the AST are not automatically reflected in the type or reference maps. +// Temporarily needed to transition to just using the public Ast. class AstImpl : public Ast { public: - using ReferenceMap = absl::flat_hash_map; - using TypeMap = absl::flat_hash_map; + using ReferenceMap = Ast::ReferenceMap; + using TypeMap = Ast::TypeMap; // Overloads for down casting from the public interface to the internal // implementation. @@ -60,106 +52,21 @@ class AstImpl : public Ast { return cel::internal::down_cast(ast); } - AstImpl() : is_checked_(false) {} + AstImpl() = default; AstImpl(Expr expr, SourceInfo source_info) - : root_expr_(std::move(expr)), - source_info_(std::move(source_info)), - is_checked_(false) {} + : Ast(std::move(expr), std::move(source_info)) {} AstImpl(Expr expr, SourceInfo source_info, ReferenceMap reference_map, TypeMap type_map, std::string expr_version) - : root_expr_(std::move(expr)), - source_info_(std::move(source_info)), - reference_map_(std::move(reference_map)), - type_map_(std::move(type_map)), - expr_version_(std::move(expr_version)), - is_checked_(true) {} + : Ast(std::move(expr), std::move(source_info), std::move(reference_map), + std::move(type_map), std::move(expr_version)) {} // Move-only AstImpl(const AstImpl& other) = delete; AstImpl& operator=(const AstImpl& other) = delete; AstImpl(AstImpl&& other) = default; AstImpl& operator=(AstImpl&& other) = default; - - // Implement public Ast APIs. - bool IsChecked() const override { return is_checked_; } - - bool is_checked() const { return is_checked_; } - void set_is_checked(bool is_checked) { is_checked_ = is_checked; } - - // The root expression of the AST. - // - // This is the entry point for evaluation and determines the overall result - // of the expression given a context. - const Expr& root_expr() const { return root_expr_; } - Expr& mutable_root_expr() { return root_expr_; } - - // Metadata about the source expression. - const SourceInfo& source_info() const { return source_info_; } - SourceInfo& mutable_source_info() { return source_info_; } - - // Returns the type of the expression with the given `expr_id`. - // - // Returns `nullptr` if the expression node is not found or has dynamic type. - const TypeSpec* absl_nullable GetType(int64_t expr_id) const; - const TypeSpec& GetTypeOrDyn(int64_t expr_id) const; - const TypeSpec& GetReturnType() const; - - // Returns the resolved reference for the expression with the given `expr_id`. - // - // Returns `nullptr` if the expression node is not found or no reference was - // resolved. - const Reference* absl_nullable GetReference(int64_t expr_id) const; - - // A map from expression ids to resolved references. - // - // The following entries are in this table: - // - // - An Ident or Select expression is represented here if it resolves to a - // declaration. For instance, if `a.b.c` is represented by - // `select(select(id(a), b), c)`, and `a.b` resolves to a declaration, - // while `c` is a field selection, then the reference is attached to the - // nested select expression (but not to the id or or the outer select). - // In turn, if `a` resolves to a declaration and `b.c` are field selections, - // the reference is attached to the ident expression. - // - Every Call expression has an entry here, identifying the function being - // called. - // - Every CreateStruct expression for a message has an entry, identifying - // the message. - // - // Unpopulated if the AST is not checked. - const ReferenceMap& reference_map() const { return reference_map_; } - ReferenceMap& mutable_reference_map() { return reference_map_; } - - // A map from expression ids to types. - // - // Every expression node which has a type different than DYN has a mapping - // here. If an expression has type DYN, it is omitted from this map to save - // space. - // - // Unpopulated if the AST is not checked. - const TypeMap& type_map() const { return type_map_; } - TypeMap& mutable_type_map() { return type_map_; } - - // The expr version indicates the major / minor version number of the `expr` - // representation. - // - // The most common reason for a version change will be to indicate to the CEL - // runtimes that transformations have been performed on the expr during static - // analysis. - absl::string_view expr_version() const { return expr_version_; } - void set_expr_version(absl::string_view expr_version) { - expr_version_ = expr_version; - } - - private: - Expr root_expr_; - SourceInfo source_info_; - ReferenceMap reference_map_; - TypeMap type_map_; - std::string expr_version_; - bool is_checked_; }; } // namespace cel::ast_internal diff --git a/common/ast/metadata.h b/common/ast/metadata.h index 707240b9a..00d70432c 100644 --- a/common/ast/metadata.h +++ b/common/ast/metadata.h @@ -16,7 +16,7 @@ // // These are more direct equivalents to the public protobuf definitions. // -// IWYU pragma: private, include "common/ast/ast_impl.h" +// IWYU pragma: private, include "common/ast.h" #ifndef THIRD_PARTY_CEL_CPP_COMMON_AST_METADATA_H_ #define THIRD_PARTY_CEL_CPP_COMMON_AST_METADATA_H_