From c1e3026841f93b01774264e2c7171d7e55549119 Mon Sep 17 00:00:00 2001 From: Jonathan Tatum Date: Mon, 20 Apr 2026 13:33:49 -0700 Subject: [PATCH] Make error handling in wrapped legacy map consistent with modern version. The wrapped legacy implementation would use absl status return to propagate expected errors leading to inconsistencies in error handling depending on whether the underlying map was backed by a modern or legacy map. Update so that expected errors are cel::ErrorValue and unexpected errors (internal bug or client error) are returned as non-ok Status return value. Add Documentation for MapValue members. PiperOrigin-RevId: 902798507 --- common/legacy_value.cc | 17 ++-- common/values/custom_map_value.h | 16 ++-- common/values/legacy_map_value.h | 12 +-- common/values/map_value.h | 55 ++++++++---- common/values/null_value.h | 3 +- common/values/parsed_json_map_value.h | 12 +-- common/values/parsed_map_field_value.h | 12 +-- common/values/values.h | 1 - eval/eval/equality_steps.cc | 32 +++---- eval/public/builtin_func_test.cc | 3 +- eval/public/cel_type_registry.h | 6 -- eval/public/structs/field_access_impl.cc | 6 +- .../proto_message_type_adapter_test.cc | 4 +- .../container_membership_functions.cc | 90 ++++++++++--------- 14 files changed, 142 insertions(+), 127 deletions(-) diff --git a/common/legacy_value.cc b/common/legacy_value.cc index 5c81fdacb..7fbf16732 100644 --- a/common/legacy_value.cc +++ b/common/legacy_value.cc @@ -700,7 +700,8 @@ absl::Status LegacyMapValue::Get( case ValueKind::kString: break; default: - return InvalidMapKeyTypeError(key.kind()); + *result = ErrorValue(InvalidMapKeyTypeError(key.kind())); + return absl::OkStatus(); } CEL_ASSIGN_OR_RETURN(auto cel_key, LegacyValue(arena, key)); auto cel_value = impl_->Get(arena, cel_key); @@ -732,7 +733,7 @@ absl::StatusOr LegacyMapValue::Find( case ValueKind::kString: break; default: - return InvalidMapKeyTypeError(key.kind()); + *result = ErrorValue(InvalidMapKeyTypeError(key.kind())); } CEL_ASSIGN_OR_RETURN(auto cel_key, LegacyValue(arena, key)); auto cel_value = impl_->Get(arena, cel_key); @@ -764,11 +765,17 @@ absl::Status LegacyMapValue::Has( case ValueKind::kString: break; default: - return InvalidMapKeyTypeError(key.kind()); + *result = ErrorValue(InvalidMapKeyTypeError(key.kind())); + return absl::OkStatus(); } CEL_ASSIGN_OR_RETURN(auto cel_key, LegacyValue(arena, key)); - CEL_ASSIGN_OR_RETURN(auto has, impl_->Has(cel_key)); - *result = BoolValue{has}; + absl::StatusOr has = impl_->Has(cel_key); + if (!has.ok()) { + *result = ErrorValue(std::move(has).status()); + return absl::OkStatus(); + } + + *result = BoolValue(*has); return absl::OkStatus(); } diff --git a/common/values/custom_map_value.h b/common/values/custom_map_value.h index 9e840e07f..ca6e1e025 100644 --- a/common/values/custom_map_value.h +++ b/common/values/custom_map_value.h @@ -225,7 +225,7 @@ class CustomMapValueInterface { // Returns the number of entries in this map. virtual size_t Size() const = 0; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. virtual absl::Status ListKeys( const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, @@ -233,7 +233,7 @@ class CustomMapValueInterface { google::protobuf::Arena* absl_nonnull arena, ListValue* absl_nonnull result) const = 0; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. virtual absl::Status ForEach( ForEachCallback callback, @@ -347,7 +347,7 @@ class CustomMapValue final size_t Size() const; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::Status Get(const Value& key, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, @@ -356,7 +356,7 @@ class CustomMapValue final Value* absl_nonnull result) const; using MapValueMixin::Get; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::StatusOr Find( const Value& key, @@ -365,7 +365,7 @@ class CustomMapValue final google::protobuf::Arena* absl_nonnull arena, Value* absl_nonnull result) const; using MapValueMixin::Find; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::Status Has(const Value& key, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, @@ -374,7 +374,7 @@ class CustomMapValue final Value* absl_nonnull result) const; using MapValueMixin::Has; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::Status ListKeys( const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, @@ -386,7 +386,7 @@ class CustomMapValue final // documentation. using ForEachCallback = typename CustomMapValueInterface::ForEachCallback; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::Status ForEach( ForEachCallback callback, @@ -394,7 +394,7 @@ class CustomMapValue final google::protobuf::MessageFactory* absl_nonnull message_factory, google::protobuf::Arena* absl_nonnull arena) const; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::StatusOr NewIterator() const; diff --git a/common/values/legacy_map_value.h b/common/values/legacy_map_value.h index 31865a873..c83b7fc2f 100644 --- a/common/values/legacy_map_value.h +++ b/common/values/legacy_map_value.h @@ -102,7 +102,7 @@ class LegacyMapValue final size_t Size() const; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::Status Get(const Value& key, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, @@ -111,7 +111,7 @@ class LegacyMapValue final Value* absl_nonnull result) const; using MapValueMixin::Get; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::StatusOr Find( const Value& key, @@ -120,7 +120,7 @@ class LegacyMapValue final google::protobuf::Arena* absl_nonnull arena, Value* absl_nonnull result) const; using MapValueMixin::Find; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::Status Has(const Value& key, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, @@ -129,7 +129,7 @@ class LegacyMapValue final Value* absl_nonnull result) const; using MapValueMixin::Has; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::Status ListKeys( const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, @@ -137,11 +137,11 @@ class LegacyMapValue final google::protobuf::Arena* absl_nonnull arena, ListValue* absl_nonnull result) const; using MapValueMixin::ListKeys; - // See the corresponding type declaration of `MapValueInterface` for + // See the corresponding type declaration of `MapValue` for // documentation. using ForEachCallback = typename CustomMapValueInterface::ForEachCallback; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::Status ForEach( ForEachCallback callback, diff --git a/common/values/map_value.h b/common/values/map_value.h index ffbdea6c9..b6e69ea57 100644 --- a/common/values/map_value.h +++ b/common/values/map_value.h @@ -15,10 +15,16 @@ // IWYU pragma: private, include "common/value.h" // IWYU pragma: friend "common/value.h" -// `MapValue` represents values of the primitive `map` type. `MapValueView` -// is a non-owning view of `MapValue`. `MapValueInterface` is the abstract -// base class of implementations. `MapValue` and `MapValueView` act as smart -// pointers to `MapValueInterface`. +// `MapValue` represents values of the primitive `map` type. It provides a +// unified interface for accessing map contents, regardless of the underlying +// implementation (e.g., JSON, protobuf map field, or custom implementation). +// +// Public member functions: +// - `IsEmpty()` / `Size()`: Query map size. +// - `Get()` / `Find()` / `Has()`: Access entries by key. +// - `ListKeys()` / `NewIterator()` / `ForEach()`: Iterate over entries. +// - `ConvertToJson()` / `ConvertToJsonObject()`: JSON conversion. +// - `IsCustom()` / `AsCustom()` / `GetCustom()`: Access custom implementation. #ifndef THIRD_PARTY_CEL_CPP_COMMON_VALUES_MAP_VALUE_H_ #define THIRD_PARTY_CEL_CPP_COMMON_VALUES_MAP_VALUE_H_ @@ -54,7 +60,6 @@ namespace cel { -class MapValueInterface; class MapValue; class Value; @@ -119,8 +124,13 @@ class MapValue final : private common_internal::MapValueMixin { absl::StatusOr Size() const; - // See the corresponding member function of `MapValueInterface` for - // documentation. + // `Get` sets the value `result` to (via `result`) the value associated with + // `key`. If `key` is not found, `no such key` is set to `result`. If an error + // occurs (e.g., invalid key type), an `no such key` is returned. + // + // A non-ok status may be returned if an unexpected error is encountered or to + // propagate an error from a custom implementation, in which case `result` is + // unspecified. absl::Status Get(const Value& key, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, google::protobuf::MessageFactory* absl_nonnull message_factory, @@ -128,8 +138,13 @@ class MapValue final : private common_internal::MapValueMixin { Value* absl_nonnull result) const; using MapValueMixin::Get; - // See the corresponding member function of `MapValueInterface` for - // documentation. + // `Find` returns `true` if `key` is found in the map, and stores the + // associated value in `result`. If `key` is not found, `false` is returned + // and `result` is unchanged. + // + // A non-ok status may be returned if an unexpected error is encountered or to + // propagate an error from a custom implementation, in which case `result` is + // unspecified. absl::StatusOr Find( const Value& key, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, @@ -137,8 +152,13 @@ class MapValue final : private common_internal::MapValueMixin { google::protobuf::Arena* absl_nonnull arena, Value* absl_nonnull result) const; using MapValueMixin::Find; - // See the corresponding member function of `MapValueInterface` for - // documentation. + // `Has` returns `true` if `key` is found in the map, and stores the BoolValue + // result in `result`. In case of an error, the result is set to an + // ErrorValue. + // + // A non-ok status may be returned if an unexpected error is encountered or to + // propagate an error from a custom implementation, in which case `result` is + // unspecified. absl::Status Has(const Value& key, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, google::protobuf::MessageFactory* absl_nonnull message_factory, @@ -146,28 +166,25 @@ class MapValue final : private common_internal::MapValueMixin { Value* absl_nonnull result) const; using MapValueMixin::Has; - // See the corresponding member function of `MapValueInterface` for - // documentation. + // `ListKeys` returns a `ListValue` containing all keys in the map. absl::Status ListKeys( const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, google::protobuf::MessageFactory* absl_nonnull message_factory, google::protobuf::Arena* absl_nonnull arena, ListValue* absl_nonnull result) const; using MapValueMixin::ListKeys; - // See the corresponding type declaration of `MapValueInterface` for - // documentation. + // `ForEachCallback` is the callback type for `ForEach`. using ForEachCallback = typename CustomMapValueInterface::ForEachCallback; - // See the corresponding member function of `MapValueInterface` for - // documentation. + // `ForEach` calls `callback` for each entry in the map. Iteration continues + // until all entries are visited or `callback` returns an error or `false`. absl::Status ForEach( ForEachCallback callback, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, google::protobuf::MessageFactory* absl_nonnull message_factory, google::protobuf::Arena* absl_nonnull arena) const; - // See the corresponding member function of `MapValueInterface` for - // documentation. + // `NewIterator` returns a new iterator for the map. absl::StatusOr NewIterator() const; // Returns `true` if this value is an instance of a custom map value. diff --git a/common/values/null_value.h b/common/values/null_value.h index 53c3161a1..d4d05dba3 100644 --- a/common/values/null_value.h +++ b/common/values/null_value.h @@ -37,8 +37,7 @@ namespace cel { class Value; class NullValue; -// `NullValue` represents values of the primitive `duration` type. - +// `NullValue` represents the CEL `null` value. class NullValue final : private common_internal::ValueMixin { public: static constexpr ValueKind kKind = ValueKind::kNull; diff --git a/common/values/parsed_json_map_value.h b/common/values/parsed_json_map_value.h index b20fe032b..ba8d3490d 100644 --- a/common/values/parsed_json_map_value.h +++ b/common/values/parsed_json_map_value.h @@ -132,7 +132,7 @@ class ParsedJsonMapValue final size_t Size() const; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::Status Get(const Value& key, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, @@ -141,7 +141,7 @@ class ParsedJsonMapValue final Value* absl_nonnull result) const; using MapValueMixin::Get; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::StatusOr Find( const Value& key, @@ -150,7 +150,7 @@ class ParsedJsonMapValue final google::protobuf::Arena* absl_nonnull arena, Value* absl_nonnull result) const; using MapValueMixin::Find; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::Status Has(const Value& key, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, @@ -159,7 +159,7 @@ class ParsedJsonMapValue final Value* absl_nonnull result) const; using MapValueMixin::Has; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::Status ListKeys( const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, @@ -167,11 +167,11 @@ class ParsedJsonMapValue final google::protobuf::Arena* absl_nonnull arena, ListValue* absl_nonnull result) const; using MapValueMixin::ListKeys; - // See the corresponding type declaration of `MapValueInterface` for + // See the corresponding type declaration of `MapValue` for // documentation. using ForEachCallback = typename CustomMapValueInterface::ForEachCallback; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::Status ForEach( ForEachCallback callback, diff --git a/common/values/parsed_map_field_value.h b/common/values/parsed_map_field_value.h index 3478f75bc..21d686bfd 100644 --- a/common/values/parsed_map_field_value.h +++ b/common/values/parsed_map_field_value.h @@ -117,7 +117,7 @@ class ParsedMapFieldValue final size_t Size() const; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::Status Get(const Value& key, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, @@ -126,7 +126,7 @@ class ParsedMapFieldValue final Value* absl_nonnull result) const; using MapValueMixin::Get; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::StatusOr Find( const Value& key, @@ -135,7 +135,7 @@ class ParsedMapFieldValue final google::protobuf::Arena* absl_nonnull arena, Value* absl_nonnull result) const; using MapValueMixin::Find; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::Status Has(const Value& key, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, @@ -144,7 +144,7 @@ class ParsedMapFieldValue final Value* absl_nonnull result) const; using MapValueMixin::Has; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::Status ListKeys( const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, @@ -152,11 +152,11 @@ class ParsedMapFieldValue final google::protobuf::Arena* absl_nonnull arena, ListValue* absl_nonnull result) const; using MapValueMixin::ListKeys; - // See the corresponding type declaration of `MapValueInterface` for + // See the corresponding type declaration of `MapValue` for // documentation. using ForEachCallback = typename CustomMapValueInterface::ForEachCallback; - // See the corresponding member function of `MapValueInterface` for + // See the corresponding member function of `MapValue` for // documentation. absl::Status ForEach( ForEachCallback callback, diff --git a/common/values/values.h b/common/values/values.h index c9703dcbb..aaa6f8659 100644 --- a/common/values/values.h +++ b/common/values/values.h @@ -48,7 +48,6 @@ namespace cel { class ValueInterface; class ListValueInterface; -class MapValueInterface; class StructValueInterface; class Value; diff --git a/eval/eval/equality_steps.cc b/eval/eval/equality_steps.cc index e134069d5..d720302e4 100644 --- a/eval/eval/equality_steps.cc +++ b/eval/eval/equality_steps.cc @@ -132,15 +132,11 @@ class IterativeEqualityStep : public ExpressionStepBase { absl::StatusOr EvaluateInMap(ExecutionFrameBase& frame, const Value& item, const MapValue& container) { - absl::StatusOr result = {BoolValue(false)}; switch (item.kind()) { case ValueKind::kBool: case ValueKind::kString: case ValueKind::kInt: case ValueKind::kUint: - result = container.Has(item, frame.descriptor_pool(), - frame.message_factory(), frame.arena()); - break; case ValueKind::kDouble: break; default: @@ -148,9 +144,12 @@ absl::StatusOr EvaluateInMap(ExecutionFrameBase& frame, cel::runtime_internal::CreateNoMatchingOverloadError( cel::builtin::kIn)); } + Value result; + CEL_RETURN_IF_ERROR(container.Has(item, frame.descriptor_pool(), + frame.message_factory(), frame.arena(), + &result)); - if (result.ok() && result.value().IsBool() && - result.value().GetBool().NativeValue()) { + if (result.IsTrue()) { return result; } @@ -159,10 +158,10 @@ absl::StatusOr EvaluateInMap(ExecutionFrameBase& frame, ? Number::FromDouble(item.GetDouble().NativeValue()) : Number::FromUint64(item.GetUint().NativeValue()); if (number.LosslessConvertibleToInt()) { - result = container.Has(IntValue(number.AsInt()), frame.descriptor_pool(), - frame.message_factory(), frame.arena()); - if (result.ok() && result.value().IsBool() && - result.value().GetBool().NativeValue()) { + CEL_RETURN_IF_ERROR( + container.Has(IntValue(number.AsInt()), frame.descriptor_pool(), + frame.message_factory(), frame.arena(), &result)); + if (result.IsTrue()) { return result; } } @@ -173,21 +172,16 @@ absl::StatusOr EvaluateInMap(ExecutionFrameBase& frame, ? Number::FromDouble(item.GetDouble().NativeValue()) : Number::FromInt64(item.GetInt().NativeValue()); if (number.LosslessConvertibleToUint()) { - result = + CEL_RETURN_IF_ERROR( container.Has(UintValue(number.AsUint()), frame.descriptor_pool(), - frame.message_factory(), frame.arena()); - if (result.ok() && result.value().IsBool() && - result.value().GetBool().NativeValue()) { + frame.message_factory(), frame.arena(), &result)); + if (result.IsTrue()) { return result; } } } - if (!result.ok()) { - return BoolValue(false); - } - - return result; + return BoolValue(false); } absl::StatusOr EvaluateIn(ExecutionFrameBase& frame, const Value& item, diff --git a/eval/public/builtin_func_test.cc b/eval/public/builtin_func_test.cc index 037fa8345..b73a2dc55 100644 --- a/eval/public/builtin_func_test.cc +++ b/eval/public/builtin_func_test.cc @@ -1632,7 +1632,8 @@ TEST_F(BuiltinsTest, TestMapInError) { CelValue result_value; ASSERT_NO_FATAL_FAILURE(PerformRun( builtin::kIn, {}, {key, CelValue::CreateMap(&cel_map)}, &result_value)); - EXPECT_TRUE(result_value.IsBool()); + ASSERT_TRUE(result_value.IsBool()) + << key.DebugString() << " : " << result_value.DebugString(); EXPECT_FALSE(result_value.BoolOrDie()); } diff --git a/eval/public/cel_type_registry.h b/eval/public/cel_type_registry.h index 0c01eb8e9..3fb80bcea 100644 --- a/eval/public/cel_type_registry.h +++ b/eval/public/cel_type_registry.h @@ -28,7 +28,6 @@ #include "base/type_provider.h" #include "eval/public/structs/legacy_type_adapter.h" #include "eval/public/structs/legacy_type_provider.h" -#include "eval/public/structs/protobuf_descriptor_type_provider.h" #include "runtime/type_registry.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/message.h" @@ -139,11 +138,6 @@ class CelTypeRegistry { private: // Internal modern registry. cel::TypeRegistry modern_type_registry_; - - // TODO(uncreated-issue/44): This is needed to inspect the registered legacy type - // providers for client tests. This can be removed when they are migrated to - // use the modern APIs. - std::shared_ptr legacy_type_provider_; }; } // namespace google::api::expr::runtime diff --git a/eval/public/structs/field_access_impl.cc b/eval/public/structs/field_access_impl.cc index 3b3cb9847..2bd9fff9d 100644 --- a/eval/public/structs/field_access_impl.cc +++ b/eval/public/structs/field_access_impl.cc @@ -139,8 +139,7 @@ class FieldAccessor { case FieldDescriptor::TYPE_BYTES: return CelValue::CreateBytesView(value); default: - return absl::Status(absl::StatusCode::kInvalidArgument, - "Error handling C++ string conversion"); + break; } break; } @@ -153,8 +152,7 @@ class FieldAccessor { return CelValue::CreateInt64(enum_value); } default: - return absl::Status(absl::StatusCode::kInvalidArgument, - "Unhandled C++ type conversion"); + break; } return absl::Status(absl::StatusCode::kInvalidArgument, "Unhandled C++ type conversion"); diff --git a/eval/public/structs/proto_message_type_adapter_test.cc b/eval/public/structs/proto_message_type_adapter_test.cc index 088d20d48..32608bc3f 100644 --- a/eval/public/structs/proto_message_type_adapter_test.cc +++ b/eval/public/structs/proto_message_type_adapter_test.cc @@ -69,8 +69,8 @@ class ProtoMessageTypeAccessorTest : public testing::TestWithParam { bool use_generic_instance = GetParam(); if (use_generic_instance) { // implementation detail: in general, type info implementations may - // return a different accessor object based on the messsage instance, but - // this implemenation returns the same one no matter the message. + // return a different accessor object based on the message instance, but + // this implementation returns the same one no matter the message. return *GetGenericProtoTypeInfoInstance().GetAccessApis(dummy_); } else { diff --git a/runtime/standard/container_membership_functions.cc b/runtime/standard/container_membership_functions.cc index 9f5ca3755..cc0638429 100644 --- a/runtime/standard/container_membership_functions.cc +++ b/runtime/standard/container_membership_functions.cc @@ -174,15 +174,16 @@ absl::Status RegisterMapMembershipFunctions(FunctionRegistry& registry, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, google::protobuf::MessageFactory* absl_nonnull message_factory, google::protobuf::Arena* absl_nonnull arena) -> absl::StatusOr { - auto result = - map_value.Has(BoolValue(key), descriptor_pool, message_factory, arena); - if (result.ok()) { - return std::move(*result); + Value has; + CEL_RETURN_IF_ERROR(map_value.Has(BoolValue(key), descriptor_pool, + message_factory, arena, &has)); + if (has.IsTrue()) { + return has; } if (enable_heterogeneous_equality) { return BoolValue(false); } - return ErrorValue(result.status()); + return has; }; auto intKeyInSet = @@ -191,27 +192,26 @@ absl::Status RegisterMapMembershipFunctions(FunctionRegistry& registry, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, google::protobuf::MessageFactory* absl_nonnull message_factory, google::protobuf::Arena* absl_nonnull arena) -> absl::StatusOr { - auto result = - map_value.Has(IntValue(key), descriptor_pool, message_factory, arena); + Value result; + CEL_RETURN_IF_ERROR(map_value.Has(IntValue(key), descriptor_pool, + message_factory, arena, &result)); if (enable_heterogeneous_equality) { - if (result.ok() && result->IsTrue()) { - return std::move(*result); + if (result.IsTrue()) { + return result; } Number number = Number::FromInt64(key); if (number.LosslessConvertibleToUint()) { - const auto& result = - map_value.Has(UintValue(number.AsUint()), descriptor_pool, - message_factory, arena); - if (result.ok() && result->IsTrue()) { - return std::move(*result); + Value result_alt; + CEL_RETURN_IF_ERROR(map_value.Has(UintValue(number.AsUint()), + descriptor_pool, message_factory, + arena, &result_alt)); + if (result_alt.IsTrue()) { + return result_alt; } } return BoolValue(false); } - if (!result.ok()) { - return ErrorValue(result.status()); - } - return std::move(*result); + return result; }; auto stringKeyInSet = @@ -220,14 +220,16 @@ absl::Status RegisterMapMembershipFunctions(FunctionRegistry& registry, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, google::protobuf::MessageFactory* absl_nonnull message_factory, google::protobuf::Arena* absl_nonnull arena) -> absl::StatusOr { - auto result = map_value.Has(key, descriptor_pool, message_factory, arena); - if (result.ok()) { - return std::move(*result); + Value result; + CEL_RETURN_IF_ERROR( + map_value.Has(key, descriptor_pool, message_factory, arena, &result)); + if (result.IsBool()) { + return result; } if (enable_heterogeneous_equality) { return BoolValue(false); } - return ErrorValue(result.status()); + return result; }; auto uintKeyInSet = @@ -236,26 +238,26 @@ absl::Status RegisterMapMembershipFunctions(FunctionRegistry& registry, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, google::protobuf::MessageFactory* absl_nonnull message_factory, google::protobuf::Arena* absl_nonnull arena) -> absl::StatusOr { - const auto& result = - map_value.Has(UintValue(key), descriptor_pool, message_factory, arena); + Value has; + CEL_RETURN_IF_ERROR(map_value.Has(UintValue(key), descriptor_pool, + message_factory, arena, &has)); if (enable_heterogeneous_equality) { - if (result.ok() && result->IsTrue()) { - return std::move(*result); + if (has.IsTrue()) { + return has; } + Value has_alt; Number number = Number::FromUint64(key); if (number.LosslessConvertibleToInt()) { - const auto& result = map_value.Has( - IntValue(number.AsInt()), descriptor_pool, message_factory, arena); - if (result.ok() && result->IsTrue()) { - return std::move(*result); + CEL_RETURN_IF_ERROR(map_value.Has(IntValue(number.AsInt()), + descriptor_pool, message_factory, + arena, &has_alt)); + if (has.IsTrue()) { + return has; } } return BoolValue(false); } - if (!result.ok()) { - return ErrorValue(result.status()); - } - return std::move(*result); + return has; }; auto doubleKeyInSet = @@ -265,17 +267,21 @@ absl::Status RegisterMapMembershipFunctions(FunctionRegistry& registry, google::protobuf::Arena* absl_nonnull arena) -> absl::StatusOr { Number number = Number::FromDouble(key); if (number.LosslessConvertibleToInt()) { - const auto& result = map_value.Has( - IntValue(number.AsInt()), descriptor_pool, message_factory, arena); - if (result.ok() && result->IsTrue()) { - return std::move(*result); + Value has; + CEL_RETURN_IF_ERROR(map_value.Has(IntValue(number.AsInt()), + descriptor_pool, message_factory, arena, + &has)); + if (has.IsTrue()) { + return has; } } if (number.LosslessConvertibleToUint()) { - const auto& result = map_value.Has( - UintValue(number.AsUint()), descriptor_pool, message_factory, arena); - if (result.ok() && result->IsTrue()) { - return std::move(*result); + Value has; + CEL_RETURN_IF_ERROR(map_value.Has(UintValue(number.AsUint()), + descriptor_pool, message_factory, arena, + &has)); + if (has.IsTrue()) { + return has; } } return BoolValue(false);