Skip to content

Migrate some from cel::ErrorValue() to cel::ErrorValue::From() #2297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
copybara-service merged 1 commit into from
Sep 28, 2026
Merged

Migrate some from cel::ErrorValue() to cel::ErrorValue::From() #2297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions codelab/network_functions.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,8 @@ cel::Value parseAddress(
absl::string_view addr = str.ToStringView(&buf);
std::optional<NetworkAddressRep> rep = NetworkAddressRep::Parse(addr);
if (!rep.has_value()) {
return cel::ErrorValue(absl::InvalidArgumentError("invalid address"));
return cel::ErrorValue::From(absl::InvalidArgumentError("invalid address"),
arena);
}
return NetworkAddressRep::MakeValue(*rep);
}
Expand All @@ -337,21 +338,25 @@ cel::Value parseAddressMatcher(
absl::string_view addr = str.ToStringView(&buf);
std::optional<NetworkAddressMatcher> rep = NetworkAddressMatcher::Parse(addr);
if (!rep.has_value()) {
return cel::ErrorValue(
absl::InvalidArgumentError("invalid address matcher"));
return cel::ErrorValue::From(
absl::InvalidArgumentError("invalid address matcher"), arena);
}

return NetworkAddressMatcher::MakeValue(arena, std::move(rep).value());
}

cel::Value containsAddress(const cel::OpaqueValue& matcher,
const cel::OpaqueValue& addr) {
cel::Value containsAddress(
const cel::OpaqueValue& matcher, const cel::OpaqueValue& addr,
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
google::protobuf::MessageFactory* absl_nonnull message_factory,
google::protobuf::Arena* absl_nonnull arena) {
const auto* matcher_rep = NetworkAddressMatcher::Unwrap(matcher);
auto addr_rep = NetworkAddressRep::Unwrap(addr);
if (matcher_rep == nullptr || !addr_rep.has_value()) {
// dispatcher should catch this, but right now only distiguishes at the
// kind level.
return cel::ErrorValue(absl::InvalidArgumentError("no matching overload"));
return cel::ErrorValue::From(
absl::InvalidArgumentError("no matching overload"), arena);
}
return cel::BoolValue(matcher_rep->Match(*addr_rep));
}
Expand Down
1 change: 1 addition & 0 deletions common/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,7 @@ cc_library(
"@com_google_absl//absl/strings:string_view",
"@com_google_absl//absl/time",
"@com_google_absl//absl/types:optional",
"@com_google_absl//absl/types:source_location",
"@com_google_absl//absl/types:span",
"@com_google_absl//absl/types:variant",
"@com_google_absl//absl/utility",
Expand Down
13 changes: 7 additions & 6 deletions common/legacy_value.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,8 @@ absl::Status LegacyListValue::Get(
google::protobuf::MessageFactory* absl_nonnull message_factory,
google::protobuf::Arena* absl_nonnull arena, Value* absl_nonnull result) const {
if (ABSL_PREDICT_FALSE(index < 0 || index >= impl_->size())) {
*result = ErrorValue(absl::InvalidArgumentError("index out of bounds"));
*result = ErrorValue::From(
absl::InvalidArgumentError("index out of bounds"), arena);
return absl::OkStatus();
}
CEL_RETURN_IF_ERROR(
Expand Down Expand Up @@ -714,7 +715,7 @@ absl::Status LegacyMapValue::Get(
case ValueKind::kString:
break;
default:
*result = ErrorValue(InvalidMapKeyTypeError(key.kind()));
*result = ErrorValue::From(InvalidMapKeyTypeError(key.kind()), arena);
return absl::OkStatus();
}
CEL_ASSIGN_OR_RETURN(auto cel_key, LegacyValue(arena, key));
Expand Down Expand Up @@ -747,7 +748,7 @@ absl::StatusOr<bool> LegacyMapValue::Find(
case ValueKind::kString:
break;
default:
*result = ErrorValue(InvalidMapKeyTypeError(key.kind()));
*result = ErrorValue::From(InvalidMapKeyTypeError(key.kind()), arena);
}
CEL_ASSIGN_OR_RETURN(auto cel_key, LegacyValue(arena, key));
auto cel_value = impl_->Get(arena, cel_key);
Expand Down Expand Up @@ -779,13 +780,13 @@ absl::Status LegacyMapValue::Has(
case ValueKind::kString:
break;
default:
*result = ErrorValue(InvalidMapKeyTypeError(key.kind()));
*result = ErrorValue::From(InvalidMapKeyTypeError(key.kind()), arena);
return absl::OkStatus();
}
CEL_ASSIGN_OR_RETURN(auto cel_key, LegacyValue(arena, key));
absl::StatusOr<bool> has = impl_->Has(cel_key);
if (!has.ok()) {
*result = ErrorValue(std::move(has).status());
*result = ErrorValue::From(std::move(has).status(), arena);
return absl::OkStatus();
}

Expand Down Expand Up @@ -1192,7 +1193,7 @@ absl::StatusOr<Value> FromLegacyValue(google::protobuf::Arena* arena,
return CreateTypeValueFromView(arena,
legacy_value.CelTypeOrDie().value());
case CelValue::Type::kError:
return ErrorValue(*legacy_value.ErrorOrDie());
return ErrorValue::From(*legacy_value.ErrorOrDie(), arena);
case CelValue::Type::kAny:
return absl::InternalError(absl::StrCat(
"illegal attempt to convert special CelValue type ",
Expand Down
80 changes: 55 additions & 25 deletions common/value.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

#include "google/protobuf/struct.pb.h"
#include "absl/base/attributes.h"
#include "absl/base/no_destructor.h"
#include "absl/base/nullability.h"
#include "absl/base/optimization.h"
#include "absl/functional/overload.h"
Expand All @@ -39,6 +40,7 @@
#include "absl/strings/string_view.h"
#include "absl/time/time.h"
#include "absl/types/optional.h"
#include "absl/types/source_location.h"
#include "absl/types/variant.h"
#include "common/allocator.h"
#include "common/memory.h"
Expand Down Expand Up @@ -322,12 +324,20 @@ Value NonNullEnumValue(const google::protobuf::EnumValueDescriptor* absl_nonnull
}

Value NonNullEnumValue(const google::protobuf::EnumDescriptor* absl_nonnull type,
int32_t number) {
int32_t number, google::protobuf::Arena* absl_nullable arena) {
ABSL_DCHECK(type != nullptr);
if (type->is_closed()) {
if (ABSL_PREDICT_FALSE(type->FindValueByNumber(number) == nullptr)) {
return ErrorValue(absl::InvalidArgumentError(absl::StrCat(
"closed enum has no such value: ", type->full_name(), ".", number)));
if (arena == nullptr) {
static const absl::NoDestructor<absl::Status> error(
absl::InvalidArgumentError("closed enum has no such value",
absl::SourceLocation()));
return ErrorValue::WrapUnsafe(&*error);
}
return ErrorValue::From(absl::InvalidArgumentError(absl::StrCat(
"closed enum has no such value: ",
type->full_name(), ".", number)),
arena);
}
}
return IntValue(number);
Expand All @@ -351,7 +361,18 @@ Value Value::Enum(const google::protobuf::EnumDescriptor* absl_nonnull type,
ABSL_DCHECK_EQ(number, 0);
return NullValue();
}
return NonNullEnumValue(type, number);
return NonNullEnumValue(type, number, nullptr);
}

Value Value::Enum(const google::protobuf::EnumDescriptor* absl_nonnull type,
int32_t number, google::protobuf::Arena* absl_nonnull arena) {
ABSL_DCHECK(type != nullptr);
ABSL_DCHECK(arena != nullptr);
if (type->full_name() == "google.protobuf.NullValue") {
ABSL_DCHECK_EQ(number, 0);
return NullValue();
}
return NonNullEnumValue(type, number, arena);
}

namespace common_internal {
Expand Down Expand Up @@ -669,7 +690,7 @@ void EnumMapFieldValueAccessor(
ABSL_DCHECK(!field->is_repeated());
ABSL_DCHECK_EQ(field->cpp_type(), google::protobuf::FieldDescriptor::CPPTYPE_ENUM);

*result = NonNullEnumValue(field->enum_type(), value.GetEnumValue());
*result = NonNullEnumValue(field->enum_type(), value.GetEnumValue(), arena);
}

void NullMapFieldValueAccessor(
Expand Down Expand Up @@ -1044,7 +1065,7 @@ void EnumRepeatedFieldAccessor(

*result = NonNullEnumValue(
field->enum_type(),
reflection->GetRepeatedEnumValue(*message, field, index));
reflection->GetRepeatedEnumValue(*message, field, index), arena);
}

void NullRepeatedFieldAccessor(
Expand Down Expand Up @@ -1363,7 +1384,7 @@ Value Value::FromMessage(
auto status_or_adapted = well_known_types::AdaptFromMessage(
arena, message, descriptor_pool, message_factory, scratch);
if (ABSL_PREDICT_FALSE(!status_or_adapted.ok())) {
return ErrorValue(std::move(status_or_adapted).status());
return ErrorValue::From(std::move(status_or_adapted).status(), arena);
}
return absl::visit(
absl::Overload(OwningWellKnownTypesValueVisitor{
Expand Down Expand Up @@ -1391,7 +1412,7 @@ Value Value::FromMessage(
auto status_or_adapted = well_known_types::AdaptFromMessage(
arena, message, descriptor_pool, message_factory, scratch);
if (ABSL_PREDICT_FALSE(!status_or_adapted.ok())) {
return ErrorValue(std::move(status_or_adapted).status());
return ErrorValue::From(std::move(status_or_adapted).status(), arena);
}
return absl::visit(
absl::Overload(OwningWellKnownTypesValueVisitor{
Expand Down Expand Up @@ -1421,7 +1442,7 @@ Value Value::WrapMessage(
well_known_types::AdaptFromMessage(arena, *message, descriptor_pool,
message_factory, scratch);
if (ABSL_PREDICT_FALSE(!adapted_value.ok())) {
return ErrorValue(std::move(adapted_value).status());
return ErrorValue::From(std::move(adapted_value).status(), arena);
}
return absl::visit(
absl::Overload(BorrowingWellKnownTypesValueVisitor{
Expand Down Expand Up @@ -1455,7 +1476,7 @@ Value Value::WrapMessageUnsafe(
well_known_types::AdaptFromMessage(arena, *message, descriptor_pool,
message_factory, scratch);
if (ABSL_PREDICT_FALSE(!adapted_value.ok())) {
return ErrorValue(std::move(adapted_value).status());
return ErrorValue::From(std::move(adapted_value).status(), arena);
}
return absl::visit(
absl::Overload(BorrowingWellKnownTypesValueVisitor{
Expand Down Expand Up @@ -1519,9 +1540,10 @@ Value WrapFieldImpl(
if (ABSL_PREDICT_FALSE(reflection == nullptr)) {
// This only happens for special implementations of Message that
// should not normally be used with CEL.
return ErrorValue(absl::InvalidArgumentError(
absl::StrCat("failed to get reflection for message type: ",
message->GetDescriptor()->full_name())));
return ErrorValue::From(absl::InvalidArgumentError(absl::StrCat(
"failed to get reflection for message type: ",
message->GetDescriptor()->full_name())),
arena);
}
if (field->is_map()) {
if constexpr (Unsafe::value) {
Expand Down Expand Up @@ -1631,9 +1653,11 @@ Value WrapFieldImpl(
case google::protobuf::FieldDescriptor::TYPE_SINT64:
return IntValue(reflection->GetInt64(*message, field));
default:
return ErrorValue(absl::InvalidArgumentError(
absl::StrCat("unexpected protocol buffer message field type: ",
field->type_name())));
return ErrorValue::From(
absl::InvalidArgumentError(
absl::StrCat("unexpected protocol buffer message field type: ",
field->type_name())),
arena);
}
}

Expand Down Expand Up @@ -1661,14 +1685,16 @@ Value WrapRepeatedFieldImpl(
if (ABSL_PREDICT_FALSE(reflection == nullptr)) {
// This only happens for special implementations of Message that
// should not normally be used with CEL.
return ErrorValue(absl::InvalidArgumentError(
absl::StrCat("failed to get reflection for message type: ",
message->GetDescriptor()->full_name())));
return ErrorValue::From(absl::InvalidArgumentError(absl::StrCat(
"failed to get reflection for message type: ",
message->GetDescriptor()->full_name())),
arena);
}
const int size = reflection->FieldSize(*message, field);
if (ABSL_PREDICT_FALSE(index < 0 || index >= size)) {
return ErrorValue(absl::InvalidArgumentError(
absl::StrCat("index out of bounds: ", index)));
return ErrorValue::From(absl::InvalidArgumentError(
absl::StrCat("index out of bounds: ", index)),
arena);
}
switch (field->type()) {
case google::protobuf::FieldDescriptor::TYPE_DOUBLE:
Expand Down Expand Up @@ -1757,8 +1783,10 @@ Value WrapRepeatedFieldImpl(
return Value::Enum(field->enum_type(), reflection->GetRepeatedEnumValue(
*message, field, index));
default:
return ErrorValue(absl::InvalidArgumentError(
absl::StrCat("unexpected message field type: ", field->type_name())));
return ErrorValue::From(
absl::InvalidArgumentError(absl::StrCat(
"unexpected message field type: ", field->type_name())),
arena);
}
}

Expand Down Expand Up @@ -1837,8 +1865,10 @@ Value WrapMapFieldValueImpl(
case google::protobuf::FieldDescriptor::TYPE_ENUM:
return Value::Enum(field->enum_type(), value.GetEnumValue());
default:
return ErrorValue(absl::InvalidArgumentError(
absl::StrCat("unexpected message field type: ", field->type_name())));
return ErrorValue::From(
absl::InvalidArgumentError(absl::StrCat(
"unexpected message field type: ", field->type_name())),
arena);
}
}

Expand Down
3 changes: 3 additions & 0 deletions common/value.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,11 @@ class Value final : private common_internal::ValueMixin<Value> {
// enums, returns `cel::IntValue`. For closed enums, returns `cel::ErrorValue`
// if the value is not present in the enum otherwise returns `cel::IntValue`.
static Value Enum(const google::protobuf::EnumValueDescriptor* absl_nonnull value);
ABSL_DEPRECATED("Use overload which takes an arena pointer")
static Value Enum(const google::protobuf::EnumDescriptor* absl_nonnull type,
int32_t number);
static Value Enum(const google::protobuf::EnumDescriptor* absl_nonnull type,
int32_t number, google::protobuf::Arena* absl_nonnull arena);

// SFINAE overload for generated protobuf enums which are not well-known.
// Always returns `cel::IntValue`.
Expand Down
18 changes: 9 additions & 9 deletions common/values/custom_map_value.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ absl::Status CustomMapValueInterface::ForEach(
CEL_ASSIGN_OR_RETURN(
bool found, Find(key, descriptor_pool, message_factory, arena, &value));
if (!found) {
value = ErrorValue(NoSuchKeyError(key));
value = ErrorValue::From(NoSuchKeyError(key), arena);
}
CEL_ASSIGN_OR_RETURN(auto ok, callback(key, value));
if (!ok) {
Expand Down Expand Up @@ -639,7 +639,7 @@ absl::Status CustomMapValue::Get(
case ValueKind::kUnknown:
break;
default:
*result = ErrorValue(NoSuchKeyError(key));
*result = ErrorValue::From(NoSuchKeyError(key), arena);
break;
}
}
Expand Down Expand Up @@ -671,7 +671,7 @@ absl::StatusOr<bool> CustomMapValue::Find(
case ValueKind::kString:
break;
default:
*result = ErrorValue(InvalidMapKeyTypeError(key.kind()));
*result = ErrorValue::From(InvalidMapKeyTypeError(key.kind()), arena);
return false;
}

Expand All @@ -682,7 +682,7 @@ absl::StatusOr<bool> CustomMapValue::Find(
auto status_or_found = content.interface->Find(
key, descriptor_pool, message_factory, arena, result);
if (!status_or_found.ok()) {
*result = ErrorValue(std::move(status_or_found).status());
*result = ErrorValue::From(std::move(status_or_found).status(), arena);
return false;
}
if (!*status_or_found) {
Expand All @@ -695,7 +695,7 @@ absl::StatusOr<bool> CustomMapValue::Find(
dispatcher_->find(dispatcher_, content_, key, descriptor_pool,
message_factory, arena, result);
if (!status_or_found.ok()) {
*result = ErrorValue(std::move(status_or_found).status());
*result = ErrorValue::From(std::move(status_or_found).status(), arena);
return false;
}
if (!*status_or_found) {
Expand Down Expand Up @@ -730,7 +730,7 @@ absl::Status CustomMapValue::Has(
case ValueKind::kString:
break;
default:
*result = ErrorValue(InvalidMapKeyTypeError(key.kind()));
*result = ErrorValue::From(InvalidMapKeyTypeError(key.kind()), arena);
return absl::OkStatus();
}
if (dispatcher_ == nullptr) {
Expand All @@ -740,7 +740,7 @@ absl::Status CustomMapValue::Has(
auto status_or_has =
content.interface->Has(key, descriptor_pool, message_factory, arena);
if (!status_or_has.ok()) {
*result = ErrorValue(std::move(status_or_has).status());
*result = ErrorValue::From(std::move(status_or_has).status(), arena);
return absl::OkStatus();
}
*result = BoolValue(*status_or_has);
Expand All @@ -749,7 +749,7 @@ absl::Status CustomMapValue::Has(
auto status_or_has = dispatcher_->has(
dispatcher_, content_, key, descriptor_pool, message_factory, arena);
if (!status_or_has.ok()) {
*result = ErrorValue(std::move(status_or_has).status());
*result = ErrorValue::From(std::move(status_or_has).status(), arena);
return absl::OkStatus();
}
*result = BoolValue(*status_or_has);
Expand Down Expand Up @@ -814,7 +814,7 @@ absl::Status CustomMapValue::ForEach(
dispatcher_->find(dispatcher_, content_, key, descriptor_pool,
message_factory, arena, &value));
if (!found) {
value = ErrorValue(NoSuchKeyError(key));
value = ErrorValue::From(NoSuchKeyError(key), arena);
}
CEL_ASSIGN_OR_RETURN(auto ok, callback(key, value));
if (!ok) {
Expand Down
6 changes: 6 additions & 0 deletions common/values/error_value.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ class ABSL_ATTRIBUTE_TRIVIAL_ABI ErrorValue final
arena, google::protobuf::Arena::Create<absl::Status>(arena, std::move(value)));
}

[[nodiscard]]
static ErrorValue WrapUnsafe(const absl::Status* absl_nonnull value) {
ABSL_DCHECK(!value->ok()) << "ErrorValue requires a non-OK absl::Status";
return ErrorValue(nullptr, value);
}

ABSL_DEPRECATED("Use From")
explicit ErrorValue(absl::Status value)
: arena_(nullptr), status_ptr_(nullptr) {
Expand Down
Loading
Loading