google · copybara-service · Feb 14, 2025 · Feb 14, 2025
diff --git a/common/values/legacy_list_value.h b/common/values/legacy_list_value.h
@@ -93,9 +93,6 @@ class LegacyListValue final
       absl::Nonnull<google::protobuf::Arena*> arena, absl::Nonnull<Value*> result) const;
   using ListValueMixin::Equal;
 
-  absl::Status Contains(ValueManager& value_manager, const Value& other,
-                        Value& result) const;
-
   bool IsZeroValue() const { return IsEmpty(); }
 
   bool IsEmpty() const;

diff --git a/eval/compiler/BUILD b/eval/compiler/BUILD
@@ -617,6 +617,7 @@ cc_test(
         "//runtime:type_registry",
         "//runtime/internal:runtime_env",
         "//runtime/internal:runtime_env_testing",
+        "//runtime/internal:runtime_value_manager",
         "@com_google_absl//absl/base:nullability",
         "@com_google_absl//absl/container:flat_hash_map",
         "@com_google_absl//absl/status",

diff --git a/eval/compiler/instrumentation_test.cc b/eval/compiler/instrumentation_test.cc
@@ -37,6 +37,7 @@
 #include "runtime/function_registry.h"
 #include "runtime/internal/runtime_env.h"
 #include "runtime/internal/runtime_env_testing.h"
+#include "runtime/internal/runtime_value_manager.h"
 #include "runtime/managed_value_factory.h"
 #include "runtime/runtime_options.h"
 #include "runtime/standard_functions.h"
@@ -62,9 +63,8 @@ class InstrumentationTest : public ::testing::Test {
       : env_(NewTestingRuntimeEnv()),
         function_registry_(env_->function_registry),
         type_registry_(env_->type_registry),
-        managed_value_factory_(
-            type_registry_.GetComposedTypeProvider(),
-            cel::extensions::ProtoMemoryManagerRef(&arena_)) {}
+        value_manager_(&arena_, env_->descriptor_pool.get(),
+                       env_->MutableMessageFactory()) {}
   void SetUp() override {
     ASSERT_OK(cel::RegisterStandardFunctions(function_registry_, options_));
   }
@@ -75,7 +75,7 @@ class InstrumentationTest : public ::testing::Test {
   cel::FunctionRegistry& function_registry_;
   cel::TypeRegistry& type_registry_;
   google::protobuf::Arena arena_;
-  cel::ManagedValueFactory managed_value_factory_;
+  cel::runtime_internal::RuntimeValueManager value_manager_;
 };
 
 MATCHER_P(IsIntValue, expected, "") {
@@ -106,7 +106,7 @@ TEST_F(InstrumentationTest, Basic) {
                        builder.CreateExpressionImpl(std::move(ast),
                                                     /*issues=*/nullptr));
 
-  auto state = plan.MakeEvaluatorState(managed_value_factory_.get());
+  auto state = plan.MakeEvaluatorState(value_manager_);
   cel::Activation activation;
 
   ASSERT_OK_AND_ASSIGN(
@@ -152,7 +152,7 @@ TEST_F(InstrumentationTest, BasicWithConstFolding) {
                            Pair(2, IsIntValue(3)), Pair(5, IsIntValue(3))));
   expr_id_to_value.clear();
 
-  auto state = plan.MakeEvaluatorState(managed_value_factory_.get());
+  auto state = plan.MakeEvaluatorState(value_manager_);
   cel::Activation activation;
 
   ASSERT_OK_AND_ASSIGN(
@@ -190,22 +190,19 @@ TEST_F(InstrumentationTest, AndShortCircuit) {
                        builder.CreateExpressionImpl(std::move(ast),
                                                     /*issues=*/nullptr));
 
-  auto state = plan.MakeEvaluatorState(managed_value_factory_.get());
+  auto state = plan.MakeEvaluatorState(value_manager_);
   cel::Activation activation;
 
-  activation.InsertOrAssignValue(
-      "a", managed_value_factory_.get().CreateBoolValue(true));
-  activation.InsertOrAssignValue(
-      "b", managed_value_factory_.get().CreateBoolValue(false));
+  activation.InsertOrAssignValue("a", value_manager_.CreateBoolValue(true));
+  activation.InsertOrAssignValue("b", value_manager_.CreateBoolValue(false));
 
   ASSERT_OK_AND_ASSIGN(
       auto value,
       plan.EvaluateWithCallback(activation, EvaluationListener(), state));
 
   EXPECT_THAT(expr_ids, ElementsAre(1, 2, 3));
 
-  activation.InsertOrAssignValue(
-      "a", managed_value_factory_.get().CreateBoolValue(false));
+  activation.InsertOrAssignValue("a", value_manager_.CreateBoolValue(false));
 
   ASSERT_OK_AND_ASSIGN(value, plan.EvaluateWithCallback(
                                   activation, EvaluationListener(), state));
@@ -235,22 +232,19 @@ TEST_F(InstrumentationTest, OrShortCircuit) {
                        builder.CreateExpressionImpl(std::move(ast),
                                                     /*issues=*/nullptr));
 
-  auto state = plan.MakeEvaluatorState(managed_value_factory_.get());
+  auto state = plan.MakeEvaluatorState(value_manager_);
   cel::Activation activation;
 
-  activation.InsertOrAssignValue(
-      "a", managed_value_factory_.get().CreateBoolValue(false));
-  activation.InsertOrAssignValue(
-      "b", managed_value_factory_.get().CreateBoolValue(true));
+  activation.InsertOrAssignValue("a", value_manager_.CreateBoolValue(false));
+  activation.InsertOrAssignValue("b", value_manager_.CreateBoolValue(true));
 
   ASSERT_OK_AND_ASSIGN(
       auto value,
       plan.EvaluateWithCallback(activation, EvaluationListener(), state));
 
   EXPECT_THAT(expr_ids, ElementsAre(1, 2, 3));
   expr_ids.clear();
-  activation.InsertOrAssignValue(
-      "a", managed_value_factory_.get().CreateBoolValue(true));
+  activation.InsertOrAssignValue("a", value_manager_.CreateBoolValue(true));
 
   ASSERT_OK_AND_ASSIGN(value, plan.EvaluateWithCallback(
                                   activation, EvaluationListener(), state));
@@ -280,15 +274,12 @@ TEST_F(InstrumentationTest, Ternary) {
                        builder.CreateExpressionImpl(std::move(ast),
                                                     /*issues=*/nullptr));
 
-  auto state = plan.MakeEvaluatorState(managed_value_factory_.get());
+  auto state = plan.MakeEvaluatorState(value_manager_);
   cel::Activation activation;
 
-  activation.InsertOrAssignValue(
-      "c", managed_value_factory_.get().CreateBoolValue(true));
-  activation.InsertOrAssignValue(
-      "a", managed_value_factory_.get().CreateIntValue(1));
-  activation.InsertOrAssignValue(
-      "b", managed_value_factory_.get().CreateIntValue(2));
+  activation.InsertOrAssignValue("c", value_manager_.CreateBoolValue(true));
+  activation.InsertOrAssignValue("a", value_manager_.CreateIntValue(1));
+  activation.InsertOrAssignValue("b", value_manager_.CreateIntValue(2));
 
   ASSERT_OK_AND_ASSIGN(
       auto value,
@@ -301,8 +292,7 @@ TEST_F(InstrumentationTest, Ternary) {
   EXPECT_THAT(expr_ids, ElementsAre(1, 3, 2));
   expr_ids.clear();
 
-  activation.InsertOrAssignValue(
-      "c", managed_value_factory_.get().CreateBoolValue(false));
+  activation.InsertOrAssignValue("c", value_manager_.CreateBoolValue(false));
 
   ASSERT_OK_AND_ASSIGN(value, plan.EvaluateWithCallback(
                                   activation, EvaluationListener(), state));
@@ -336,7 +326,7 @@ TEST_F(InstrumentationTest, OptimizedStepsNotEvaluated) {
                        builder.CreateExpressionImpl(std::move(ast),
                                                     /*issues=*/nullptr));
 
-  auto state = plan.MakeEvaluatorState(managed_value_factory_.get());
+  auto state = plan.MakeEvaluatorState(value_manager_);
   cel::Activation activation;
 
   ASSERT_OK_AND_ASSIGN(
@@ -362,15 +352,12 @@ TEST_F(InstrumentationTest, NoopSkipped) {
                        builder.CreateExpressionImpl(std::move(ast),
                                                     /*issues=*/nullptr));
 
-  auto state = plan.MakeEvaluatorState(managed_value_factory_.get());
+  auto state = plan.MakeEvaluatorState(value_manager_);
   cel::Activation activation;
 
-  activation.InsertOrAssignValue(
-      "c", managed_value_factory_.get().CreateBoolValue(true));
-  activation.InsertOrAssignValue(
-      "a", managed_value_factory_.get().CreateIntValue(1));
-  activation.InsertOrAssignValue(
-      "b", managed_value_factory_.get().CreateIntValue(2));
+  activation.InsertOrAssignValue("c", value_manager_.CreateBoolValue(true));
+  activation.InsertOrAssignValue("a", value_manager_.CreateIntValue(1));
+  activation.InsertOrAssignValue("b", value_manager_.CreateIntValue(2));
 
   ASSERT_OK_AND_ASSIGN(
       auto value,

diff --git a/eval/eval/BUILD b/eval/eval/BUILD
@@ -713,11 +713,14 @@ cc_test(
         "//eval/public:activation",
         "//eval/public:cel_attribute",
         "//internal:testing",
+        "//internal:testing_descriptor_pool",
+        "//internal:testing_message_factory",
         "//runtime:activation",
-        "//runtime:managed_value_factory",
         "//runtime:runtime_options",
         "//runtime/internal:runtime_env_testing",
+        "//runtime/internal:runtime_value_manager",
         "@com_google_absl//absl/status",
+        "@com_google_protobuf//:protobuf",
     ],
 )
 

diff --git a/eval/eval/ident_step.cc b/eval/eval/ident_step.cc
@@ -58,8 +58,10 @@ absl::Status LookupIdent(const std::string& name, ExecutionFrameBase& frame,
     }
   }
 
-  CEL_ASSIGN_OR_RETURN(auto found, frame.activation().FindVariable(
-                                       frame.value_manager(), name, result));
+  CEL_ASSIGN_OR_RETURN(
+      auto found, frame.activation().FindVariable(name, frame.descriptor_pool(),
+                                                  frame.message_factory(),
+                                                  frame.arena(), &result));
 
   if (found) {
     return absl::OkStatus();

diff --git a/eval/eval/ident_step_test.cc b/eval/eval/ident_step_test.cc
@@ -16,10 +16,13 @@
 #include "eval/public/activation.h"
 #include "eval/public/cel_attribute.h"
 #include "internal/testing.h"
+#include "internal/testing_descriptor_pool.h"
+#include "internal/testing_message_factory.h"
 #include "runtime/activation.h"
 #include "runtime/internal/runtime_env_testing.h"
-#include "runtime/managed_value_factory.h"
+#include "runtime/internal/runtime_value_manager.h"
 #include "runtime/runtime_options.h"
+#include "google/protobuf/arena.h"
 
 namespace google::api::expr::runtime {
 
@@ -30,7 +33,6 @@ using ::cel::Cast;
 using ::cel::ErrorValue;
 using ::cel::InstanceOf;
 using ::cel::IntValue;
-using ::cel::ManagedValueFactory;
 using ::cel::MemoryManagerRef;
 using ::cel::RuntimeOptions;
 using ::cel::TypeProvider;
@@ -233,14 +235,16 @@ TEST(IdentStepTest, TestIdentStepUnknownAttribute) {
 }
 
 TEST(DirectIdentStepTest, Basic) {
-  ManagedValueFactory value_factory(TypeProvider::Builtin(),
-                                    MemoryManagerRef::ReferenceCounting());
+  google::protobuf::Arena arena;
+  cel::runtime_internal::RuntimeValueManager value_factory(
+      &arena, cel::internal::GetTestingDescriptorPool(),
+      cel::internal::GetTestingMessageFactory());
   cel::Activation activation;
   RuntimeOptions options;
 
   activation.InsertOrAssignValue("var1", IntValue(42));
 
-  ExecutionFrameBase frame(activation, options, value_factory.get());
+  ExecutionFrameBase frame(activation, options, value_factory);
   Value result;
   AttributeTrail trail;
 
@@ -253,16 +257,18 @@ TEST(DirectIdentStepTest, Basic) {
 }
 
 TEST(DirectIdentStepTest, UnknownAttribute) {
-  ManagedValueFactory value_factory(TypeProvider::Builtin(),
-                                    MemoryManagerRef::ReferenceCounting());
+  google::protobuf::Arena arena;
+  cel::runtime_internal::RuntimeValueManager value_factory(
+      &arena, cel::internal::GetTestingDescriptorPool(),
+      cel::internal::GetTestingMessageFactory());
   cel::Activation activation;
   RuntimeOptions options;
   options.unknown_processing = cel::UnknownProcessingOptions::kAttributeOnly;
 
   activation.InsertOrAssignValue("var1", IntValue(42));
   activation.SetUnknownPatterns({CreateCelAttributePattern("var1", {})});
 
-  ExecutionFrameBase frame(activation, options, value_factory.get());
+  ExecutionFrameBase frame(activation, options, value_factory);
   Value result;
   AttributeTrail trail;
 
@@ -275,16 +281,18 @@ TEST(DirectIdentStepTest, UnknownAttribute) {
 }
 
 TEST(DirectIdentStepTest, MissingAttribute) {
-  ManagedValueFactory value_factory(TypeProvider::Builtin(),
-                                    MemoryManagerRef::ReferenceCounting());
+  google::protobuf::Arena arena;
+  cel::runtime_internal::RuntimeValueManager value_factory(
+      &arena, cel::internal::GetTestingDescriptorPool(),
+      cel::internal::GetTestingMessageFactory());
   cel::Activation activation;
   RuntimeOptions options;
   options.enable_missing_attribute_errors = true;
 
   activation.InsertOrAssignValue("var1", IntValue(42));
   activation.SetMissingPatterns({CreateCelAttributePattern("var1", {})});
 
-  ExecutionFrameBase frame(activation, options, value_factory.get());
+  ExecutionFrameBase frame(activation, options, value_factory);
   Value result;
   AttributeTrail trail;
 
@@ -298,12 +306,14 @@ TEST(DirectIdentStepTest, MissingAttribute) {
 }
 
 TEST(DirectIdentStepTest, NotFound) {
-  ManagedValueFactory value_factory(TypeProvider::Builtin(),
-                                    MemoryManagerRef::ReferenceCounting());
+  google::protobuf::Arena arena;
+  cel::runtime_internal::RuntimeValueManager value_factory(
+      &arena, cel::internal::GetTestingDescriptorPool(),
+      cel::internal::GetTestingMessageFactory());
   cel::Activation activation;
   RuntimeOptions options;
 
-  ExecutionFrameBase frame(activation, options, value_factory.get());
+  ExecutionFrameBase frame(activation, options, value_factory);
   Value result;
   AttributeTrail trail;
 

diff --git a/eval/eval/shadowable_value_step.cc b/eval/eval/shadowable_value_step.cc
@@ -39,7 +39,8 @@ absl::Status ShadowableValueStep::Evaluate(ExecutionFrame* frame) const {
   cel::Value result;
   CEL_ASSIGN_OR_RETURN(auto found,
                        frame->modern_activation().FindVariable(
-                           frame->value_factory(), identifier_, result));
+                           identifier_, frame->descriptor_pool(),
+                           frame->message_factory(), frame->arena(), &result));
   if (found) {
     frame->value_stack().Push(std::move(result));
   } else {
@@ -70,8 +71,9 @@ class DirectShadowableValueStep : public DirectExpressionStep {
 absl::Status DirectShadowableValueStep::Evaluate(
     ExecutionFrameBase& frame, Value& result, AttributeTrail& attribute) const {
   CEL_ASSIGN_OR_RETURN(auto found,
-                       frame.activation().FindVariable(frame.value_manager(),
-                                                       identifier_, result));
+                       frame.activation().FindVariable(
+                           identifier_, frame.descriptor_pool(),
+                           frame.message_factory(), frame.arena(), &result));
   if (!found) {
     result = value_;
   }

diff --git a/eval/internal/BUILD b/eval/internal/BUILD
@@ -82,14 +82,13 @@ cc_library(
     deps = [
         ":interop",
         "//base:attributes",
-        "//common:memory",
         "//common:value",
         "//eval/public:base_activation",
         "//eval/public:cel_value",
-        "//extensions/protobuf:memory_manager",
         "//internal:status_macros",
         "//runtime:activation_interface",
         "//runtime:function_overload_reference",
+        "@com_google_absl//absl/base:nullability",
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/types:optional",

diff --git a/eval/internal/adapter_activation_impl.cc b/eval/internal/adapter_activation_impl.cc
@@ -16,33 +16,37 @@
 
 #include <vector>
 
+#include "absl/base/nullability.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
 #include "absl/types/optional.h"
+#include "common/value.h"
 #include "eval/internal/interop.h"
 #include "eval/public/cel_value.h"
-#include "extensions/protobuf/memory_manager.h"
 #include "internal/status_macros.h"
 #include "runtime/function_overload_reference.h"
 #include "google/protobuf/arena.h"
+#include "google/protobuf/descriptor.h"
+#include "google/protobuf/message.h"
 
 namespace cel::interop_internal {
 
 using ::google::api::expr::runtime::CelFunction;
 
 absl::StatusOr<bool> AdapterActivationImpl::FindVariable(
-    ValueManager& value_factory, absl::string_view name, Value& result) const {
+    absl::string_view name,
+    absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool,
+    absl::Nonnull<google::protobuf::MessageFactory*> message_factory,
+    absl::Nonnull<google::protobuf::Arena*> arena, absl::Nonnull<Value*> result) const {
   // This implementation should only be used during interop, when we can
   // always assume the memory manager is backed by a protobuf arena.
-  google::protobuf::Arena* arena =
-      extensions::ProtoMemoryManagerArena(value_factory.GetMemoryManager());
 
   absl::optional<google::api::expr::runtime::CelValue> legacy_value =
       legacy_activation_.FindValue(name, arena);
   if (!legacy_value.has_value()) {
     return false;
   }
-  CEL_RETURN_IF_ERROR(ModernValue(arena, *legacy_value, result));
+  CEL_RETURN_IF_ERROR(ModernValue(arena, *legacy_value, *result));
   return true;
 }