Migrate to a native AST and Expr representation
Migrate to a native AST and Expr representation.
The original cel-go implementation was heavily based on protobuf definitions
for types, declarations, and expressions. While this was expedient, it has also
been cumbersome. With PR #568, the types and declarations were replaced
by Go-native structs. This issue tracks the migration of the Expr, SourceInfo,
and AST objects.
The motives for making this change are many:
- Performance
- Remove (most) hard dependencies on protobuf
- Allow for better interop between serialization formats and type systems
As part of the optimizer work, it's clear that the split between cel.Ast and ast.AST is confusing. The two structures exist separately to ease migration, but should be merged before this issue can be considered closed.
Apologies if this is not the right place to ask this. Is the plan to unify the two AST data types into one in a future release?
I am a bit confused by how we are supposed to use the new native types right now. Most of the methods to work with parsed expressions take arguments of type ast.Expr. However, the return type of Compile and Check methods of cel.Env is cel.Ast which doesn't have a method to obtain the underlying ast.Expr object. I am currently forced to do something along the lines of the following:
celAST, _ := env.Compile(exprString)
checkedExpr, _ := cel.AstToCheckedExpr(celAST)
astAST, _ := ast.ToAST(checkedExpr)
expr := astAST.Expr() // finally have an `ast.Expr` that I can use with `ast.PreOrderVisit` etc.
Is there a better way to do this with the current version of CEL (0.18.1) or should I wait for the AST types to be unified?
@TristonianJones apologies for pinging you directly. I am stuck on the above issue of not being able to access the underlying ast.Expr of the cel.Ast without jumping through a lot of hoops. Would you be open to a PR that introduces two functions to obtain the Expr and SourceInfo of an Ast object like the following or adding them as methods to cel.Ast?
diff --git a/cel/io.go b/cel/io.go
index 3133fb9..50bb878 100644
--- a/cel/io.go
+++ b/cel/io.go
@@ -101,22 +101,36 @@ func AstToString(a *Ast) (string, error) {
return parser.Unparse(a.impl.Expr(), a.impl.SourceInfo())
}
+// AstToExpr returns the underlying expr of this Ast
+func AstToExpr(a *Ast) ast.Expr {
+ return a.impl.Expr()
+}
+
+// AstToSourceInfo returns the underlying source information of this Ast
+func AstToSourceInfo(a *Ast) *ast.SourceInfo {
+ return a.impl.SourceInfo()
+}
@charithe I added a NativeRep() call on the cel.Ast to convert to an ast.AST as part of #853 ; hopefully, this helps.
@charithe I added a
NativeRep()call on thecel.Astto convert to anast.ASTas part of #853 ; hopefully, this helps.
@TristonianJones Thank you so much! A little more performant for straight iterating than creating a new optimizer. Appreciate you and all of the work on cel-go!