Skip to content

Commit 457a5b5

Browse files
jnthntatumcopybara-github
jnthntatum
authored and
copybara-github
committed
Mark old overload of cel::Function::Invoke as deprecated and de-virtualize.
Migrate internal classes overriding. This is a breaking change for any callers providing a custom implementation of cel::Function, but this should be rare as most applications use the FunctionAdapter helpers. PiperOrigin-RevId: 850158364
1 parent c4915f0 commit 457a5b5

10 files changed

Lines changed: 50 additions & 92 deletions

File tree

‎eval/compiler/flat_expr_builder_test.cc‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2395,9 +2395,7 @@ struct ConstantFoldingTestCase {
23952395

23962396
class UnknownFunctionImpl : public cel::Function {
23972397
absl::StatusOr<Value> Invoke(absl::Span<const Value> args,
2398-
const google::protobuf::DescriptorPool* absl_nonnull,
2399-
google::protobuf::MessageFactory* absl_nonnull,
2400-
google::protobuf::Arena* absl_nonnull) const override {
2398+
const InvokeContext& context) const override {
24012399
return cel::UnknownValue();
24022400
}
24032401
};

‎eval/public/BUILD‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,6 @@ cc_library(
198198
"//eval/internal:interop",
199199
"//internal:status_macros",
200200
"//runtime:function",
201-
"@com_google_absl//absl/base:nullability",
202201
"@com_google_absl//absl/status",
203202
"@com_google_absl//absl/status:statusor",
204203
"@com_google_absl//absl/types:span",

‎eval/public/cel_function.cc‎

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,13 @@
33
#include <cstddef>
44
#include <vector>
55

6-
#include "absl/base/nullability.h"
76
#include "absl/status/statusor.h"
87
#include "absl/types/span.h"
98
#include "common/value.h"
109
#include "eval/internal/interop.h"
1110
#include "eval/public/cel_value.h"
1211
#include "internal/status_macros.h"
1312
#include "runtime/function.h"
14-
#include "google/protobuf/arena.h"
15-
#include "google/protobuf/descriptor.h"
16-
#include "google/protobuf/message.h"
1713

1814
namespace google::api::expr::runtime {
1915

@@ -56,9 +52,7 @@ bool CelFunction::MatchArguments(absl::Span<const cel::Value> arguments) const {
5652

5753
absl::StatusOr<Value> CelFunction::Invoke(
5854
absl::Span<const cel::Value> arguments,
59-
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
60-
google::protobuf::MessageFactory* absl_nonnull message_factory,
61-
google::protobuf::Arena* absl_nonnull arena) const {
55+
const cel::Function::InvokeContext& context) const {
6256
std::vector<CelValue> legacy_args;
6357
legacy_args.reserve(arguments.size());
6458

@@ -68,22 +62,15 @@ absl::StatusOr<Value> CelFunction::Invoke(
6862
// interpreter expects to only be used with internal program steps.
6963
for (const auto& arg : arguments) {
7064
CEL_ASSIGN_OR_RETURN(legacy_args.emplace_back(),
71-
ToLegacyValue(arena, arg, true));
65+
ToLegacyValue(context.arena(), arg, true));
7266
}
7367

7468
CelValue legacy_result;
7569

76-
CEL_RETURN_IF_ERROR(Evaluate(legacy_args, &legacy_result, arena));
70+
CEL_RETURN_IF_ERROR(Evaluate(legacy_args, &legacy_result, context.arena()));
7771

7872
return cel::interop_internal::LegacyValueToModernValueOrDie(
79-
arena, legacy_result, /*unchecked=*/true);
80-
}
81-
82-
absl::StatusOr<Value> CelFunction::Invoke(
83-
absl::Span<const cel::Value> arguments,
84-
const cel::Function::InvokeContext& context) const {
85-
return CelFunction::Invoke(arguments, context.descriptor_pool(),
86-
context.message_factory(), context.arena());
73+
context.arena(), legacy_result, /*unchecked=*/true);
8774
}
8875

8976
} // namespace google::api::expr::runtime

‎eval/public/cel_function.h‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
#include <utility>
55

6-
#include "absl/base/nullability.h"
76
#include "absl/status/status.h"
87
#include "absl/status/statusor.h"
98
#include "absl/types/span.h"
@@ -12,8 +11,6 @@
1211
#include "eval/public/cel_value.h"
1312
#include "runtime/function.h"
1413
#include "google/protobuf/arena.h"
15-
#include "google/protobuf/descriptor.h"
16-
#include "google/protobuf/message.h"
1714

1815
namespace google::api::expr::runtime {
1916

@@ -65,11 +62,8 @@ class CelFunction : public ::cel::Function {
6562
bool MatchArguments(absl::Span<const cel::Value> arguments) const;
6663

6764
// Implements cel::Function.
68-
absl::StatusOr<cel::Value> Invoke(
69-
absl::Span<const cel::Value> arguments,
70-
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
71-
google::protobuf::MessageFactory* absl_nonnull message_factory,
72-
google::protobuf::Arena* absl_nonnull arena) const final;
65+
using cel::Function::Invoke;
66+
7367
absl::StatusOr<cel::Value> Invoke(
7468
absl::Span<const cel::Value> arguments,
7569
const cel::Function::InvokeContext& context) const final;

‎runtime/BUILD‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,11 @@ cc_test(
156156
":function_registry",
157157
"//common:function_descriptor",
158158
"//common:kind",
159+
"//common:value",
159160
"//internal:testing",
160-
"@com_google_absl//absl/base:nullability",
161161
"@com_google_absl//absl/status",
162-
"@com_google_protobuf//:protobuf",
162+
"@com_google_absl//absl/status:statusor",
163+
"@com_google_absl//absl/types:span",
163164
],
164165
)
165166

@@ -505,13 +506,11 @@ cc_library(
505506
":function",
506507
":register_function_helper",
507508
"//common:function_descriptor",
508-
"//common:kind",
509509
"//common:value",
510510
"//internal:status_macros",
511511
"//runtime/internal:function_adapter",
512512
"@com_google_absl//absl/base:nullability",
513513
"@com_google_absl//absl/functional:any_invocable",
514-
"@com_google_absl//absl/functional:bind_front",
515514
"@com_google_absl//absl/status",
516515
"@com_google_absl//absl/status:statusor",
517516
"@com_google_absl//absl/strings",
@@ -588,7 +587,6 @@ cc_test(
588587
"//parser",
589588
"//parser:options",
590589
"//runtime/internal:runtime_impl",
591-
"@com_google_absl//absl/base:nullability",
592590
"@com_google_absl//absl/status",
593591
"@com_google_absl//absl/status:status_matchers",
594592
"@com_google_absl//absl/status:statusor",
@@ -605,6 +603,7 @@ cc_library(
605603
],
606604
deps = [
607605
"//common:value",
606+
"@com_google_absl//absl/base:core_headers",
608607
"@com_google_absl//absl/base:nullability",
609608
"@com_google_absl//absl/status:statusor",
610609
"@com_google_absl//absl/types:span",

‎runtime/activation_test.cc‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,7 @@ class FunctionImpl : public cel::Function {
6767
FunctionImpl() = default;
6868

6969
absl::StatusOr<Value> Invoke(absl::Span<const Value> args,
70-
const google::protobuf::DescriptorPool* absl_nonnull,
71-
google::protobuf::MessageFactory* absl_nonnull,
72-
google::protobuf::Arena* absl_nonnull) const override {
70+
const InvokeContext& context) const override {
7371
return NullValue();
7472
}
7573
};

‎runtime/function.h‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#ifndef THIRD_PARTY_CEL_CPP_COMMON_FUNCTION_H_
1616
#define THIRD_PARTY_CEL_CPP_COMMON_FUNCTION_H_
1717

18+
#include "absl/base/attributes.h"
1819
#include "absl/base/nullability.h"
1920
#include "absl/status/statusor.h"
2021
#include "absl/types/span.h"
@@ -81,6 +82,13 @@ class Function {
8182
const EmbedderContext* absl_nullable embedder_context_;
8283
};
8384

85+
ABSL_DEPRECATED("Use the InvokeContext overload instead.")
86+
inline absl::StatusOr<Value> Invoke(
87+
absl::Span<const Value> args,
88+
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
89+
google::protobuf::MessageFactory* absl_nonnull message_factory,
90+
google::protobuf::Arena* absl_nonnull arena) const;
91+
8492
// Attempt to evaluate an extension function based on the runtime arguments
8593
// during the evaluation of a CEL expression.
8694
//
@@ -89,18 +97,19 @@ class Function {
8997
//
9098
// A cel::ErrorValue typed result is considered a recoverable error and
9199
// follows CEL's logical short-circuiting behavior.
92-
virtual absl::StatusOr<Value> Invoke(
93-
absl::Span<const Value> args,
94-
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
95-
google::protobuf::MessageFactory* absl_nonnull message_factory,
96-
google::protobuf::Arena* absl_nonnull arena) const = 0;
97100
virtual absl::StatusOr<Value> Invoke(absl::Span<const Value> args,
98-
const InvokeContext& context) const {
99-
return Invoke(args, context.descriptor_pool(), context.message_factory(),
100-
context.arena());
101-
}
101+
const InvokeContext& context) const = 0;
102102
};
103103

104+
absl::StatusOr<Value> Function::Invoke(
105+
absl::Span<const Value> args,
106+
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
107+
google::protobuf::MessageFactory* absl_nonnull message_factory,
108+
google::protobuf::Arena* absl_nonnull arena) const {
109+
InvokeContext context(descriptor_pool, message_factory, arena);
110+
return Invoke(args, context);
111+
}
112+
104113
} // namespace cel
105114

106115
#endif // THIRD_PARTY_CEL_CPP_COMMON_FUNCTION_H_

‎runtime/function_adapter.h‎

Lines changed: 12 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -159,22 +159,6 @@ struct ToArgsHelper {
159159
}
160160
};
161161

162-
class FunctionAdapterBase : public Function {
163-
public:
164-
using Function::Invoke;
165-
166-
// Should not be called by CEL, but added for backward compatibility for
167-
// client code tests.
168-
absl::StatusOr<Value> Invoke(
169-
absl::Span<const Value> args,
170-
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
171-
google::protobuf::MessageFactory* absl_nonnull message_factory,
172-
google::protobuf::Arena* absl_nonnull arena) const final {
173-
Function::InvokeContext context(descriptor_pool, message_factory, arena);
174-
return Invoke(args, context);
175-
}
176-
};
177-
178162
} // namespace runtime_internal
179163

180164
// Adapter class for generating CEL extension functions from a one argument
@@ -247,12 +231,12 @@ class NullaryFunctionAdapter
247231
}
248232

249233
private:
250-
class UnaryFunctionImpl : public runtime_internal::FunctionAdapterBase {
234+
class UnaryFunctionImpl : public Function {
251235
public:
252236
explicit UnaryFunctionImpl(FunctionType fn) : fn_(std::move(fn)) {}
253237
absl::StatusOr<Value> Invoke(
254238
absl::Span<const Value> args,
255-
const Function::InvokeContext& context) const override {
239+
const Function::InvokeContext& context) const final {
256240
if (args.size() != 0) {
257241
return absl::InvalidArgumentError(
258242
"unexpected number of arguments for nullary function");
@@ -346,12 +330,12 @@ class UnaryFunctionAdapter : public RegisterHelper<UnaryFunctionAdapter<T, U>> {
346330
}
347331

348332
private:
349-
class UnaryFunctionImpl : public runtime_internal::FunctionAdapterBase {
333+
class UnaryFunctionImpl : public Function {
350334
public:
351335
explicit UnaryFunctionImpl(FunctionType fn) : fn_(std::move(fn)) {}
352336
absl::StatusOr<Value> Invoke(
353337
absl::Span<const Value> args,
354-
const Function::InvokeContext& context) const override {
338+
const Function::InvokeContext& context) const final {
355339
using ArgTraits = runtime_internal::AdaptedTypeTraits<U>;
356340
if (args.size() != 1) {
357341
return absl::InvalidArgumentError(
@@ -497,12 +481,12 @@ class BinaryFunctionAdapter
497481
}
498482

499483
private:
500-
class BinaryFunctionImpl : public runtime_internal::FunctionAdapterBase {
484+
class BinaryFunctionImpl : public Function {
501485
public:
502486
explicit BinaryFunctionImpl(FunctionType fn) : fn_(std::move(fn)) {}
503487
absl::StatusOr<Value> Invoke(
504488
absl::Span<const Value> args,
505-
const Function::InvokeContext& context) const override {
489+
const Function::InvokeContext& context) const final {
506490
using Arg1Traits = runtime_internal::AdaptedTypeTraits<U>;
507491
using Arg2Traits = runtime_internal::AdaptedTypeTraits<V>;
508492
if (args.size() != 2) {
@@ -588,12 +572,12 @@ class TernaryFunctionAdapter
588572
}
589573

590574
private:
591-
class TernaryFunctionImpl : public runtime_internal::FunctionAdapterBase {
575+
class TernaryFunctionImpl : public Function {
592576
public:
593577
explicit TernaryFunctionImpl(FunctionType fn) : fn_(std::move(fn)) {}
594578
absl::StatusOr<Value> Invoke(
595579
absl::Span<const Value> args,
596-
const Function::InvokeContext& context) const override {
580+
const Function::InvokeContext& context) const final {
597581
using Arg1Traits = runtime_internal::AdaptedTypeTraits<U>;
598582
using Arg2Traits = runtime_internal::AdaptedTypeTraits<V>;
599583
using Arg3Traits = runtime_internal::AdaptedTypeTraits<W>;
@@ -684,12 +668,12 @@ class QuaternaryFunctionAdapter
684668
}
685669

686670
private:
687-
class QuaternaryFunctionImpl : public runtime_internal::FunctionAdapterBase {
671+
class QuaternaryFunctionImpl : public Function {
688672
public:
689673
explicit QuaternaryFunctionImpl(FunctionType fn) : fn_(std::move(fn)) {}
690674
absl::StatusOr<Value> Invoke(
691675
absl::Span<const Value> args,
692-
const Function::InvokeContext& context) const override {
676+
const Function::InvokeContext& context) const final {
693677
using Arg1Traits = runtime_internal::AdaptedTypeTraits<U>;
694678
using Arg2Traits = runtime_internal::AdaptedTypeTraits<V>;
695679
using Arg3Traits = runtime_internal::AdaptedTypeTraits<W>;
@@ -807,7 +791,7 @@ class NaryFunctionAdapter
807791
}
808792

809793
private:
810-
class NaryFunctionImpl : public runtime_internal::FunctionAdapterBase {
794+
class NaryFunctionImpl : public Function {
811795
private:
812796
using ArgBuffer = std::tuple<
813797
typename runtime_internal::AdaptedTypeTraits<Args>::AssignableType...>;
@@ -816,7 +800,7 @@ class NaryFunctionAdapter
816800
explicit NaryFunctionImpl(FunctionType fn) : fn_(std::move(fn)) {}
817801
absl::StatusOr<Value> Invoke(
818802
absl::Span<const Value> args,
819-
const Function::InvokeContext& context) const override {
803+
const Function::InvokeContext& context) const final {
820804
if (args.size() != sizeof...(Args)) {
821805
return absl::InvalidArgumentError(
822806
absl::StrCat("unexpected number of arguments for ", sizeof...(Args),

‎runtime/function_registry_test.cc‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,18 @@
1919
#include <tuple>
2020
#include <vector>
2121

22-
#include "absl/base/nullability.h"
2322
#include "absl/status/status.h"
23+
#include "absl/status/statusor.h"
24+
#include "absl/types/span.h"
2425
#include "common/function_descriptor.h"
2526
#include "common/kind.h"
27+
#include "common/value.h"
2628
#include "internal/testing.h"
2729
#include "runtime/activation.h"
2830
#include "runtime/function.h"
2931
#include "runtime/function_adapter.h"
3032
#include "runtime/function_overload_reference.h"
3133
#include "runtime/function_provider.h"
32-
#include "google/protobuf/arena.h"
33-
#include "google/protobuf/descriptor.h"
34-
#include "google/protobuf/message.h"
3534

3635
namespace cel {
3736

@@ -50,11 +49,8 @@ class ConstIntFunction : public cel::Function {
5049
return {"ConstFunction", false, {}};
5150
}
5251

53-
absl::StatusOr<Value> Invoke(
54-
absl::Span<const Value> args,
55-
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
56-
google::protobuf::MessageFactory* absl_nonnull message_factory,
57-
google::protobuf::Arena* absl_nonnull arena) const override {
52+
absl::StatusOr<Value> Invoke(absl::Span<const Value> args,
53+
const InvokeContext& context) const override {
5854
return IntValue(42);
5955
}
6056
};

‎runtime/optional_types_test.cc‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
#include <vector>
2323

2424
#include "cel/expr/syntax.pb.h"
25-
#include "absl/base/nullability.h"
2625
#include "absl/status/status.h"
2726
#include "absl/status/status_matchers.h"
2827
#include "absl/status/statusor.h"
@@ -45,8 +44,6 @@
4544
#include "runtime/runtime_options.h"
4645
#include "runtime/standard_runtime_builder_factory.h"
4746
#include "google/protobuf/arena.h"
48-
#include "google/protobuf/descriptor.h"
49-
#include "google/protobuf/message.h"
5047

5148
namespace cel::extensions {
5249
namespace {
@@ -306,13 +303,10 @@ class UnreachableFunction final : public cel::Function {
306303
public:
307304
explicit UnreachableFunction(int64_t* count) : count_(count) {}
308305

309-
absl::StatusOr<Value> Invoke(
310-
absl::Span<const Value> args,
311-
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
312-
google::protobuf::MessageFactory* absl_nonnull message_factory,
313-
google::protobuf::Arena* absl_nonnull arena) const override {
306+
absl::StatusOr<Value> Invoke(absl::Span<const Value> args,
307+
const InvokeContext& context) const override {
314308
++(*count_);
315-
return ErrorValue{absl::CancelledError()};
309+
return ErrorValue(absl::CancelledError());
316310
}
317311

318312
private:

0 commit comments

Comments
 (0)