Skip to content

Commit

Permalink
Introduce CelTypeRegistry for tracking type identifiers with updates …
Browse files Browse the repository at this point in the history 
…in type resolution.

Type and enum constant values may be shadowed by variables with the same
name provided within an Activation in order to preserve backward compatibility
with potential existing usages of the library.

This change makes it possible to find core CEL type names as identifiers, and
if the `enable_qualified_type_identifiers` option is enabled, then qualified
names which appear within Select expressions can be resolved to types which
have either been registered with the CelTypeRegistry or to protobuf type names
which have been linked into the binary.

PiperOrigin-RevId: 353680095
  • Loading branch information
@TristonianJones @kyessenov
TristonianJones authored and kyessenov committed Jan 29, 2021
1 parent b19dad9 commit bca699e
Show file tree
Hide file tree
Showing 35 changed files with 1,533 additions and 484 deletions.
10 changes: 2 additions & 8 deletions conformance/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@ ALL_TESTS = [
"@com_google_cel_spec//tests/simple:testdata/macros.textproto",
"@com_google_cel_spec//tests/simple:testdata/namespace.textproto",
"@com_google_cel_spec//tests/simple:testdata/plumbing.textproto",
# TODO(issues/92): Support for parse-only proto message creation within a container.
# "@com_google_cel_spec//tests/simple:testdata/proto2.textproto",
# "@com_google_cel_spec//tests/simple:testdata/proto3.textproto",
"@com_google_cel_spec//tests/simple:testdata/proto2.textproto",
"@com_google_cel_spec//tests/simple:testdata/proto3.textproto",
"@com_google_cel_spec//tests/simple:testdata/string.textproto",
"@com_google_cel_spec//tests/simple:testdata/timestamps.textproto",
"@com_google_cel_spec//tests/simple:testdata/unknowns.textproto",
Expand Down Expand Up @@ -93,16 +92,11 @@ cc_binary(
"--skip_test=conversions/uint/double_nearest,double_nearest_int,double_half_away",
# TODO(issues/82): Unexpected behavior when converting invalid bytes to string.
"--skip_test=conversions/string/bytes_invalid",
# TODO(issues/83): Missing type() conversion functions
"--skip_test=conversions/type",
# TODO(issues/96): Well-known type conversion support.
"--skip_test=proto2/literal_wellknown",
"--skip_test=proto3/literal_wellknown",
"--skip_test=proto2/empty_field/wkt",
"--skip_test=proto3/empty_field/wkt",
# TODO(issues/92): Support for parse-only proto message creation within a container.
"--skip_test=proto2/has/undefined",
"--skip_test=proto3/has/undefined",
# Requires container support
"--skip_test=namespace/namespace/self_eval_container_lookup_unchecked",
"--skip_test=basic/self_eval_nonzeroish/self_eval_bytes_invalid_utf8",
Expand Down
31 changes: 16 additions & 15 deletions conformance/server.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@
#include "proto/test/v1/proto3/test_all_types.pb.h"


using absl::Status;
using absl::StatusCode;
using ::google::protobuf::Arena;
using ::google::protobuf::util::JsonStringToMessage;
using ::google::protobuf::util::MessageToJsonString;
Expand All @@ -42,10 +40,10 @@ class ConformanceServiceImpl {
public:
explicit ConformanceServiceImpl(std::unique_ptr<CelExpressionBuilder> builder)
: builder_(std::move(builder)),
proto2Tests_(&google::api::expr::test::v1::proto2::TestAllTypes::
default_instance()),
proto3Tests_(&google::api::expr::test::v1::proto3::TestAllTypes::
default_instance()) {}
proto2_tests_(&google::api::expr::test::v1::proto2::TestAllTypes::
default_instance()),
proto3_tests_(&google::api::expr::test::v1::proto3::TestAllTypes::
default_instance()) {}

void Parse(const v1alpha1::ParseRequest* request,
v1alpha1::ParseResponse* response) {
Expand All @@ -63,7 +61,7 @@ class ConformanceServiceImpl {
} else {
google::api::expr::v1alpha1::ParsedExpr out;
(out).MergeFrom(parse_status.value());
response->mutable_parsed_expr()->CopyFrom(out);
*response->mutable_parsed_expr() = out;
}
}

Expand All @@ -87,6 +85,7 @@ class ConformanceServiceImpl {
google::api::expr::v1alpha1::SourceInfo source_info;
google::api::expr::v1alpha1::Expr out;
(out).MergeFrom(*expr);
builder_->set_container(request->container());
auto cel_expression_status = builder_->CreateExpression(&out, &source_info);

if (!cel_expression_status.ok()) {
Expand Down Expand Up @@ -144,13 +143,14 @@ class ConformanceServiceImpl {

private:
std::unique_ptr<CelExpressionBuilder> builder_;
const google::api::expr::test::v1::proto2::TestAllTypes* proto2Tests_;
const google::api::expr::test::v1::proto3::TestAllTypes* proto3Tests_;
const google::api::expr::test::v1::proto2::TestAllTypes* proto2_tests_;
const google::api::expr::test::v1::proto3::TestAllTypes* proto3_tests_;
};

int RunServer(bool optimize) {
google::protobuf::Arena arena;
InterpreterOptions options;
options.enable_qualified_type_identifiers = true;

if (optimize) {
std::cerr << "Enabling optimizations" << std::endl;
Expand All @@ -160,14 +160,15 @@ int RunServer(bool optimize) {

std::unique_ptr<CelExpressionBuilder> builder =
CreateCelExpressionBuilder(options);
builder->AddResolvableEnum(
auto type_registry = builder->GetTypeRegistry();
type_registry->Register(
google::api::expr::test::v1::proto2::GlobalEnum_descriptor());
builder->AddResolvableEnum(
type_registry->Register(
google::api::expr::test::v1::proto3::GlobalEnum_descriptor());
builder->AddResolvableEnum(google::api::expr::test::v1::proto2::TestAllTypes::
NestedEnum_descriptor());
builder->AddResolvableEnum(google::api::expr::test::v1::proto3::TestAllTypes::
NestedEnum_descriptor());
type_registry->Register(google::api::expr::test::v1::proto2::TestAllTypes::
NestedEnum_descriptor());
type_registry->Register(google::api::expr::test::v1::proto3::TestAllTypes::
NestedEnum_descriptor());
auto register_status = RegisterBuiltinFunctions(builder->GetRegistry());
if (!register_status.ok()) {
std::cerr << "Failed to initialize: " << register_status.ToString()
Expand Down
34 changes: 34 additions & 0 deletions eval/compiler/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ cc_library(
deps = [
":constant_folding",
":qualified_reference_resolver",
":resolver",
"//base:status_macros",
"//eval/eval:comprehension_step",
"//eval/eval:const_value_step",
Expand All @@ -30,6 +31,7 @@ cc_library(
"//eval/eval:jump_step",
"//eval/eval:logic_step",
"//eval/eval:select_step",
"//eval/eval:shadowable_value_step",
"//eval/eval:ternary_step",
"//eval/public:ast_traverse",
"//eval/public:ast_visitor",
Expand Down Expand Up @@ -147,6 +149,7 @@ cc_library(
"qualified_reference_resolver.h",
],
deps = [
":resolver",
"//base:status_macros",
"//eval/eval:const_value_step",
"//eval/eval:expression_build_warning",
Expand All @@ -162,6 +165,21 @@ cc_library(
],
)

cc_library(
name = "resolver",
srcs = ["resolver.cc"],
hdrs = ["resolver.h"],
deps = [
"//eval/public:cel_builtins",
"//eval/public:cel_function_registry",
"//eval/public:cel_type_registry",
"//eval/public:cel_value",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/types:optional",
"@com_google_protobuf//:protobuf",
],
)

cc_test(
name = "qualified_reference_resolver_test",
srcs = [
Expand All @@ -174,6 +192,7 @@ cc_test(
"//eval/public:cel_builtins",
"//eval/public:cel_function",
"//eval/public:cel_function_registry",
"//eval/public:cel_type_registry",
"//testutil:util",
"@com_google_absl//absl/status",
"@com_google_absl//absl/types:optional",
Expand Down Expand Up @@ -203,3 +222,18 @@ cc_test(
"@com_google_protobuf//:protobuf",
],
)

cc_test(
name = "resolver_test",
size = "small",
srcs = ["resolver_test.cc"],
deps = [
":resolver",
"//eval/public:cel_function",
"//eval/public:cel_function_registry",
"//eval/public:cel_type_registry",
"//eval/testutil:test_message_cc_proto",
"@com_google_absl//absl/status",
"@com_google_googletest//:gtest_main",
],
)
Loading

0 comments on commit bca699e

Please # to comment.