Make ErrorValue trivial

common/BUILD

@@ -589,6 +589,7 @@ cc_test(
         "//internal:testing",
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/time",
+        "@com_google_protobuf//:protobuf",
     ],
 )
 

common/legacy_value.cc

@@ -721,7 +721,7 @@ absl::Status LegacyMapValue::Get(
   CEL_ASSIGN_OR_RETURN(auto cel_key, LegacyValue(arena, key));
   auto cel_value = impl_->Get(arena, cel_key);
   if (!cel_value.has_value()) {
-    *result = NoSuchKeyError(key.DebugString());
+    *result = NoSuchKeyError(key.DebugString(), arena);
     return absl::OkStatus();
   }
   CEL_RETURN_IF_ERROR(ModernValue(arena, *cel_value, *result));
@@ -928,7 +928,7 @@ absl::Status LegacyStructValue::GetFieldByName(
     google::protobuf::MessageFactory* absl_nonnull message_factory,
     google::protobuf::Arena* absl_nonnull arena, Value* absl_nonnull result) const {
   if (ABSL_PREDICT_FALSE(legacy_type_info_ == TrivialTypeInfo::GetInstance())) {
-    *result = NoSuchFieldError(name);
+    *result = NoSuchFieldError(name, arena);
     return absl::OkStatus();
   }
 
@@ -939,7 +939,7 @@ absl::Status LegacyStructValue::GetFieldByName(
     field = descriptor->file()->pool()->FindExtensionByPrintableName(descriptor,
                                                                      name);
     if (field == nullptr) {
-      *result = NoSuchFieldError(name);
+      *result = NoSuchFieldError(name, arena);
       return absl::OkStatus();
     }
   }
@@ -962,15 +962,15 @@ absl::StatusOr<bool> LegacyStructValue::HasFieldByName(
     absl::string_view name) const {
   ABSL_DCHECK(message_ptr_ != nullptr);
   if (ABSL_PREDICT_FALSE(legacy_type_info_ == TrivialTypeInfo::GetInstance())) {
-    return NoSuchFieldError(name).ToStatus();
+    return common_internal::MakeNoSuchFieldError(name);
   }
   return UnsafeParsedMessageValue(message_ptr_).HasFieldByName(name);
 }
 
 absl::StatusOr<bool> LegacyStructValue::HasFieldByNumber(int64_t number) const {
   ABSL_DCHECK(message_ptr_ != nullptr);
   if (ABSL_PREDICT_FALSE(legacy_type_info_ == TrivialTypeInfo::GetInstance())) {
-    return NoSuchFieldError(absl::StrCat(number)).ToStatus();
+    return common_internal::MakeNoSuchFieldError(absl::StrCat(number));
   }
   return UnsafeParsedMessageValue(message_ptr_).HasFieldByNumber(number);
 }
@@ -1008,7 +1008,7 @@ absl::Status LegacyStructValue::Qualify(
               return field.GetStringKey().value_or("<invalid field>");
             }),
         qualifiers.front());
-    *result = NoSuchFieldError(field_name);
+    *result = NoSuchFieldError(field_name, arena);
     *count = -1;
     return absl::OkStatus();
   }
@@ -1084,7 +1084,7 @@ absl::Status ModernValue(google::protobuf::Arena* arena,
       return absl::OkStatus();
     }
     case CelValue::Type::kError:
-      result = ErrorValue{*legacy_value.ErrorOrDie()};
+      result = ErrorValue::From(*legacy_value.ErrorOrDie(), arena);
       return absl::OkStatus();
     case CelValue::Type::kAny:
       return absl::InternalError(absl::StrCat(

common/value.cc

@@ -200,9 +200,8 @@ absl::Status Value::ConvertToJsonArray(
                                               json);
       },
       [](const auto& alternative) -> absl::Status {
-        return TypeConversionError(alternative.GetTypeName(),
-                                   "google.protobuf.ListValue")
-            .NativeValue();
+        return common_internal::MakeTypeConversionError(
+            alternative.GetTypeName(), "google.protobuf.ListValue");
       }));
 }
 
@@ -257,9 +256,8 @@ absl::Status Value::ConvertToJsonObject(
                                                json);
       },
       [](const auto& alternative) -> absl::Status {
-        return TypeConversionError(alternative.GetTypeName(),
-                                   "google.protobuf.Struct")
-            .NativeValue();
+        return common_internal::MakeTypeConversionError(
+            alternative.GetTypeName(), "google.protobuf.Struct");
       }));
 }
 

common/value.h

@@ -2612,8 +2612,7 @@ static_assert(std::is_nothrow_swappable_v<Value>);
 
 inline common_internal::ImplicitlyConvertibleStatus
 ErrorValueAssign::operator()(absl::Status status) const {
-  *value_ = arena_ != nullptr ? ErrorValue::From(std::move(status), arena_)
-                              : ErrorValue(std::move(status));
+  *value_ = ErrorValue::From(std::move(status), arena_);
   return common_internal::ImplicitlyConvertibleStatus();
 }
 

common/value_testing_test.cc

@@ -21,6 +21,7 @@
 #include "absl/time/time.h"
 #include "common/value.h"
 #include "internal/testing.h"
+#include "google/protobuf/arena.h"
 
 namespace cel::test {
 namespace {
@@ -156,12 +157,14 @@ TEST(BytesValueIs, NonMatchMessage) {
 }
 
 TEST(ErrorValueIs, Match) {
-  EXPECT_THAT(ErrorValue(absl::InternalError("test")),
+  google::protobuf::Arena arena;
+  EXPECT_THAT(ErrorValue::From(absl::InternalError("test"), &arena),
               ErrorValueIs(StatusIs(absl::StatusCode::kInternal, "test")));
 }
 
 TEST(ErrorValueIs, NoMatch) {
-  EXPECT_THAT(ErrorValue(absl::UnknownError("test")),
+  google::protobuf::Arena arena;
+  EXPECT_THAT(ErrorValue::From(absl::UnknownError("test"), &arena),
               Not(ErrorValueIs(StatusIs(absl::StatusCode::kInternal, "test"))));
   EXPECT_THAT(IntValue(2), Not(ErrorValueIs(_)));
 }

common/values/custom_list_value.cc

@@ -97,9 +97,9 @@ class EmptyListValue final : public common_internal::CompatListValue {
  private:
   absl::Status Get(size_t index, const google::protobuf::DescriptorPool* absl_nonnull,
                    google::protobuf::MessageFactory* absl_nonnull,
-                   google::protobuf::Arena* absl_nonnull,
+                   google::protobuf::Arena* absl_nonnull arena,
                    Value* absl_nonnull result) const override {
-    *result = IndexOutOfBoundsError(index);
+    *result = IndexOutOfBoundsError(index, arena);
     return absl::OkStatus();
   }
 };

common/values/custom_list_value_test.cc

@@ -24,7 +24,6 @@
 #include "absl/status/statusor.h"
 #include "absl/strings/cord.h"
 #include "absl/strings/string_view.h"
-#include "absl/types/optional.h"
 #include "common/memory.h"
 #include "common/native_type.h"
 #include "common/value.h"
@@ -121,7 +120,7 @@ class CustomListValueInterfaceTest final : public CustomListValueInterface {
       *result = IntValue(1);
       return absl::OkStatus();
     }
-    *result = IndexOutOfBoundsError(index);
+    *result = IndexOutOfBoundsError(index, arena);
     return absl::OkStatus();
   }
 
@@ -221,7 +220,7 @@ class CustomListValueTest : public common_internal::ValueTest<> {
           *result = IntValue(1);
           return absl::OkStatus();
         }
-        *result = IndexOutOfBoundsError(index);
+        *result = IndexOutOfBoundsError(index, arena);
         return absl::OkStatus();
       },
       .clone = [](const CustomListValueDispatcher* absl_nonnull dispatcher,

common/values/custom_map_value_test.cc

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include <cstddef>
+#include <optional>
 #include <string>
 #include <utility>
 #include <vector>
@@ -24,7 +25,6 @@
 #include "absl/status/statusor.h"
 #include "absl/strings/cord.h"
 #include "absl/strings/string_view.h"
-#include "absl/types/optional.h"
 #include "common/memory.h"
 #include "common/native_type.h"
 #include "common/value.h"
@@ -591,7 +591,8 @@ TEST_F(CustomMapValueTest, Interface_Find_InvalidKeyType) {
 TEST_F(CustomMapValueTest, Dispatcher_Find_SpecialKeys) {
   CustomMapValue map = MakeDispatcher();
   Value result;
-  ErrorValue error_key(absl::CancelledError("cancelled"));
+  ErrorValue error_key =
+      ErrorValue::From(absl::CancelledError("cancelled"), arena());
   ASSERT_THAT(map.Find(error_key, descriptor_pool(), message_factory(), arena(),
                        &result),
               IsOkAndHolds(false));
@@ -623,7 +624,8 @@ TEST_F(CustomMapValueTest, Dispatcher_Find_SpecialKeys) {
 TEST_F(CustomMapValueTest, Interface_Find_SpecialKeys) {
   CustomMapValue map = MakeInterface();
   Value result;
-  ErrorValue error_key(absl::CancelledError("cancelled"));
+  ErrorValue error_key =
+      ErrorValue::From(absl::CancelledError("cancelled"), arena());
   ASSERT_THAT(map.Find(error_key, descriptor_pool(), message_factory(), arena(),
                        &result),
               IsOkAndHolds(false));
@@ -735,7 +737,8 @@ TEST_F(CustomMapValueTest, Interface_Has_InvalidKeyType) {
 TEST_F(CustomMapValueTest, Dispatcher_Has_SpecialKeys) {
   CustomMapValue map = MakeDispatcher();
   Value result;
-  ErrorValue error_key(absl::CancelledError("cancelled"));
+  ErrorValue error_key =
+      ErrorValue::From(absl::CancelledError("cancelled"), arena());
   ASSERT_THAT(map.Has(error_key, descriptor_pool(), message_factory(), arena(),
                       &result),
               IsOk());
@@ -758,7 +761,8 @@ TEST_F(CustomMapValueTest, Dispatcher_Has_SpecialKeys) {
 TEST_F(CustomMapValueTest, Interface_Has_SpecialKeys) {
   CustomMapValue map = MakeInterface();
   Value result;
-  ErrorValue error_key(absl::CancelledError("cancelled"));
+  ErrorValue error_key =
+      ErrorValue::From(absl::CancelledError("cancelled"), arena());
   ASSERT_THAT(map.Has(error_key, descriptor_pool(), message_factory(), arena(),
                       &result),
               IsOk());

common/values/custom_struct_value_test.cc

@@ -117,7 +117,7 @@ class CustomStructValueInterfaceTest final : public CustomStructValueInterface {
       *result = IntValue(1);
       return absl::OkStatus();
     }
-    return NoSuchFieldError(name).ToStatus();
+    return common_internal::MakeNoSuchFieldError(name);
   }
 
   absl::Status GetFieldByNumber(
@@ -134,7 +134,7 @@ class CustomStructValueInterfaceTest final : public CustomStructValueInterface {
       *result = IntValue(1);
       return absl::OkStatus();
     }
-    return NoSuchFieldError(absl::StrCat(number)).ToStatus();
+    return common_internal::MakeNoSuchFieldError(absl::StrCat(number));
   }
 
   absl::StatusOr<bool> HasFieldByName(absl::string_view name) const override {
@@ -144,7 +144,7 @@ class CustomStructValueInterfaceTest final : public CustomStructValueInterface {
     if (name == "bar") {
       return true;
     }
-    return NoSuchFieldError(name).ToStatus();
+    return common_internal::MakeNoSuchFieldError(name);
   }
 
   absl::StatusOr<bool> HasFieldByNumber(int64_t number) const override {
@@ -154,7 +154,7 @@ class CustomStructValueInterfaceTest final : public CustomStructValueInterface {
     if (number == 2) {
       return true;
     }
-    return NoSuchFieldError(absl::StrCat(number)).ToStatus();
+    return common_internal::MakeNoSuchFieldError(absl::StrCat(number));
   }
 
   absl::Status ForEachField(
@@ -283,7 +283,7 @@ class CustomStructValueTest : public common_internal::ValueTest<> {
           *result = IntValue(1);
           return absl::OkStatus();
         }
-        return NoSuchFieldError(name).ToStatus();
+        return common_internal::MakeNoSuchFieldError(name);
       },
       .get_field_by_number =
           [](const CustomStructValueDispatcher* absl_nonnull dispatcher,
@@ -301,7 +301,7 @@ class CustomStructValueTest : public common_internal::ValueTest<> {
           *result = IntValue(1);
           return absl::OkStatus();
         }
-        return NoSuchFieldError(absl::StrCat(number)).ToStatus();
+        return common_internal::MakeNoSuchFieldError(absl::StrCat(number));
       },
       .has_field_by_name =
           [](const CustomStructValueDispatcher* absl_nonnull dispatcher,
@@ -313,7 +313,7 @@ class CustomStructValueTest : public common_internal::ValueTest<> {
         if (name == "bar") {
           return true;
         }
-        return NoSuchFieldError(name).ToStatus();
+        return common_internal::MakeNoSuchFieldError(name);
       },
       .has_field_by_number =
           [](const CustomStructValueDispatcher* absl_nonnull dispatcher,
@@ -325,7 +325,7 @@ class CustomStructValueTest : public common_internal::ValueTest<> {
         if (number == 2) {
           return true;
         }
-        return NoSuchFieldError(absl::StrCat(number)).ToStatus();
+        return common_internal::MakeNoSuchFieldError(absl::StrCat(number));
       },
       .for_each_field =
           [](const CustomStructValueDispatcher* absl_nonnull dispatcher,