Remove dependency on getting list/map value builders from factory/managers

PiperOrigin-RevId: 689544397

Author: jcking

common/values/list_value_builder.h

@@ -96,6 +96,8 @@ const MutableListValue& GetMutableListValue(
 const MutableListValue& GetMutableListValue(
     const ListValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND);
 
+absl::Nonnull<cel::ListValueBuilderPtr> NewListValueBuilder(
+    Allocator<> allocator);
 absl::Nonnull<cel::ListValueBuilderPtr> NewListValueBuilder(
     ValueFactory& value_factory);
 

common/values/map_value_builder.h

@@ -96,6 +96,8 @@ const MutableMapValue& GetMutableMapValue(
 const MutableMapValue& GetMutableMapValue(
     const MapValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND);
 
+absl::Nonnull<cel::MapValueBuilderPtr> NewMapValueBuilder(
+    Allocator<> allocator);
 absl::Nonnull<cel::MapValueBuilderPtr> NewMapValueBuilder(
     ValueFactory& value_factory);
 

common/values/value_builder.cc

@@ -502,11 +502,8 @@ class NonTrivialMutableListValueImpl final : public MutableListValue {
 
 class TrivialListValueBuilderImpl final : public ListValueBuilder {
  public:
-  TrivialListValueBuilderImpl(ValueFactory& value_factory,
-                              absl::Nonnull<google::protobuf::Arena*> arena)
-      : value_factory_(value_factory), elements_(arena) {
-    ABSL_DCHECK_EQ(value_factory_.GetMemoryManager().arena(), arena);
-  }
+  explicit TrivialListValueBuilderImpl(absl::Nonnull<google::protobuf::Arena*> arena)
+      : arena_(arena), elements_(arena_) {}
 
   absl::Status Add(Value value) override {
     CEL_RETURN_IF_ERROR(CheckListElement(value));
@@ -524,19 +521,18 @@ class TrivialListValueBuilderImpl final : public ListValueBuilder {
       return ListValue();
     }
     return ParsedListValue(
-        value_factory_.GetMemoryManager().MakeShared<TrivialListValueImpl>(
+        MemoryManager::Pooling(arena_).MakeShared<TrivialListValueImpl>(
             std::move(elements_)));
   }
 
  private:
-  ValueFactory& value_factory_;
+  absl::Nonnull<google::protobuf::Arena*> const arena_;
   TrivialValueVector elements_;
 };
 
 class NonTrivialListValueBuilderImpl final : public ListValueBuilder {
  public:
-  explicit NonTrivialListValueBuilderImpl(ValueFactory& value_factory)
-      : value_factory_(value_factory) {}
+  NonTrivialListValueBuilderImpl() = default;
 
   absl::Status Add(Value value) override {
     CEL_RETURN_IF_ERROR(CheckListElement(value));
@@ -553,12 +549,11 @@ class NonTrivialListValueBuilderImpl final : public ListValueBuilder {
       return ListValue();
     }
     return ParsedListValue(
-        value_factory_.GetMemoryManager().MakeShared<NonTrivialListValueImpl>(
+        MemoryManager::ReferenceCounting().MakeShared<NonTrivialListValueImpl>(
             std::move(elements_)));
   }
 
  private:
-  ValueFactory& value_factory_;
   NonTrivialValueVector elements_;
 };
 
@@ -676,13 +671,17 @@ const MutableListValue& GetMutableListValue(const ListValue& value) {
 }
 
 absl::Nonnull<cel::ListValueBuilderPtr> NewListValueBuilder(
-    ValueFactory& value_factory) {
-  if (absl::Nullable<google::protobuf::Arena*> arena =
-          value_factory.GetMemoryManager().arena();
+    Allocator<> allocator) {
+  if (absl::Nullable<google::protobuf::Arena*> arena = allocator.arena();
       arena != nullptr) {
-    return std::make_unique<TrivialListValueBuilderImpl>(value_factory, arena);
+    return std::make_unique<TrivialListValueBuilderImpl>(arena);
   }
-  return std::make_unique<NonTrivialListValueBuilderImpl>(value_factory);
+  return std::make_unique<NonTrivialListValueBuilderImpl>();
+}
+
+absl::Nonnull<cel::ListValueBuilderPtr> NewListValueBuilder(
+    ValueFactory& value_factory) {
+  return NewListValueBuilder(value_factory.GetMemoryManager());
 }
 
 }  // namespace common_internal
@@ -1451,11 +1450,8 @@ class NonTrivialMutableMapValueImpl final : public MutableMapValue {
 
 class TrivialMapValueBuilderImpl final : public MapValueBuilder {
  public:
-  TrivialMapValueBuilderImpl(ValueFactory& value_factory,
-                             absl::Nonnull<google::protobuf::Arena*> arena)
-      : value_factory_(value_factory), map_(arena) {
-    ABSL_DCHECK_EQ(value_factory_.GetMemoryManager().arena(), arena);
-  }
+  explicit TrivialMapValueBuilderImpl(absl::Nonnull<google::protobuf::Arena*> arena)
+      : arena_(arena), map_(arena_) {}
 
   absl::Status Put(Value key, Value value) override {
     CEL_RETURN_IF_ERROR(CheckMapKey(key));
@@ -1480,20 +1476,18 @@ class TrivialMapValueBuilderImpl final : public MapValueBuilder {
       return MapValue();
     }
     return ParsedMapValue(
-        value_factory_.GetMemoryManager().MakeShared<TrivialMapValueImpl>(
+        MemoryManager::Pooling(arena_).MakeShared<TrivialMapValueImpl>(
             std::move(map_)));
   }
 
  private:
-  ValueFactory& value_factory_;
+  absl::Nonnull<google::protobuf::Arena*> const arena_;
   TrivialValueFlatHashMap map_;
 };
 
 class NonTrivialMapValueBuilderImpl final : public MapValueBuilder {
  public:
-  explicit NonTrivialMapValueBuilderImpl(ValueFactory& value_factory)
-      : value_factory_(value_factory),
-        map_(NonTrivialValueFlatHashMapAllocator{}) {}
+  NonTrivialMapValueBuilderImpl() = default;
 
   absl::Status Put(Value key, Value value) override {
     CEL_RETURN_IF_ERROR(CheckMapKey(key));
@@ -1517,12 +1511,11 @@ class NonTrivialMapValueBuilderImpl final : public MapValueBuilder {
       return MapValue();
     }
     return ParsedMapValue(
-        value_factory_.GetMemoryManager().MakeShared<NonTrivialMapValueImpl>(
+        MemoryManager::ReferenceCounting().MakeShared<NonTrivialMapValueImpl>(
             std::move(map_)));
   }
 
  private:
-  ValueFactory& value_factory_;
   NonTrivialValueFlatHashMap map_;
 };
 
@@ -1644,13 +1637,17 @@ const MutableMapValue& GetMutableMapValue(const MapValue& value) {
 }
 
 absl::Nonnull<cel::MapValueBuilderPtr> NewMapValueBuilder(
-    ValueFactory& value_factory) {
-  if (absl::Nullable<google::protobuf::Arena*> arena =
-          value_factory.GetMemoryManager().arena();
+    Allocator<> allocator) {
+  if (absl::Nullable<google::protobuf::Arena*> arena = allocator.arena();
       arena != nullptr) {
-    return std::make_unique<TrivialMapValueBuilderImpl>(value_factory, arena);
+    return std::make_unique<TrivialMapValueBuilderImpl>(arena);
   }
-  return std::make_unique<NonTrivialMapValueBuilderImpl>(value_factory);
+  return std::make_unique<NonTrivialMapValueBuilderImpl>();
+}
+
+absl::Nonnull<cel::MapValueBuilderPtr> NewMapValueBuilder(
+    ValueFactory& value_factory) {
+  return NewMapValueBuilder(value_factory.GetMemoryManager());
 }
 
 }  // namespace common_internal

eval/eval/create_list_step.cc

@@ -12,7 +12,6 @@
 #include "absl/types/optional.h"
 #include "base/ast_internal/expr.h"
 #include "common/casting.h"
-#include "common/type.h"
 #include "common/value.h"
 #include "common/values/list_value_builder.h"
 #include "eval/eval/attribute_trail.h"
@@ -29,9 +28,10 @@ namespace {
 using ::cel::Cast;
 using ::cel::ErrorValue;
 using ::cel::InstanceOf;
-using ::cel::ListValueBuilderInterface;
+using ::cel::ListValueBuilderPtr;
 using ::cel::UnknownValue;
 using ::cel::Value;
+using ::cel::common_internal::NewListValueBuilder;
 
 class CreateListStep : public ExpressionStepBase {
  public:
@@ -83,8 +83,7 @@ absl::Status CreateListStep::Evaluate(ExecutionFrame* frame) const {
     }
   }
 
-  CEL_ASSIGN_OR_RETURN(auto builder, frame->value_manager().NewListValueBuilder(
-                                         cel::ListType()));
+  ListValueBuilderPtr builder = NewListValueBuilder(frame->memory_manager());
 
   builder->Reserve(args.size());
   for (size_t i = 0; i < args.size(); ++i) {
@@ -130,9 +129,8 @@ class CreateListDirectStep : public DirectExpressionStep {
 
   absl::Status Evaluate(ExecutionFrameBase& frame, Value& result,
                         AttributeTrail& attribute_trail) const override {
-    CEL_ASSIGN_OR_RETURN(
-        auto builder,
-        frame.value_manager().NewListValueBuilder(cel::ListType()));
+    ListValueBuilderPtr builder =
+        NewListValueBuilder(frame.value_manager().GetMemoryManager());
 
     builder->Reserve(elements_.size());
     AttributeUtility::Accumulator unknowns =
@@ -231,8 +229,6 @@ class DirectMutableListStep : public DirectExpressionStep {
 absl::Status DirectMutableListStep::Evaluate(
     ExecutionFrameBase& frame, Value& result,
     AttributeTrail& attribute_trail) const {
-  CEL_ASSIGN_OR_RETURN(
-      auto builder, frame.value_manager().NewListValueBuilder(cel::ListType()));
   result = cel::ParsedListValue(cel::common_internal::NewMutableListValue(
       frame.value_manager().GetMemoryManager().arena()));
   return absl::OkStatus();

eval/eval/create_map_step.cc

@@ -26,9 +26,9 @@
 #include "absl/strings/string_view.h"
 #include "absl/types/optional.h"
 #include "common/casting.h"
-#include "common/type.h"
 #include "common/value.h"
 #include "common/value_manager.h"
+#include "common/values/map_value_builder.h"
 #include "eval/eval/attribute_trail.h"
 #include "eval/eval/direct_expression_step.h"
 #include "eval/eval/evaluator_core.h"
@@ -42,9 +42,10 @@ namespace {
 using ::cel::Cast;
 using ::cel::ErrorValue;
 using ::cel::InstanceOf;
-using ::cel::StructValueBuilderInterface;
+using ::cel::MapValueBuilderPtr;
 using ::cel::UnknownValue;
 using ::cel::Value;
+using ::cel::common_internal::NewMapValueBuilder;
 
 // `CreateStruct` implementation for map.
 class CreateStructStepForMap final : public ExpressionStepBase {
@@ -77,8 +78,7 @@ absl::StatusOr<Value> CreateStructStepForMap::DoEvaluate(
     }
   }
 
-  CEL_ASSIGN_OR_RETURN(
-      auto builder, frame->value_manager().NewMapValueBuilder(cel::MapType{}));
+  MapValueBuilderPtr builder = NewMapValueBuilder(frame->memory_manager());
   builder->Reserve(entry_count_);
 
   for (size_t i = 0; i < entry_count_; i += 1) {
@@ -151,8 +151,8 @@ absl::Status DirectCreateMapStep::Evaluate(
   AttributeTrail tmp_attr;
   auto unknowns = frame.attribute_utility().CreateAccumulator();
 
-  CEL_ASSIGN_OR_RETURN(
-      auto builder, frame.value_manager().NewMapValueBuilder(cel::MapType()));
+  MapValueBuilderPtr builder =
+      NewMapValueBuilder(frame.value_manager().GetMemoryManager());
   builder->Reserve(entry_count_);
 
   for (size_t i = 0; i < entry_count_; i += 1) {