Devirtualize ExpressionStep::id and ExpressionStep::ComesFromAst

eval/eval/evaluator_core.cc

@@ -138,7 +138,7 @@ absl::StatusOr<cel::Value> ExecutionFrame::Evaluate(
         return std::move(status).Consume();
       }
 
-      if (!expr->ComesFromAst()) {
+      if (!expr->comes_from_ast()) {
         continue;
       }
 

eval/eval/evaluator_core.h

@@ -53,6 +53,12 @@ using EvaluationListener = cel::TraceableProgram::EvaluationListener;
 // Class Expression represents single execution step.
 class ExpressionStep {
  public:
+  explicit ExpressionStep(int64_t id, bool comes_from_ast = true)
+      : id_(id), comes_from_ast_(comes_from_ast) {}
+
+  ExpressionStep(const ExpressionStep&) = delete;
+  ExpressionStep& operator=(const ExpressionStep&) = delete;
+
   virtual ~ExpressionStep() = default;
 
   // Performs actual evaluation.
@@ -70,15 +76,21 @@ class ExpressionStep {
   // expression associated (e.g. a jump step), or if there is no ID assigned to
   // the corresponding expression. Useful for error scenarios where information
   // from Expr object is needed to create CelError.
-  virtual int64_t id() const = 0;
+  int64_t id() const { return id_; }
 
   // Returns if the execution step comes from AST.
-  virtual bool ComesFromAst() const = 0;
+  bool comes_from_ast() const { return comes_from_ast_; }
 
   // Return the type of the underlying expression step for special handling in
   // the planning phase. This should only be overridden by special cases, and
   // callers must not make any assumptions about the default case.
-  virtual cel::NativeTypeId GetNativeTypeId() const = 0;
+  virtual cel::NativeTypeId GetNativeTypeId() const {
+    return cel::NativeTypeId();
+  }
+
+ private:
+  const int64_t id_;
+  const bool comes_from_ast_;
 };
 
 using ExecutionPath = std::vector<std::unique_ptr<const ExpressionStep>>;

eval/eval/evaluator_core_test.cc

@@ -31,24 +31,20 @@ using testing::Eq;
 // Pushes int64_t(0) on top of value stack.
 class FakeConstExpressionStep : public ExpressionStep {
  public:
+  FakeConstExpressionStep() : ExpressionStep(0, true) {}
+
   absl::Status Evaluate(ExecutionFrame* frame) const override {
     frame->value_stack().Push(CreateIntValue(0));
     return absl::OkStatus();
   }
-
-  int64_t id() const override { return 0; }
-
-  bool ComesFromAst() const override { return true; }
-
-  cel::NativeTypeId GetNativeTypeId() const override {
-    return cel::NativeTypeId();
-  }
 };
 
 // Fake expression implementation
 // Increments argument on top of the stack.
 class FakeIncrementExpressionStep : public ExpressionStep {
  public:
+  FakeIncrementExpressionStep() : ExpressionStep(0, true) {}
+
   absl::Status Evaluate(ExecutionFrame* frame) const override {
     auto value = frame->value_stack().Peek();
     frame->value_stack().Pop(1);
@@ -57,14 +53,6 @@ class FakeIncrementExpressionStep : public ExpressionStep {
     frame->value_stack().Push(CreateIntValue(val + 1));
     return absl::OkStatus();
   }
-
-  int64_t id() const override { return 0; }
-
-  bool ComesFromAst() const override { return true; }
-
-  cel::NativeTypeId GetNativeTypeId() const override {
-    return cel::NativeTypeId();
-  }
 };
 
 TEST(EvaluatorCoreTest, ExecutionFrameNext) {

eval/eval/expression_step_base.h

@@ -1,35 +1,11 @@
 #ifndef THIRD_PARTY_CEL_CPP_EVAL_EVAL_EXPRESSION_STEP_BASE_H_
 #define THIRD_PARTY_CEL_CPP_EVAL_EVAL_EXPRESSION_STEP_BASE_H_
 
-#include <cstdint>
-
 #include "eval/eval/evaluator_core.h"
 
 namespace google::api::expr::runtime {
 
-class ExpressionStepBase : public ExpressionStep {
- public:
-  explicit ExpressionStepBase(int64_t expr_id, bool comes_from_ast = true)
-      : id_(expr_id), comes_from_ast_(comes_from_ast) {}
-
-  // Non-copyable
-  ExpressionStepBase(const ExpressionStepBase&) = delete;
-  ExpressionStepBase& operator=(const ExpressionStepBase&) = delete;
-
-  // Returns corresponding expression object ID.
-  int64_t id() const override { return id_; }
-
-  // Returns if the execution step comes from AST.
-  bool ComesFromAst() const override { return comes_from_ast_; }
-
-  cel::NativeTypeId GetNativeTypeId() const override {
-    return cel::NativeTypeId();
-  }
-
- private:
-  int64_t id_;
-  bool comes_from_ast_;
-};
+using ExpressionStepBase = ExpressionStep;
 
 }  // namespace google::api::expr::runtime