Begin getting rid of cel::MemoryManager

common/BUILD

@@ -506,7 +506,6 @@ cc_library(
         ":allocator",
         "//internal:to_address",
         "@com_google_absl//absl/base:core_headers",
-        "@com_google_absl//absl/base:no_destructor",
         "@com_google_absl//absl/base:nullability",
         "@com_google_absl//absl/log:absl_check",
         "@com_google_absl//absl/numeric:bits",

common/memory.cc

@@ -19,10 +19,8 @@
 #include <new>
 #include <ostream>
 
-#include "absl/base/no_destructor.h"
 #include "absl/log/absl_check.h"
 #include "absl/numeric/bits.h"
-#include "google/protobuf/arena.h"
 
 namespace cel {
 
@@ -73,11 +71,4 @@ bool ReferenceCountingMemoryManager::Deallocate(void* ptr, size_t size,
   return true;
 }
 
-MemoryManager MemoryManager::Unmanaged() {
-  // A static singleton arena, using `absl::NoDestructor` to avoid warnings
-  // related static variables without trivial destructors.
-  static absl::NoDestructor<google::protobuf::Arena> arena;
-  return MemoryManager::Pooling(&*arena);
-}
-
 }  // namespace cel

common/memory.h

@@ -152,17 +152,6 @@ class PoolingMemoryManager final {
 // resources.
 class MemoryManager final {
  public:
-  // Returns a `MemoryManager` which utilizes an arena but never frees its
-  // memory. It is effectively a memory leak and should only be used for limited
-  // use cases, such as initializing singletons which live for the life of the
-  // program.
-  static MemoryManager Unmanaged();
-
-  // Returns a `MemoryManager` which utilizes reference counting.
-  ABSL_MUST_USE_RESULT static MemoryManager ReferenceCounting() {
-    return MemoryManager(nullptr);
-  }
-
   // Returns a `MemoryManager` which utilizes an arena.
   ABSL_MUST_USE_RESULT static MemoryManager Pooling(
       google::protobuf::Arena* absl_nonnull arena) {

eval/public/BUILD

@@ -96,7 +96,6 @@ cc_library(
         "//common:native_type",
         "//eval/internal:errors",
         "//eval/public/structs:legacy_type_info_apis",
-        "//extensions/protobuf:memory_manager",
         "//internal:casts",
         "//internal:status_macros",
         "//internal:utf8",

eval/public/cel_value.cc

@@ -14,10 +14,8 @@
 #include "absl/strings/str_join.h"
 #include "absl/strings/string_view.h"
 #include "absl/types/optional.h"
-#include "common/memory.h"
 #include "eval/internal/errors.h"
 #include "eval/public/structs/legacy_type_info_apis.h"
-#include "extensions/protobuf/memory_manager.h"
 #include "google/protobuf/arena.h"
 
 namespace google::api::expr::runtime {
@@ -286,23 +284,6 @@ CelValue CelValue::CreateList() { return CreateList(EmptyCelList::Get()); }
 
 CelValue CelValue::CreateMap() { return CreateMap(EmptyCelMap::Get()); }
 
-CelValue CreateErrorValue(cel::MemoryManagerRef manager,
-                          absl::string_view message,
-                          absl::StatusCode error_code) {
-  // TODO(uncreated-issue/1): assume arena-style allocator while migrating to new
-  // value type.
-  Arena* arena = cel::extensions::ProtoMemoryManagerArena(manager);
-  return CreateErrorValue(arena, message, error_code);
-}
-
-CelValue CreateErrorValue(cel::MemoryManagerRef manager,
-                          const absl::Status& status) {
-  // TODO(uncreated-issue/1): assume arena-style allocator while migrating to new
-  // value type.
-  Arena* arena = cel::extensions::ProtoMemoryManagerArena(manager);
-  return CreateErrorValue(arena, status);
-}
-
 CelValue CreateErrorValue(Arena* arena, absl::string_view message,
                           absl::StatusCode error_code) {
   CelError* error = Arena::Create<CelError>(arena, error_code, message);
@@ -314,12 +295,6 @@ CelValue CreateErrorValue(Arena* arena, const absl::Status& status) {
   return CelValue::CreateError(error);
 }
 
-CelValue CreateNoMatchingOverloadError(cel::MemoryManagerRef manager,
-                                       absl::string_view fn) {
-  return CelValue::CreateError(interop::CreateNoMatchingOverloadError(
-      cel::extensions::ProtoMemoryManagerArena(manager), fn));
-}
-
 CelValue CreateNoMatchingOverloadError(google::protobuf::Arena* arena,
                                        absl::string_view fn) {
   return CelValue::CreateError(
@@ -333,22 +308,10 @@ bool CheckNoMatchingOverloadError(CelValue value) {
                            cel::runtime_internal::kErrNoMatchingOverload);
 }
 
-CelValue CreateNoSuchFieldError(cel::MemoryManagerRef manager,
-                                absl::string_view field) {
-  return CelValue::CreateError(interop::CreateNoSuchFieldError(
-      cel::extensions::ProtoMemoryManagerArena(manager), field));
-}
-
 CelValue CreateNoSuchFieldError(google::protobuf::Arena* arena, absl::string_view field) {
   return CelValue::CreateError(interop::CreateNoSuchFieldError(arena, field));
 }
 
-CelValue CreateNoSuchKeyError(cel::MemoryManagerRef manager,
-                              absl::string_view key) {
-  return CelValue::CreateError(interop::CreateNoSuchKeyError(
-      cel::extensions::ProtoMemoryManagerArena(manager), key));
-}
-
 CelValue CreateNoSuchKeyError(google::protobuf::Arena* arena, absl::string_view key) {
   return CelValue::CreateError(interop::CreateNoSuchKeyError(arena, key));
 }
@@ -365,15 +328,6 @@ CelValue CreateMissingAttributeError(google::protobuf::Arena* arena,
       interop::CreateMissingAttributeError(arena, missing_attribute_path));
 }
 
-CelValue CreateMissingAttributeError(cel::MemoryManagerRef manager,
-                                     absl::string_view missing_attribute_path) {
-  // TODO(uncreated-issue/1): assume arena-style allocator while migrating
-  // to new value type.
-  return CelValue::CreateError(interop::CreateMissingAttributeError(
-      cel::extensions::ProtoMemoryManagerArena(manager),
-      missing_attribute_path));
-}
-
 bool IsMissingAttributeError(const CelValue& value) {
   const CelError* error;
   if (!value.GetValue(&error)) return false;
@@ -385,12 +339,6 @@ bool IsMissingAttributeError(const CelValue& value) {
   return false;
 }
 
-CelValue CreateUnknownFunctionResultError(cel::MemoryManagerRef manager,
-                                          absl::string_view help_message) {
-  return CelValue::CreateError(interop::CreateUnknownFunctionResultError(
-      cel::extensions::ProtoMemoryManagerArena(manager), help_message));
-}
-
 CelValue CreateUnknownFunctionResultError(google::protobuf::Arena* arena,
                                           absl::string_view help_message) {
   return CelValue::CreateError(

eval/public/cel_value.h

@@ -653,70 +653,84 @@ class CelMap {
 // Utility method that generates CelValue containing CelError.
 // message an error message
 // error_code error code
-CelValue CreateErrorValue(
-    cel::MemoryManagerRef manager ABSL_ATTRIBUTE_LIFETIME_BOUND,
-    absl::string_view message,
-    absl::StatusCode error_code = absl::StatusCode::kUnknown);
 CelValue CreateErrorValue(
     google::protobuf::Arena* arena, absl::string_view message,
     absl::StatusCode error_code = absl::StatusCode::kUnknown);
-
-// Utility method for generating a CelValue from an absl::Status.
-CelValue CreateErrorValue(cel::MemoryManagerRef manager
-                              ABSL_ATTRIBUTE_LIFETIME_BOUND,
-                          const absl::Status& status);
+ABSL_DEPRECATE_AND_INLINE()
+inline CelValue CreateErrorValue(
+    cel::MemoryManagerRef manager ABSL_ATTRIBUTE_LIFETIME_BOUND,
+    absl::string_view message,
+    absl::StatusCode error_code = absl::StatusCode::kUnknown) {
+  return CreateErrorValue(manager.arena(), message, error_code);
+}
 
 // Utility method for generating a CelValue from an absl::Status.
 CelValue CreateErrorValue(google::protobuf::Arena* arena, const absl::Status& status);
+ABSL_DEPRECATE_AND_INLINE()
+inline CelValue CreateErrorValue(cel::MemoryManagerRef manager
+                                     ABSL_ATTRIBUTE_LIFETIME_BOUND,
+                                 const absl::Status& status) {
+  return CreateErrorValue(manager.arena(), status);
+}
 
 // Create an error for failed overload resolution, optionally including the name
 // of the function.
-CelValue CreateNoMatchingOverloadError(cel::MemoryManagerRef manager
-                                           ABSL_ATTRIBUTE_LIFETIME_BOUND,
-                                       absl::string_view fn = "");
-ABSL_DEPRECATED("Prefer using the generic MemoryManager overload")
 CelValue CreateNoMatchingOverloadError(google::protobuf::Arena* arena,
                                        absl::string_view fn = "");
+ABSL_DEPRECATE_AND_INLINE()
+inline CelValue CreateNoMatchingOverloadError(cel::MemoryManagerRef manager
+                                                  ABSL_ATTRIBUTE_LIFETIME_BOUND,
+                                              absl::string_view fn = "") {
+  return CreateNoMatchingOverloadError(manager.arena(), fn);
+}
 bool CheckNoMatchingOverloadError(CelValue value);
 
-CelValue CreateNoSuchFieldError(cel::MemoryManagerRef manager
-                                    ABSL_ATTRIBUTE_LIFETIME_BOUND,
-                                absl::string_view field = "");
-ABSL_DEPRECATED("Prefer using the generic MemoryManager overload")
 CelValue CreateNoSuchFieldError(google::protobuf::Arena* arena,
                                 absl::string_view field = "");
+ABSL_DEPRECATE_AND_INLINE()
+inline CelValue CreateNoSuchFieldError(cel::MemoryManagerRef manager
+                                           ABSL_ATTRIBUTE_LIFETIME_BOUND,
+                                       absl::string_view field = "") {
+  return CreateNoSuchFieldError(manager.arena(), field);
+}
 
-CelValue CreateNoSuchKeyError(cel::MemoryManagerRef manager
-                                  ABSL_ATTRIBUTE_LIFETIME_BOUND,
-                              absl::string_view key);
-ABSL_DEPRECATED("Prefer using the generic MemoryManager overload")
 CelValue CreateNoSuchKeyError(google::protobuf::Arena* arena, absl::string_view key);
+ABSL_DEPRECATE_AND_INLINE()
+inline CelValue CreateNoSuchKeyError(cel::MemoryManagerRef manager
+                                         ABSL_ATTRIBUTE_LIFETIME_BOUND,
+                                     absl::string_view key) {
+  return CreateNoSuchKeyError(manager.arena(), key);
+}
 
 bool CheckNoSuchKeyError(CelValue value);
 
 // Returns an error indicating that evaluation has accessed an attribute whose
 // value is undefined. For example, this may represent a field in a proto
 // message bound to the activation whose value can't be determined by the
 // hosting application.
-CelValue CreateMissingAttributeError(cel::MemoryManagerRef manager
-                                         ABSL_ATTRIBUTE_LIFETIME_BOUND,
-                                     absl::string_view missing_attribute_path);
-ABSL_DEPRECATED("Prefer using the generic MemoryManager overload")
 CelValue CreateMissingAttributeError(google::protobuf::Arena* arena,
                                      absl::string_view missing_attribute_path);
+ABSL_DEPRECATE_AND_INLINE()
+inline CelValue CreateMissingAttributeError(
+    cel::MemoryManagerRef manager ABSL_ATTRIBUTE_LIFETIME_BOUND,
+    absl::string_view missing_attribute_path) {
+  return CreateMissingAttributeError(manager.arena(), missing_attribute_path);
+}
 
 ABSL_CONST_INIT extern const absl::string_view kPayloadUrlMissingAttributePath;
 bool IsMissingAttributeError(const CelValue& value);
 
 // Returns error indicating the result of the function is unknown. This is used
 // as a signal to create an unknown set if unknown function handling is opted
 // into.
-CelValue CreateUnknownFunctionResultError(cel::MemoryManagerRef manager
-                                              ABSL_ATTRIBUTE_LIFETIME_BOUND,
-                                          absl::string_view help_message);
-ABSL_DEPRECATED("Prefer using the generic MemoryManager overload")
 CelValue CreateUnknownFunctionResultError(google::protobuf::Arena* arena,
                                           absl::string_view help_message);
+ABSL_DEPRECATE_AND_INLINE()
+inline CelValue CreateUnknownFunctionResultError(
+    cel::MemoryManagerRef manager ABSL_ATTRIBUTE_LIFETIME_BOUND,
+    absl::string_view help_message) {
+  return CreateUnknownFunctionResultError(manager.arena(), help_message);
+}
 
 // Returns true if this is unknown value error indicating that evaluation
 // called an extension function whose value is unknown for the given args.

extensions/protobuf/memory_manager.cc

@@ -23,8 +23,7 @@ namespace cel {
 namespace extensions {
 
 MemoryManagerRef ProtoMemoryManager(google::protobuf::Arena* arena) {
-  return arena != nullptr ? MemoryManagerRef::Pooling(arena)
-                          : MemoryManagerRef::ReferenceCounting();
+  return MemoryManagerRef::Pooling(arena);
 }
 
 google::protobuf::Arena* absl_nullable ProtoMemoryManagerArena(

extensions/protobuf/memory_manager_test.cc

@@ -22,7 +22,6 @@ namespace cel::extensions {
 namespace {
 
 using ::testing::Eq;
-using ::testing::IsNull;
 using ::testing::NotNull;
 
 TEST(ProtoMemoryManager, MemoryManagement) {
@@ -41,17 +40,12 @@ TEST(ProtoMemoryManagerRef, MemoryManagement) {
   google::protobuf::Arena arena;
   auto memory_manager = ProtoMemoryManagerRef(&arena);
   EXPECT_EQ(memory_manager.memory_management(), MemoryManagement::kPooling);
-  memory_manager = ProtoMemoryManagerRef(nullptr);
-  EXPECT_EQ(memory_manager.memory_management(),
-            MemoryManagement::kReferenceCounting);
 }
 
 TEST(ProtoMemoryManagerRef, Arena) {
   google::protobuf::Arena arena;
   auto memory_manager = ProtoMemoryManagerRef(&arena);
   EXPECT_THAT(ProtoMemoryManagerArena(memory_manager), Eq(&arena));
-  memory_manager = ProtoMemoryManagerRef(nullptr);
-  EXPECT_THAT(ProtoMemoryManagerArena(memory_manager), IsNull());
 }
 
 }  // namespace