diff --git a/common/values/legacy_list_value.h b/common/values/legacy_list_value.h index 71580574c..f804514b7 100644 --- a/common/values/legacy_list_value.h +++ b/common/values/legacy_list_value.h @@ -93,9 +93,6 @@ class LegacyListValue final absl::Nonnull arena, absl::Nonnull result) const; using ListValueMixin::Equal; - absl::Status Contains(ValueManager& value_manager, const Value& other, - Value& result) const; - bool IsZeroValue() const { return IsEmpty(); } bool IsEmpty() const; diff --git a/eval/compiler/BUILD b/eval/compiler/BUILD index e12417794..641aa22c7 100644 --- a/eval/compiler/BUILD +++ b/eval/compiler/BUILD @@ -617,6 +617,7 @@ cc_test( "//runtime:type_registry", "//runtime/internal:runtime_env", "//runtime/internal:runtime_env_testing", + "//runtime/internal:runtime_value_manager", "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/status", diff --git a/eval/compiler/instrumentation_test.cc b/eval/compiler/instrumentation_test.cc index 78b2ba59b..b97736e80 100644 --- a/eval/compiler/instrumentation_test.cc +++ b/eval/compiler/instrumentation_test.cc @@ -37,6 +37,7 @@ #include "runtime/function_registry.h" #include "runtime/internal/runtime_env.h" #include "runtime/internal/runtime_env_testing.h" +#include "runtime/internal/runtime_value_manager.h" #include "runtime/managed_value_factory.h" #include "runtime/runtime_options.h" #include "runtime/standard_functions.h" @@ -62,9 +63,8 @@ class InstrumentationTest : public ::testing::Test { : env_(NewTestingRuntimeEnv()), function_registry_(env_->function_registry), type_registry_(env_->type_registry), - managed_value_factory_( - type_registry_.GetComposedTypeProvider(), - cel::extensions::ProtoMemoryManagerRef(&arena_)) {} + value_manager_(&arena_, env_->descriptor_pool.get(), + env_->MutableMessageFactory()) {} void SetUp() override { ASSERT_OK(cel::RegisterStandardFunctions(function_registry_, options_)); } @@ -75,7 +75,7 @@ class InstrumentationTest : public ::testing::Test { cel::FunctionRegistry& function_registry_; cel::TypeRegistry& type_registry_; google::protobuf::Arena arena_; - cel::ManagedValueFactory managed_value_factory_; + cel::runtime_internal::RuntimeValueManager value_manager_; }; MATCHER_P(IsIntValue, expected, "") { @@ -106,7 +106,7 @@ TEST_F(InstrumentationTest, Basic) { builder.CreateExpressionImpl(std::move(ast), /*issues=*/nullptr)); - auto state = plan.MakeEvaluatorState(managed_value_factory_.get()); + auto state = plan.MakeEvaluatorState(value_manager_); cel::Activation activation; ASSERT_OK_AND_ASSIGN( @@ -152,7 +152,7 @@ TEST_F(InstrumentationTest, BasicWithConstFolding) { Pair(2, IsIntValue(3)), Pair(5, IsIntValue(3)))); expr_id_to_value.clear(); - auto state = plan.MakeEvaluatorState(managed_value_factory_.get()); + auto state = plan.MakeEvaluatorState(value_manager_); cel::Activation activation; ASSERT_OK_AND_ASSIGN( @@ -190,13 +190,11 @@ TEST_F(InstrumentationTest, AndShortCircuit) { builder.CreateExpressionImpl(std::move(ast), /*issues=*/nullptr)); - auto state = plan.MakeEvaluatorState(managed_value_factory_.get()); + auto state = plan.MakeEvaluatorState(value_manager_); cel::Activation activation; - activation.InsertOrAssignValue( - "a", managed_value_factory_.get().CreateBoolValue(true)); - activation.InsertOrAssignValue( - "b", managed_value_factory_.get().CreateBoolValue(false)); + activation.InsertOrAssignValue("a", value_manager_.CreateBoolValue(true)); + activation.InsertOrAssignValue("b", value_manager_.CreateBoolValue(false)); ASSERT_OK_AND_ASSIGN( auto value, @@ -204,8 +202,7 @@ TEST_F(InstrumentationTest, AndShortCircuit) { EXPECT_THAT(expr_ids, ElementsAre(1, 2, 3)); - activation.InsertOrAssignValue( - "a", managed_value_factory_.get().CreateBoolValue(false)); + activation.InsertOrAssignValue("a", value_manager_.CreateBoolValue(false)); ASSERT_OK_AND_ASSIGN(value, plan.EvaluateWithCallback( activation, EvaluationListener(), state)); @@ -235,13 +232,11 @@ TEST_F(InstrumentationTest, OrShortCircuit) { builder.CreateExpressionImpl(std::move(ast), /*issues=*/nullptr)); - auto state = plan.MakeEvaluatorState(managed_value_factory_.get()); + auto state = plan.MakeEvaluatorState(value_manager_); cel::Activation activation; - activation.InsertOrAssignValue( - "a", managed_value_factory_.get().CreateBoolValue(false)); - activation.InsertOrAssignValue( - "b", managed_value_factory_.get().CreateBoolValue(true)); + activation.InsertOrAssignValue("a", value_manager_.CreateBoolValue(false)); + activation.InsertOrAssignValue("b", value_manager_.CreateBoolValue(true)); ASSERT_OK_AND_ASSIGN( auto value, @@ -249,8 +244,7 @@ TEST_F(InstrumentationTest, OrShortCircuit) { EXPECT_THAT(expr_ids, ElementsAre(1, 2, 3)); expr_ids.clear(); - activation.InsertOrAssignValue( - "a", managed_value_factory_.get().CreateBoolValue(true)); + activation.InsertOrAssignValue("a", value_manager_.CreateBoolValue(true)); ASSERT_OK_AND_ASSIGN(value, plan.EvaluateWithCallback( activation, EvaluationListener(), state)); @@ -280,15 +274,12 @@ TEST_F(InstrumentationTest, Ternary) { builder.CreateExpressionImpl(std::move(ast), /*issues=*/nullptr)); - auto state = plan.MakeEvaluatorState(managed_value_factory_.get()); + auto state = plan.MakeEvaluatorState(value_manager_); cel::Activation activation; - activation.InsertOrAssignValue( - "c", managed_value_factory_.get().CreateBoolValue(true)); - activation.InsertOrAssignValue( - "a", managed_value_factory_.get().CreateIntValue(1)); - activation.InsertOrAssignValue( - "b", managed_value_factory_.get().CreateIntValue(2)); + activation.InsertOrAssignValue("c", value_manager_.CreateBoolValue(true)); + activation.InsertOrAssignValue("a", value_manager_.CreateIntValue(1)); + activation.InsertOrAssignValue("b", value_manager_.CreateIntValue(2)); ASSERT_OK_AND_ASSIGN( auto value, @@ -301,8 +292,7 @@ TEST_F(InstrumentationTest, Ternary) { EXPECT_THAT(expr_ids, ElementsAre(1, 3, 2)); expr_ids.clear(); - activation.InsertOrAssignValue( - "c", managed_value_factory_.get().CreateBoolValue(false)); + activation.InsertOrAssignValue("c", value_manager_.CreateBoolValue(false)); ASSERT_OK_AND_ASSIGN(value, plan.EvaluateWithCallback( activation, EvaluationListener(), state)); @@ -336,7 +326,7 @@ TEST_F(InstrumentationTest, OptimizedStepsNotEvaluated) { builder.CreateExpressionImpl(std::move(ast), /*issues=*/nullptr)); - auto state = plan.MakeEvaluatorState(managed_value_factory_.get()); + auto state = plan.MakeEvaluatorState(value_manager_); cel::Activation activation; ASSERT_OK_AND_ASSIGN( @@ -362,15 +352,12 @@ TEST_F(InstrumentationTest, NoopSkipped) { builder.CreateExpressionImpl(std::move(ast), /*issues=*/nullptr)); - auto state = plan.MakeEvaluatorState(managed_value_factory_.get()); + auto state = plan.MakeEvaluatorState(value_manager_); cel::Activation activation; - activation.InsertOrAssignValue( - "c", managed_value_factory_.get().CreateBoolValue(true)); - activation.InsertOrAssignValue( - "a", managed_value_factory_.get().CreateIntValue(1)); - activation.InsertOrAssignValue( - "b", managed_value_factory_.get().CreateIntValue(2)); + activation.InsertOrAssignValue("c", value_manager_.CreateBoolValue(true)); + activation.InsertOrAssignValue("a", value_manager_.CreateIntValue(1)); + activation.InsertOrAssignValue("b", value_manager_.CreateIntValue(2)); ASSERT_OK_AND_ASSIGN( auto value, diff --git a/eval/eval/BUILD b/eval/eval/BUILD index 48f6e09b0..9006c7313 100644 --- a/eval/eval/BUILD +++ b/eval/eval/BUILD @@ -713,11 +713,14 @@ cc_test( "//eval/public:activation", "//eval/public:cel_attribute", "//internal:testing", + "//internal:testing_descriptor_pool", + "//internal:testing_message_factory", "//runtime:activation", - "//runtime:managed_value_factory", "//runtime:runtime_options", "//runtime/internal:runtime_env_testing", + "//runtime/internal:runtime_value_manager", "@com_google_absl//absl/status", + "@com_google_protobuf//:protobuf", ], ) diff --git a/eval/eval/ident_step.cc b/eval/eval/ident_step.cc index 168d80ecd..4a4f57830 100644 --- a/eval/eval/ident_step.cc +++ b/eval/eval/ident_step.cc @@ -58,8 +58,10 @@ absl::Status LookupIdent(const std::string& name, ExecutionFrameBase& frame, } } - CEL_ASSIGN_OR_RETURN(auto found, frame.activation().FindVariable( - frame.value_manager(), name, result)); + CEL_ASSIGN_OR_RETURN( + auto found, frame.activation().FindVariable(name, frame.descriptor_pool(), + frame.message_factory(), + frame.arena(), &result)); if (found) { return absl::OkStatus(); diff --git a/eval/eval/ident_step_test.cc b/eval/eval/ident_step_test.cc index 042adc9d8..4e8055aa2 100644 --- a/eval/eval/ident_step_test.cc +++ b/eval/eval/ident_step_test.cc @@ -16,10 +16,13 @@ #include "eval/public/activation.h" #include "eval/public/cel_attribute.h" #include "internal/testing.h" +#include "internal/testing_descriptor_pool.h" +#include "internal/testing_message_factory.h" #include "runtime/activation.h" #include "runtime/internal/runtime_env_testing.h" -#include "runtime/managed_value_factory.h" +#include "runtime/internal/runtime_value_manager.h" #include "runtime/runtime_options.h" +#include "google/protobuf/arena.h" namespace google::api::expr::runtime { @@ -30,7 +33,6 @@ using ::cel::Cast; using ::cel::ErrorValue; using ::cel::InstanceOf; using ::cel::IntValue; -using ::cel::ManagedValueFactory; using ::cel::MemoryManagerRef; using ::cel::RuntimeOptions; using ::cel::TypeProvider; @@ -233,14 +235,16 @@ TEST(IdentStepTest, TestIdentStepUnknownAttribute) { } TEST(DirectIdentStepTest, Basic) { - ManagedValueFactory value_factory(TypeProvider::Builtin(), - MemoryManagerRef::ReferenceCounting()); + google::protobuf::Arena arena; + cel::runtime_internal::RuntimeValueManager value_factory( + &arena, cel::internal::GetTestingDescriptorPool(), + cel::internal::GetTestingMessageFactory()); cel::Activation activation; RuntimeOptions options; activation.InsertOrAssignValue("var1", IntValue(42)); - ExecutionFrameBase frame(activation, options, value_factory.get()); + ExecutionFrameBase frame(activation, options, value_factory); Value result; AttributeTrail trail; @@ -253,8 +257,10 @@ TEST(DirectIdentStepTest, Basic) { } TEST(DirectIdentStepTest, UnknownAttribute) { - ManagedValueFactory value_factory(TypeProvider::Builtin(), - MemoryManagerRef::ReferenceCounting()); + google::protobuf::Arena arena; + cel::runtime_internal::RuntimeValueManager value_factory( + &arena, cel::internal::GetTestingDescriptorPool(), + cel::internal::GetTestingMessageFactory()); cel::Activation activation; RuntimeOptions options; options.unknown_processing = cel::UnknownProcessingOptions::kAttributeOnly; @@ -262,7 +268,7 @@ TEST(DirectIdentStepTest, UnknownAttribute) { activation.InsertOrAssignValue("var1", IntValue(42)); activation.SetUnknownPatterns({CreateCelAttributePattern("var1", {})}); - ExecutionFrameBase frame(activation, options, value_factory.get()); + ExecutionFrameBase frame(activation, options, value_factory); Value result; AttributeTrail trail; @@ -275,8 +281,10 @@ TEST(DirectIdentStepTest, UnknownAttribute) { } TEST(DirectIdentStepTest, MissingAttribute) { - ManagedValueFactory value_factory(TypeProvider::Builtin(), - MemoryManagerRef::ReferenceCounting()); + google::protobuf::Arena arena; + cel::runtime_internal::RuntimeValueManager value_factory( + &arena, cel::internal::GetTestingDescriptorPool(), + cel::internal::GetTestingMessageFactory()); cel::Activation activation; RuntimeOptions options; options.enable_missing_attribute_errors = true; @@ -284,7 +292,7 @@ TEST(DirectIdentStepTest, MissingAttribute) { activation.InsertOrAssignValue("var1", IntValue(42)); activation.SetMissingPatterns({CreateCelAttributePattern("var1", {})}); - ExecutionFrameBase frame(activation, options, value_factory.get()); + ExecutionFrameBase frame(activation, options, value_factory); Value result; AttributeTrail trail; @@ -298,12 +306,14 @@ TEST(DirectIdentStepTest, MissingAttribute) { } TEST(DirectIdentStepTest, NotFound) { - ManagedValueFactory value_factory(TypeProvider::Builtin(), - MemoryManagerRef::ReferenceCounting()); + google::protobuf::Arena arena; + cel::runtime_internal::RuntimeValueManager value_factory( + &arena, cel::internal::GetTestingDescriptorPool(), + cel::internal::GetTestingMessageFactory()); cel::Activation activation; RuntimeOptions options; - ExecutionFrameBase frame(activation, options, value_factory.get()); + ExecutionFrameBase frame(activation, options, value_factory); Value result; AttributeTrail trail; diff --git a/eval/eval/shadowable_value_step.cc b/eval/eval/shadowable_value_step.cc index bbb49b0f0..1c91219a2 100644 --- a/eval/eval/shadowable_value_step.cc +++ b/eval/eval/shadowable_value_step.cc @@ -39,7 +39,8 @@ absl::Status ShadowableValueStep::Evaluate(ExecutionFrame* frame) const { cel::Value result; CEL_ASSIGN_OR_RETURN(auto found, frame->modern_activation().FindVariable( - frame->value_factory(), identifier_, result)); + identifier_, frame->descriptor_pool(), + frame->message_factory(), frame->arena(), &result)); if (found) { frame->value_stack().Push(std::move(result)); } else { @@ -70,8 +71,9 @@ class DirectShadowableValueStep : public DirectExpressionStep { absl::Status DirectShadowableValueStep::Evaluate( ExecutionFrameBase& frame, Value& result, AttributeTrail& attribute) const { CEL_ASSIGN_OR_RETURN(auto found, - frame.activation().FindVariable(frame.value_manager(), - identifier_, result)); + frame.activation().FindVariable( + identifier_, frame.descriptor_pool(), + frame.message_factory(), frame.arena(), &result)); if (!found) { result = value_; } diff --git a/eval/internal/BUILD b/eval/internal/BUILD index edc4d33f6..a9650902a 100644 --- a/eval/internal/BUILD +++ b/eval/internal/BUILD @@ -82,14 +82,13 @@ cc_library( deps = [ ":interop", "//base:attributes", - "//common:memory", "//common:value", "//eval/public:base_activation", "//eval/public:cel_value", - "//extensions/protobuf:memory_manager", "//internal:status_macros", "//runtime:activation_interface", "//runtime:function_overload_reference", + "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", "@com_google_absl//absl/types:optional", diff --git a/eval/internal/adapter_activation_impl.cc b/eval/internal/adapter_activation_impl.cc index 4585ac579..bc1658aca 100644 --- a/eval/internal/adapter_activation_impl.cc +++ b/eval/internal/adapter_activation_impl.cc @@ -16,33 +16,37 @@ #include +#include "absl/base/nullability.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" +#include "common/value.h" #include "eval/internal/interop.h" #include "eval/public/cel_value.h" -#include "extensions/protobuf/memory_manager.h" #include "internal/status_macros.h" #include "runtime/function_overload_reference.h" #include "google/protobuf/arena.h" +#include "google/protobuf/descriptor.h" +#include "google/protobuf/message.h" namespace cel::interop_internal { using ::google::api::expr::runtime::CelFunction; absl::StatusOr AdapterActivationImpl::FindVariable( - ValueManager& value_factory, absl::string_view name, Value& result) const { + absl::string_view name, + absl::Nonnull descriptor_pool, + absl::Nonnull message_factory, + absl::Nonnull arena, absl::Nonnull result) const { // This implementation should only be used during interop, when we can // always assume the memory manager is backed by a protobuf arena. - google::protobuf::Arena* arena = - extensions::ProtoMemoryManagerArena(value_factory.GetMemoryManager()); absl::optional legacy_value = legacy_activation_.FindValue(name, arena); if (!legacy_value.has_value()) { return false; } - CEL_RETURN_IF_ERROR(ModernValue(arena, *legacy_value, result)); + CEL_RETURN_IF_ERROR(ModernValue(arena, *legacy_value, *result)); return true; } diff --git a/eval/internal/adapter_activation_impl.h b/eval/internal/adapter_activation_impl.h index ca72393e6..2fd0fb004 100644 --- a/eval/internal/adapter_activation_impl.h +++ b/eval/internal/adapter_activation_impl.h @@ -17,6 +17,7 @@ #include +#include "absl/base/nullability.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "absl/types/span.h" @@ -26,6 +27,9 @@ #include "eval/public/base_activation.h" #include "runtime/activation_interface.h" #include "runtime/function_overload_reference.h" +#include "google/protobuf/arena.h" +#include "google/protobuf/descriptor.h" +#include "google/protobuf/message.h" namespace cel::interop_internal { @@ -38,9 +42,12 @@ class AdapterActivationImpl : public ActivationInterface { const google::api::expr::runtime::BaseActivation& legacy_activation) : legacy_activation_(legacy_activation) {} - absl::StatusOr FindVariable(ValueManager& value_factory, - absl::string_view name, - Value& result) const override; + absl::StatusOr FindVariable( + absl::string_view name, + absl::Nonnull descriptor_pool, + absl::Nonnull message_factory, + absl::Nonnull arena, + absl::Nonnull result) const override; std::vector FindFunctionOverloads( absl::string_view name) const override; diff --git a/extensions/protobuf/BUILD b/extensions/protobuf/BUILD index 57a23add7..e3c858d97 100644 --- a/extensions/protobuf/BUILD +++ b/extensions/protobuf/BUILD @@ -233,6 +233,7 @@ cc_library( "//common:value", "//internal:status_macros", "//runtime:activation", + "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", "@com_google_protobuf//:protobuf", diff --git a/extensions/protobuf/bind_proto_to_activation.cc b/extensions/protobuf/bind_proto_to_activation.cc index 081565b98..caaa70171 100644 --- a/extensions/protobuf/bind_proto_to_activation.cc +++ b/extensions/protobuf/bind_proto_to_activation.cc @@ -16,12 +16,15 @@ #include +#include "absl/base/nullability.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "common/value.h" #include "internal/status_macros.h" #include "runtime/activation.h" +#include "google/protobuf/arena.h" #include "google/protobuf/descriptor.h" +#include "google/protobuf/message.h" namespace cel::extensions::protobuf_internal { @@ -31,8 +34,7 @@ using ::google::protobuf::Descriptor; absl::StatusOr ShouldBindField( const google::protobuf::FieldDescriptor* field_desc, const StructValue& struct_value, - BindProtoUnsetFieldBehavior unset_field_behavior, - ValueManager& value_manager) { + BindProtoUnsetFieldBehavior unset_field_behavior) { if (unset_field_behavior == BindProtoUnsetFieldBehavior::kBindDefaultValue || field_desc->is_repeated()) { return true; @@ -40,9 +42,11 @@ absl::StatusOr ShouldBindField( return struct_value.HasFieldByNumber(field_desc->number()); } -absl::StatusOr GetFieldValue(const google::protobuf::FieldDescriptor* field_desc, - const StructValue& struct_value, - ValueManager& value_manager) { +absl::StatusOr GetFieldValue( + const google::protobuf::FieldDescriptor* field_desc, const StructValue& struct_value, + absl::Nonnull descriptor_pool, + absl::Nonnull message_factory, + absl::Nonnull arena) { // Special case unset any. if (field_desc->cpp_type() == google::protobuf::FieldDescriptor::CPPTYPE_MESSAGE && field_desc->message_type()->well_known_type() == @@ -54,31 +58,33 @@ absl::StatusOr GetFieldValue(const google::protobuf::FieldDescriptor* fie } } - return struct_value.GetFieldByNumber( - field_desc->number(), value_manager.descriptor_pool(), - value_manager.message_factory(), - value_manager.GetMemoryManager().arena()); + return struct_value.GetFieldByNumber(field_desc->number(), descriptor_pool, + message_factory, arena); } } // namespace absl::Status BindProtoToActivation( const Descriptor& descriptor, const StructValue& struct_value, - ValueManager& value_manager, Activation& activation, - BindProtoUnsetFieldBehavior unset_field_behavior) { + BindProtoUnsetFieldBehavior unset_field_behavior, + absl::Nonnull descriptor_pool, + absl::Nonnull message_factory, + absl::Nonnull arena, + absl::Nonnull activation) { for (int i = 0; i < descriptor.field_count(); i++) { const google::protobuf::FieldDescriptor* field_desc = descriptor.field(i); - CEL_ASSIGN_OR_RETURN(bool should_bind, - ShouldBindField(field_desc, struct_value, - unset_field_behavior, value_manager)); + CEL_ASSIGN_OR_RETURN( + bool should_bind, + ShouldBindField(field_desc, struct_value, unset_field_behavior)); if (!should_bind) { continue; } CEL_ASSIGN_OR_RETURN( - Value field, GetFieldValue(field_desc, struct_value, value_manager)); + Value field, GetFieldValue(field_desc, struct_value, descriptor_pool, + message_factory, arena)); - activation.InsertOrAssignValue(field_desc->name(), std::move(field)); + activation->InsertOrAssignValue(field_desc->name(), std::move(field)); } return absl::OkStatus(); diff --git a/extensions/protobuf/bind_proto_to_activation.h b/extensions/protobuf/bind_proto_to_activation.h index 0756048e6..9167a3ea6 100644 --- a/extensions/protobuf/bind_proto_to_activation.h +++ b/extensions/protobuf/bind_proto_to_activation.h @@ -17,14 +17,16 @@ #include +#include "absl/base/nullability.h" #include "absl/status/status.h" #include "common/casting.h" #include "common/value.h" -#include "common/value_manager.h" #include "extensions/protobuf/value.h" #include "internal/status_macros.h" #include "runtime/activation.h" +#include "google/protobuf/arena.h" #include "google/protobuf/descriptor.h" +#include "google/protobuf/message.h" namespace cel::extensions { @@ -43,9 +45,10 @@ namespace protobuf_internal { // been adapted to a suitable struct value. absl::Status BindProtoToActivation( const google::protobuf::Descriptor& descriptor, const StructValue& struct_value, - ValueManager& value_manager, Activation& activation, - BindProtoUnsetFieldBehavior unset_field_behavior = - BindProtoUnsetFieldBehavior::kSkip); + BindProtoUnsetFieldBehavior unset_field_behavior, + absl::Nonnull descriptor_pool, + absl::Nonnull message_factory, + absl::Nonnull arena, absl::Nonnull activation); } // namespace protobuf_internal @@ -83,17 +86,17 @@ absl::Status BindProtoToActivation( // For repeated fields, an unset field is bound as an empty list. template absl::Status BindProtoToActivation( - const T& context, ValueManager& value_manager, Activation& activation, - BindProtoUnsetFieldBehavior unset_field_behavior = - BindProtoUnsetFieldBehavior::kSkip) { + const T& context, BindProtoUnsetFieldBehavior unset_field_behavior, + absl::Nonnull descriptor_pool, + absl::Nonnull message_factory, + absl::Nonnull arena, + absl::Nonnull activation) { static_assert(std::is_base_of_v); // TODO: for simplicity, just convert the whole message to a // struct value. For performance, may be better to convert members as needed. CEL_ASSIGN_OR_RETURN( Value parent, - ProtoMessageToValue(context, value_manager.descriptor_pool(), - value_manager.message_factory(), - value_manager.GetMemoryManager().arena())); + ProtoMessageToValue(context, descriptor_pool, message_factory, arena)); if (!InstanceOf(parent)) { return absl::InvalidArgumentError( @@ -108,9 +111,20 @@ absl::Status BindProtoToActivation( absl::StrCat("context missing descriptor: ", context.GetTypeName())); } - return protobuf_internal::BindProtoToActivation(*descriptor, struct_value, - value_manager, activation, - unset_field_behavior); + return protobuf_internal::BindProtoToActivation( + *descriptor, struct_value, unset_field_behavior, descriptor_pool, + message_factory, arena, activation); +} +template +absl::Status BindProtoToActivation( + const T& context, + absl::Nonnull descriptor_pool, + absl::Nonnull message_factory, + absl::Nonnull arena, + absl::Nonnull activation) { + return BindProtoToActivation(context, BindProtoUnsetFieldBehavior::kSkip, + descriptor_pool, message_factory, arena, + activation); } } // namespace cel::extensions diff --git a/extensions/protobuf/bind_proto_to_activation_test.cc b/extensions/protobuf/bind_proto_to_activation_test.cc index b4bd05150..6c2ecd490 100644 --- a/extensions/protobuf/bind_proto_to_activation_test.cc +++ b/extensions/protobuf/bind_proto_to_activation_test.cc @@ -48,10 +48,12 @@ TEST_F(BindProtoToActivationTest, BindProtoToActivation) { test_all_types.set_single_int64(123); Activation activation; - ASSERT_THAT(BindProtoToActivation(test_all_types, value_factory, activation), + ASSERT_THAT(BindProtoToActivation(test_all_types, descriptor_pool(), + message_factory(), arena(), &activation), IsOk()); - EXPECT_THAT(activation.FindVariable(value_factory, "single_int64"), + EXPECT_THAT(activation.FindVariable("single_int64", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IntValueIs(123)))); } @@ -62,7 +64,8 @@ TEST_F(BindProtoToActivationTest, BindProtoToActivationWktUnsupported) { int64_value.set_value(123); Activation activation; - EXPECT_THAT(BindProtoToActivation(int64_value, value_factory, activation), + EXPECT_THAT(BindProtoToActivation(int64_value, descriptor_pool(), + message_factory(), arena(), &activation), StatusIs(absl::StatusCode::kInvalidArgument, HasSubstr("google.protobuf.Int64Value"))); } @@ -74,13 +77,15 @@ TEST_F(BindProtoToActivationTest, BindProtoToActivationSkip) { test_all_types.set_single_int64(123); Activation activation; - ASSERT_THAT(BindProtoToActivation(test_all_types, value_factory, activation, - BindProtoUnsetFieldBehavior::kSkip), + ASSERT_THAT(BindProtoToActivation(test_all_types, descriptor_pool(), + message_factory(), arena(), &activation), IsOk()); - EXPECT_THAT(activation.FindVariable(value_factory, "single_int32"), + EXPECT_THAT(activation.FindVariable("single_int32", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Eq(absl::nullopt))); - EXPECT_THAT(activation.FindVariable(value_factory, "single_sint32"), + EXPECT_THAT(activation.FindVariable("single_sint32", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Eq(absl::nullopt))); } @@ -92,15 +97,18 @@ TEST_F(BindProtoToActivationTest, BindProtoToActivationDefault) { Activation activation; ASSERT_THAT( - BindProtoToActivation(test_all_types, value_factory, activation, - BindProtoUnsetFieldBehavior::kBindDefaultValue), + BindProtoToActivation( + test_all_types, BindProtoUnsetFieldBehavior::kBindDefaultValue, + descriptor_pool(), message_factory(), arena(), &activation), IsOk()); // from test_all_types.proto // optional int32_t single_int32 = 1 [default = -32]; - EXPECT_THAT(activation.FindVariable(value_factory, "single_int32"), + EXPECT_THAT(activation.FindVariable("single_int32", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IntValueIs(-32)))); - EXPECT_THAT(activation.FindVariable(value_factory, "single_sint32"), + EXPECT_THAT(activation.FindVariable("single_sint32", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IntValueIs(0)))); } @@ -113,11 +121,13 @@ TEST_F(BindProtoToActivationTest, BindProtoToActivationDefaultAny) { Activation activation; ASSERT_THAT( - BindProtoToActivation(test_all_types, value_factory, activation, - BindProtoUnsetFieldBehavior::kBindDefaultValue), + BindProtoToActivation( + test_all_types, BindProtoUnsetFieldBehavior::kBindDefaultValue, + descriptor_pool(), message_factory(), arena(), &activation), IsOk()); - EXPECT_THAT(activation.FindVariable(value_factory, "single_any"), + EXPECT_THAT(activation.FindVariable("single_any", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(test::IsNullValue()))); } @@ -142,10 +152,12 @@ TEST_F(BindProtoToActivationTest, BindProtoToActivationRepeated) { Activation activation; - ASSERT_THAT(BindProtoToActivation(test_all_types, value_factory, activation), + ASSERT_THAT(BindProtoToActivation(test_all_types, descriptor_pool(), + message_factory(), arena(), &activation), IsOk()); - EXPECT_THAT(activation.FindVariable(value_factory, "repeated_int64"), + EXPECT_THAT(activation.FindVariable("repeated_int64", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IsListValueOfSize(3)))); } @@ -156,10 +168,12 @@ TEST_F(BindProtoToActivationTest, BindProtoToActivationRepeatedEmpty) { test_all_types.set_single_int64(123); Activation activation; - ASSERT_THAT(BindProtoToActivation(test_all_types, value_factory, activation), + ASSERT_THAT(BindProtoToActivation(test_all_types, descriptor_pool(), + message_factory(), arena(), &activation), IsOk()); - EXPECT_THAT(activation.FindVariable(value_factory, "repeated_int32"), + EXPECT_THAT(activation.FindVariable("repeated_int32", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IsListValueOfSize(0)))); } @@ -175,11 +189,14 @@ TEST_F(BindProtoToActivationTest, BindProtoToActivationRepeatedComplex) { nested->set_bb(789); Activation activation; - ASSERT_THAT(BindProtoToActivation(test_all_types, value_factory, activation), + ASSERT_THAT(BindProtoToActivation(test_all_types, descriptor_pool(), + message_factory(), arena(), &activation), IsOk()); - EXPECT_THAT(activation.FindVariable(value_factory, "repeated_nested_message"), - IsOkAndHolds(Optional(IsListValueOfSize(3)))); + EXPECT_THAT( + activation.FindVariable("repeated_nested_message", descriptor_pool(), + message_factory(), arena()), + IsOkAndHolds(Optional(IsListValueOfSize(3)))); } MATCHER_P(IsMapValueOfSize, size, "") { @@ -202,10 +219,12 @@ TEST_F(BindProtoToActivationTest, BindProtoToActivationMap) { Activation activation; - ASSERT_THAT(BindProtoToActivation(test_all_types, value_factory, activation), + ASSERT_THAT(BindProtoToActivation(test_all_types, descriptor_pool(), + message_factory(), arena(), &activation), IsOk()); - EXPECT_THAT(activation.FindVariable(value_factory, "map_int64_int64"), + EXPECT_THAT(activation.FindVariable("map_int64_int64", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IsMapValueOfSize(2)))); } @@ -216,10 +235,12 @@ TEST_F(BindProtoToActivationTest, BindProtoToActivationMapEmpty) { test_all_types.set_single_int64(123); Activation activation; - ASSERT_THAT(BindProtoToActivation(test_all_types, value_factory, activation), + ASSERT_THAT(BindProtoToActivation(test_all_types, descriptor_pool(), + message_factory(), arena(), &activation), IsOk()); - EXPECT_THAT(activation.FindVariable(value_factory, "map_int32_int32"), + EXPECT_THAT(activation.FindVariable("map_int32_int32", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IsMapValueOfSize(0)))); } @@ -234,10 +255,12 @@ TEST_F(BindProtoToActivationTest, BindProtoToActivationMapComplex) { Activation activation; - ASSERT_THAT(BindProtoToActivation(test_all_types, value_factory, activation), + ASSERT_THAT(BindProtoToActivation(test_all_types, descriptor_pool(), + message_factory(), arena(), &activation), IsOk()); - EXPECT_THAT(activation.FindVariable(value_factory, "map_int64_message"), + EXPECT_THAT(activation.FindVariable("map_int64_message", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IsMapValueOfSize(2)))); } diff --git a/runtime/BUILD b/runtime/BUILD index a486f470f..f0e6b37b2 100644 --- a/runtime/BUILD +++ b/runtime/BUILD @@ -27,10 +27,12 @@ cc_library( "//base:attributes", "//common:value", "//internal:status_macros", + "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", "@com_google_absl//absl/types:optional", "@com_google_absl//absl/types:span", + "@com_google_protobuf//:protobuf", ], ) @@ -66,13 +68,17 @@ cc_library( "//common:function_descriptor", "//common:value", "//internal:status_macros", + "@com_google_absl//absl/base:core_headers", + "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/functional:any_invocable", + "@com_google_absl//absl/log:absl_check", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", "@com_google_absl//absl/synchronization", "@com_google_absl//absl/types:optional", "@com_google_absl//absl/types:span", + "@com_google_protobuf//:protobuf", ], ) @@ -82,15 +88,17 @@ cc_test( deps = [ ":activation", ":function", + ":function_overload_reference", "//base:attributes", "//common:function_descriptor", - "//common:memory", - "//common:type", "//common:value", + "//common:value_testing", "//internal:testing", "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/status", + "@com_google_absl//absl/status:status_matchers", "@com_google_absl//absl/status:statusor", + "@com_google_absl//absl/strings:string_view", "@com_google_absl//absl/types:optional", "@com_google_absl//absl/types:span", "@com_google_protobuf//:protobuf", diff --git a/runtime/activation.cc b/runtime/activation.cc index 96e4ef775..9eb72bfd4 100644 --- a/runtime/activation.cc +++ b/runtime/activation.cc @@ -18,6 +18,9 @@ #include #include +#include "absl/base/macros.h" +#include "absl/base/nullability.h" +#include "absl/log/absl_check.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "absl/synchronization/mutex.h" @@ -27,12 +30,21 @@ #include "internal/status_macros.h" #include "runtime/function.h" #include "runtime/function_overload_reference.h" +#include "google/protobuf/arena.h" +#include "google/protobuf/descriptor.h" +#include "google/protobuf/message.h" namespace cel { -absl::StatusOr Activation::FindVariable(ValueManager& factory, - absl::string_view name, - Value& result) const { +absl::StatusOr Activation::FindVariable( + absl::string_view name, + absl::Nonnull descriptor_pool, + absl::Nonnull message_factory, + absl::Nonnull arena, absl::Nonnull result) const { + ABSL_DCHECK(descriptor_pool != nullptr); + ABSL_DCHECK(message_factory != nullptr); + ABSL_DCHECK(result != nullptr); + auto iter = values_.find(name); if (iter == values_.end()) { return false; @@ -40,31 +52,35 @@ absl::StatusOr Activation::FindVariable(ValueManager& factory, const ValueEntry& entry = iter->second; if (entry.provider.has_value()) { - return ProvideValue(factory, name, result); + return ProvideValue(name, descriptor_pool, message_factory, arena, result); } if (entry.value.has_value()) { - result = *entry.value; + *result = *entry.value; return true; } return false; } -absl::StatusOr Activation::ProvideValue(ValueManager& factory, - absl::string_view name, - Value& result) const { +absl::StatusOr Activation::ProvideValue( + absl::string_view name, + absl::Nonnull descriptor_pool, + absl::Nonnull message_factory, + absl::Nonnull arena, absl::Nonnull result) const { absl::MutexLock lock(&mutex_); auto iter = values_.find(name); ABSL_ASSERT(iter != values_.end()); ValueEntry& entry = iter->second; if (entry.value.has_value()) { - result = *entry.value; + *result = *entry.value; return true; } - CEL_ASSIGN_OR_RETURN(auto provided, (*entry.provider)(factory, name)); + CEL_ASSIGN_OR_RETURN( + auto provided, + (*entry.provider)(name, descriptor_pool, message_factory, arena)); if (provided.has_value()) { entry.value = std::move(provided); - result = *entry.value; + *result = *entry.value; return true; } return false; diff --git a/runtime/activation.h b/runtime/activation.h index 9e011fa62..7a850f0c0 100644 --- a/runtime/activation.h +++ b/runtime/activation.h @@ -20,6 +20,7 @@ #include #include +#include "absl/base/nullability.h" #include "absl/container/flat_hash_map.h" #include "absl/functional/any_invocable.h" #include "absl/status/statusor.h" @@ -30,10 +31,12 @@ #include "base/attribute.h" #include "common/function_descriptor.h" #include "common/value.h" -#include "common/value_manager.h" #include "runtime/activation_interface.h" #include "runtime/function.h" #include "runtime/function_overload_reference.h" +#include "google/protobuf/arena.h" +#include "google/protobuf/descriptor.h" +#include "google/protobuf/message.h" namespace cel { @@ -45,7 +48,9 @@ class Activation final : public ActivationInterface { // Definition for value providers. using ValueProvider = absl::AnyInvocable>( - ValueManager&, absl::string_view)>; + absl::string_view, absl::Nonnull, + absl::Nonnull, + absl::Nonnull)>; Activation() = default; @@ -55,9 +60,12 @@ class Activation final : public ActivationInterface { Activation& operator=(Activation&& other); // Implements ActivationInterface. - absl::StatusOr FindVariable(ValueManager& factory, - absl::string_view name, - Value& result) const override; + absl::StatusOr FindVariable( + absl::string_view name, + absl::Nonnull descriptor_pool, + absl::Nonnull message_factory, + absl::Nonnull arena, + absl::Nonnull result) const override; using ActivationInterface::FindVariable; std::vector FindFunctionOverloads( @@ -122,9 +130,11 @@ class Activation final : public ActivationInterface { // Internal getter for provided values. // Assumes entry for name is present and is a provided value. // Handles synchronization for caching the provided value. - absl::StatusOr ProvideValue(ValueManager& value_factory, - absl::string_view name, - Value& result) const; + absl::StatusOr ProvideValue( + absl::string_view name, + absl::Nonnull descriptor_pool, + absl::Nonnull message_factory, + absl::Nonnull arena, absl::Nonnull result) const; // mutex_ used for safe caching of provided variables mutable absl::Mutex mutex_; diff --git a/runtime/activation_interface.h b/runtime/activation_interface.h index 882be4eaa..24867d443 100644 --- a/runtime/activation_interface.h +++ b/runtime/activation_interface.h @@ -17,15 +17,18 @@ #include +#include "absl/base/nullability.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "absl/types/span.h" #include "base/attribute.h" #include "common/value.h" -#include "common/value_manager.h" #include "internal/status_macros.h" #include "runtime/function_overload_reference.h" +#include "google/protobuf/arena.h" +#include "google/protobuf/descriptor.h" +#include "google/protobuf/message.h" namespace cel { @@ -40,13 +43,21 @@ class ActivationInterface { virtual ~ActivationInterface() = default; // Find value for a string (possibly qualified) variable name. - virtual absl::StatusOr FindVariable(ValueManager& factory, - absl::string_view name, - Value& result) const = 0; + virtual absl::StatusOr FindVariable( + absl::string_view name, + absl::Nonnull descriptor_pool, + absl::Nonnull message_factory, + absl::Nonnull arena, + absl::Nonnull result) const = 0; absl::StatusOr> FindVariable( - ValueManager& factory, absl::string_view name) const { + absl::string_view name, + absl::Nonnull descriptor_pool, + absl::Nonnull message_factory, + absl::Nonnull arena) const { Value result; - CEL_ASSIGN_OR_RETURN(auto found, FindVariable(factory, name, result)); + CEL_ASSIGN_OR_RETURN( + auto found, + FindVariable(name, descriptor_pool, message_factory, arena, &result)); if (found) { return result; } diff --git a/runtime/activation_test.cc b/runtime/activation_test.cc index fb83b3db6..f1356582f 100644 --- a/runtime/activation_test.cc +++ b/runtime/activation_test.cc @@ -16,28 +16,29 @@ #include #include +#include #include "absl/base/nullability.h" #include "absl/status/status.h" +#include "absl/status/status_matchers.h" #include "absl/status/statusor.h" +#include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "absl/types/span.h" #include "base/attribute.h" #include "common/function_descriptor.h" -#include "common/memory.h" -#include "common/type.h" -#include "common/type_reflector.h" #include "common/value.h" -#include "common/value_manager.h" -#include "common/values/legacy_value_manager.h" +#include "common/value_testing.h" #include "internal/testing.h" #include "runtime/function.h" +#include "runtime/function_overload_reference.h" #include "google/protobuf/arena.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/message.h" namespace cel { namespace { + using ::absl_testing::IsOkAndHolds; using ::absl_testing::StatusIs; using testing::ElementsAre; @@ -73,40 +74,32 @@ class FunctionImpl : public cel::Function { } }; -class ActivationTest : public testing::Test { - public: - ActivationTest() - : value_factory_(MemoryManagerRef::ReferenceCounting(), - TypeReflector::Builtin()) {} - - protected: - common_internal::LegacyValueManager value_factory_; -}; +using ActivationTest = common_internal::ValueTest<>; TEST_F(ActivationTest, ValueNotFound) { Activation activation; - EXPECT_THAT(activation.FindVariable(value_factory_, "var1"), + EXPECT_THAT(activation.FindVariable("var1", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Eq(absl::nullopt))); } TEST_F(ActivationTest, InsertValue) { Activation activation; - EXPECT_TRUE(activation.InsertOrAssignValue( - "var1", value_factory_.CreateIntValue(42))); + EXPECT_TRUE(activation.InsertOrAssignValue("var1", IntValue(42))); - EXPECT_THAT(activation.FindVariable(value_factory_, "var1"), + EXPECT_THAT(activation.FindVariable("var1", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IsIntValue(42)))); } TEST_F(ActivationTest, InsertValueOverwrite) { Activation activation; - EXPECT_TRUE(activation.InsertOrAssignValue( - "var1", value_factory_.CreateIntValue(42))); - EXPECT_FALSE( - activation.InsertOrAssignValue("var1", value_factory_.CreateIntValue(0))); + EXPECT_TRUE(activation.InsertOrAssignValue("var1", IntValue(42))); + EXPECT_FALSE(activation.InsertOrAssignValue("var1", IntValue(0))); - EXPECT_THAT(activation.FindVariable(value_factory_, "var1"), + EXPECT_THAT(activation.FindVariable("var1", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IsIntValue(0)))); } @@ -114,11 +107,13 @@ TEST_F(ActivationTest, InsertProvider) { Activation activation; EXPECT_TRUE(activation.InsertOrAssignValueProvider( - "var1", [](ValueManager& factory, absl::string_view name) { - return factory.CreateIntValue(42); - })); + "var1", + [](absl::string_view name, absl::Nonnull, + absl::Nonnull, + absl::Nonnull) { return IntValue(42); })); - EXPECT_THAT(activation.FindVariable(value_factory_, "var1"), + EXPECT_THAT(activation.FindVariable("var1", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IsIntValue(42)))); } @@ -126,11 +121,13 @@ TEST_F(ActivationTest, InsertProviderForwardsNotFound) { Activation activation; EXPECT_TRUE(activation.InsertOrAssignValueProvider( - "var1", [](ValueManager& factory, absl::string_view name) { - return absl::nullopt; - })); + "var1", + [](absl::string_view name, absl::Nonnull, + absl::Nonnull, + absl::Nonnull) { return absl::nullopt; })); - EXPECT_THAT(activation.FindVariable(value_factory_, "var1"), + EXPECT_THAT(activation.FindVariable("var1", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Eq(absl::nullopt))); } @@ -138,11 +135,15 @@ TEST_F(ActivationTest, InsertProviderForwardsStatus) { Activation activation; EXPECT_TRUE(activation.InsertOrAssignValueProvider( - "var1", [](ValueManager& factory, absl::string_view name) { + "var1", + [](absl::string_view name, absl::Nonnull, + absl::Nonnull, + absl::Nonnull) { return absl::InternalError("test"); })); - EXPECT_THAT(activation.FindVariable(value_factory_, "var1"), + EXPECT_THAT(activation.FindVariable("var1", descriptor_pool(), + message_factory(), arena()), StatusIs(absl::StatusCode::kInternal, "test")); } @@ -151,14 +152,19 @@ TEST_F(ActivationTest, ProviderMemoized) { int call_count = 0; EXPECT_TRUE(activation.InsertOrAssignValueProvider( - "var1", [&call_count](ValueManager& factory, absl::string_view name) { + "var1", [&call_count](absl::string_view name, + absl::Nonnull, + absl::Nonnull, + absl::Nonnull) { call_count++; - return factory.CreateIntValue(42); + return IntValue(42); })); - EXPECT_THAT(activation.FindVariable(value_factory_, "var1"), + EXPECT_THAT(activation.FindVariable("var1", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IsIntValue(42)))); - EXPECT_THAT(activation.FindVariable(value_factory_, "var1"), + EXPECT_THAT(activation.FindVariable("var1", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IsIntValue(42)))); EXPECT_EQ(call_count, 1); } @@ -167,15 +173,18 @@ TEST_F(ActivationTest, InsertProviderOverwrite) { Activation activation; EXPECT_TRUE(activation.InsertOrAssignValueProvider( - "var1", [](ValueManager& factory, absl::string_view name) { - return factory.CreateIntValue(42); - })); + "var1", + [](absl::string_view name, absl::Nonnull, + absl::Nonnull, + absl::Nonnull) { return IntValue(42); })); EXPECT_FALSE(activation.InsertOrAssignValueProvider( - "var1", [](ValueManager& factory, absl::string_view name) { - return factory.CreateIntValue(0); - })); + "var1", + [](absl::string_view name, absl::Nonnull, + absl::Nonnull, + absl::Nonnull) { return IntValue(0); })); - EXPECT_THAT(activation.FindVariable(value_factory_, "var1"), + EXPECT_THAT(activation.FindVariable("var1", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IsIntValue(0)))); } @@ -183,20 +192,23 @@ TEST_F(ActivationTest, ValuesAndProvidersShareNamespace) { Activation activation; bool called = false; - EXPECT_TRUE(activation.InsertOrAssignValue( - "var1", value_factory_.CreateIntValue(41))); - EXPECT_TRUE(activation.InsertOrAssignValue( - "var2", value_factory_.CreateIntValue(41))); + EXPECT_TRUE(activation.InsertOrAssignValue("var1", IntValue(41))); + EXPECT_TRUE(activation.InsertOrAssignValue("var2", IntValue(41))); EXPECT_FALSE(activation.InsertOrAssignValueProvider( - "var1", [&called](ValueManager& factory, absl::string_view name) { + "var1", [&called](absl::string_view name, + absl::Nonnull, + absl::Nonnull, + absl::Nonnull) { called = true; - return factory.CreateIntValue(42); + return IntValue(42); })); - EXPECT_THAT(activation.FindVariable(value_factory_, "var1"), + EXPECT_THAT(activation.FindVariable("var1", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IsIntValue(42)))); - EXPECT_THAT(activation.FindVariable(value_factory_, "var2"), + EXPECT_THAT(activation.FindVariable("var2", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IsIntValue(41)))); EXPECT_TRUE(called); } @@ -312,15 +324,13 @@ TEST_F(ActivationTest, MoveAssignment) { ASSERT_TRUE( moved_from.InsertFunction(FunctionDescriptor("Fn", false, {Kind::kAny}), std::make_unique())); - ASSERT_TRUE( - moved_from.InsertOrAssignValue("val", value_factory_.CreateIntValue(42))); + ASSERT_TRUE(moved_from.InsertOrAssignValue("val", IntValue(42))); ASSERT_TRUE(moved_from.InsertOrAssignValueProvider( "val_provided", - [](ValueManager& factory, - absl::string_view name) -> absl::StatusOr> { - return factory.CreateIntValue(42); - })); + [](absl::string_view name, absl::Nonnull, + absl::Nonnull, absl::Nonnull) + -> absl::StatusOr> { return IntValue(42); })); moved_from.SetUnknownPatterns( {AttributePattern("var1", {AttributeQualifierPattern::OfString("field1")}), @@ -335,9 +345,11 @@ TEST_F(ActivationTest, MoveAssignment) { Activation moved_to; moved_to = std::move(moved_from); - EXPECT_THAT(moved_to.FindVariable(value_factory_, "val"), + EXPECT_THAT(moved_to.FindVariable("val", descriptor_pool(), message_factory(), + arena()), IsOkAndHolds(Optional(IsIntValue(42)))); - EXPECT_THAT(moved_to.FindVariable(value_factory_, "val_provided"), + EXPECT_THAT(moved_to.FindVariable("val_provided", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IsIntValue(42)))); EXPECT_THAT(moved_to.FindFunctionOverloads("Fn"), SizeIs(1)); EXPECT_THAT(moved_to.GetUnknownAttributes(), SizeIs(2)); @@ -345,9 +357,11 @@ TEST_F(ActivationTest, MoveAssignment) { // moved from value is empty. (well defined but not specified state) // NOLINTBEGIN(bugprone-use-after-move) - EXPECT_THAT(moved_from.FindVariable(value_factory_, "val"), + EXPECT_THAT(moved_from.FindVariable("val", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Eq(absl::nullopt))); - EXPECT_THAT(moved_from.FindVariable(value_factory_, "val_provided"), + EXPECT_THAT(moved_from.FindVariable("val_provided", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Eq(absl::nullopt))); EXPECT_THAT(moved_from.FindFunctionOverloads("Fn"), SizeIs(0)); EXPECT_THAT(moved_from.GetUnknownAttributes(), SizeIs(0)); @@ -361,15 +375,13 @@ TEST_F(ActivationTest, MoveCtor) { ASSERT_TRUE( moved_from.InsertFunction(FunctionDescriptor("Fn", false, {Kind::kAny}), std::make_unique())); - ASSERT_TRUE( - moved_from.InsertOrAssignValue("val", value_factory_.CreateIntValue(42))); + ASSERT_TRUE(moved_from.InsertOrAssignValue("val", IntValue(42))); ASSERT_TRUE(moved_from.InsertOrAssignValueProvider( "val_provided", - [](ValueManager& factory, - absl::string_view name) -> absl::StatusOr> { - return factory.CreateIntValue(42); - })); + [](absl::string_view name, absl::Nonnull, + absl::Nonnull, absl::Nonnull) + -> absl::StatusOr> { return IntValue(42); })); moved_from.SetUnknownPatterns( {AttributePattern("var1", {AttributeQualifierPattern::OfString("field1")}), @@ -383,9 +395,11 @@ TEST_F(ActivationTest, MoveCtor) { Activation moved_to = std::move(moved_from); - EXPECT_THAT(moved_to.FindVariable(value_factory_, "val"), + EXPECT_THAT(moved_to.FindVariable("val", descriptor_pool(), message_factory(), + arena()), IsOkAndHolds(Optional(IsIntValue(42)))); - EXPECT_THAT(moved_to.FindVariable(value_factory_, "val_provided"), + EXPECT_THAT(moved_to.FindVariable("val_provided", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IsIntValue(42)))); EXPECT_THAT(moved_to.FindFunctionOverloads("Fn"), SizeIs(1)); EXPECT_THAT(moved_to.GetUnknownAttributes(), SizeIs(2)); @@ -393,9 +407,11 @@ TEST_F(ActivationTest, MoveCtor) { // moved from value is empty. // NOLINTBEGIN(bugprone-use-after-move) - EXPECT_THAT(moved_from.FindVariable(value_factory_, "val"), + EXPECT_THAT(moved_from.FindVariable("val", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Eq(absl::nullopt))); - EXPECT_THAT(moved_from.FindVariable(value_factory_, "val_provided"), + EXPECT_THAT(moved_from.FindVariable("val_provided", descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Eq(absl::nullopt))); EXPECT_THAT(moved_from.FindFunctionOverloads("Fn"), SizeIs(0)); EXPECT_THAT(moved_from.GetUnknownAttributes(), SizeIs(0));