Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions checker/internal/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ cc_library(
"//common:container",
"//common:decl",
"//common:expr",
"//common:standard_definitions",
"//common:type",
"//common:type_kind",
"//internal:lexis",
Expand Down Expand Up @@ -238,6 +239,7 @@ cc_library(
deps = [
":format_type_name",
"//common:decl",
"//common:standard_definitions",
"//common:type",
"//common:type_kind",
"@com_google_absl//absl/container:flat_hash_map",
Expand Down
7 changes: 6 additions & 1 deletion checker/internal/type_checker_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
#include "common/constant.h"
#include "common/decl.h"
#include "common/expr.h"
#include "common/standard_definitions.h"
#include "common/type.h"
#include "common/type_kind.h"
#include "internal/status_macros.h"
Expand Down Expand Up @@ -894,8 +895,12 @@ const FunctionDecl* ResolveVisitor::ResolveFunctionCallShape(
if (decl == nullptr) {
return true;
}
bool is_logical_op = (candidate == cel::StandardFunctions::kAnd ||
candidate == cel::StandardFunctions::kOr) &&
arg_count >= 2;
for (const auto& ovl : decl->overloads()) {
if (ovl.member() == is_receiver && ovl.args().size() == arg_count) {
if (ovl.member() == is_receiver &&
(ovl.args().size() == arg_count || is_logical_op)) {
return false;
}
}
Expand Down
88 changes: 88 additions & 0 deletions checker/internal/type_checker_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
#include "cel/expr/conformance/proto3/test_all_types.pb.h"
#include "google/protobuf/arena.h"
#include "google/protobuf/message.h"
#include "google/protobuf/text_format.h"

namespace cel {
namespace checker_internal {
Expand Down Expand Up @@ -1471,6 +1472,93 @@ TEST(TypeCheckerImplTest, TypeInferredFromStructCreation) {
std::make_unique<AstType>(DynTypeSpec())))))));
}

struct VariadicLogicalCheckerTestCase {
std::string expr;
};

class VariadicLogicalCheckerTest
: public testing::TestWithParam<VariadicLogicalCheckerTestCase> {};

TEST_P(VariadicLogicalCheckerTest, Check) {
const auto& test_case = GetParam();

auto builder = cel::NewParserBuilder();
builder->GetOptions().enable_variadic_logical_operators = true;
ASSERT_OK_AND_ASSIGN(auto parser, std::move(*builder).Build());
ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource(test_case.expr));
ASSERT_OK_AND_ASSIGN(auto parsed_ast, parser->Parse(*source));

google::protobuf::Arena arena;
TypeCheckEnv env(GetSharedTestingDescriptorPool());
ASSERT_THAT(RegisterMinimalBuiltins(&arena, env), IsOk());
TypeCheckerImpl impl(std::move(env));
auto checker_builder = impl.ToBuilder();
ASSERT_THAT(checker_builder->AddVariable(MakeVariableDecl("a", BoolType())),
IsOk());
ASSERT_THAT(checker_builder->AddVariable(MakeVariableDecl("b", BoolType())),
IsOk());
ASSERT_THAT(checker_builder->AddVariable(MakeVariableDecl("c", BoolType())),
IsOk());
ASSERT_THAT(checker_builder->AddVariable(MakeVariableDecl("d", BoolType())),
IsOk());
ASSERT_THAT(checker_builder->AddVariable(MakeVariableDecl("e", BoolType())),
IsOk());

ASSERT_OK_AND_ASSIGN(auto checker, checker_builder->Build());
ASSERT_OK_AND_ASSIGN(ValidationResult result,
checker->Check(std::move(parsed_ast)));

ASSERT_TRUE(result.IsValid())
<< absl::StrJoin(result.GetIssues(), "\n",
[](std::string* out, const TypeCheckIssue& issue) {
absl::StrAppend(out, issue.message());
});

ASSERT_OK_AND_ASSIGN(std::unique_ptr<Ast> checked_ast, result.ReleaseAst());
EXPECT_THAT(checked_ast->type_map(),
Contains(Pair(checked_ast->root_expr().id(),
Eq(AstType(PrimitiveType::kBool)))));
}

INSTANTIATE_TEST_SUITE_P(
VariadicLogicalChecker, VariadicLogicalCheckerTest,
testing::Values(VariadicLogicalCheckerTestCase{"true && false && true"},
VariadicLogicalCheckerTestCase{"a && b && c && d"},
VariadicLogicalCheckerTestCase{"a || b || c || d"},
VariadicLogicalCheckerTestCase{"a && b && (c || d || e)"},
VariadicLogicalCheckerTestCase{"a && b && c"},
VariadicLogicalCheckerTestCase{"a || b || c"},
VariadicLogicalCheckerTestCase{"[a, b, c].exists(x, x)"},
VariadicLogicalCheckerTestCase{"[a, b, c].all(x, x)"}));

TEST(TypeCheckerImplTest, VariadicLogicalOperatorsError) {
cel::expr::ParsedExpr parsed_expr;
ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(
R"pb(
expr {
call_expr {
function: "_&&_"
args { const_expr { bool_value: true } }
}
}
)pb",
&parsed_expr));
ASSERT_OK_AND_ASSIGN(auto parsed_ast,
cel::CreateAstFromParsedExpr(parsed_expr));

google::protobuf::Arena arena;
TypeCheckEnv env(GetSharedTestingDescriptorPool());
ASSERT_THAT(RegisterMinimalBuiltins(&arena, env), IsOk());
TypeCheckerImpl impl(std::move(env));
ASSERT_OK_AND_ASSIGN(ValidationResult result,
impl.Check(std::move(parsed_ast)));

EXPECT_FALSE(result.IsValid());
EXPECT_THAT(
result.GetIssues(),
Contains(IsIssueWithSubstring(Severity::kError, "undeclared reference")));
}

TEST(TypeCheckerImplTest, ExpectedTypeMatches) {
google::protobuf::Arena arena;
TypeCheckEnv env(GetSharedTestingDescriptorPool());
Expand Down
16 changes: 12 additions & 4 deletions checker/internal/type_inference_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "absl/types/span.h"
#include "checker/internal/format_type_name.h"
#include "common/decl.h"
#include "common/standard_definitions.h"
#include "common/type.h"
#include "common/type_kind.h"

Expand Down Expand Up @@ -537,21 +538,28 @@ TypeInferenceContext::ResolveOverload(const FunctionDecl& decl,
bool is_receiver) {
std::optional<Type> result_type;

bool is_logical_op = (decl.name() == cel::StandardFunctions::kAnd ||
decl.name() == cel::StandardFunctions::kOr) &&
argument_types.size() >= 2;

std::vector<OverloadDecl> matching_overloads;
for (const auto& ovl : decl.overloads()) {
if (ovl.member() != is_receiver ||
argument_types.size() != ovl.args().size()) {
(!is_logical_op && argument_types.size() != ovl.args().size())) {
continue;
}

auto call_type_instance = InstantiateFunctionOverload(*this, ovl);
ABSL_DCHECK_EQ(argument_types.size(),
call_type_instance.param_types.size());
if (!is_logical_op) {
ABSL_DCHECK_EQ(argument_types.size(),
call_type_instance.param_types.size());
}
bool is_match = true;
AssignabilityContext assignability_context = CreateAssignabilityContext();
for (int i = 0; i < argument_types.size(); ++i) {
int param_index = is_logical_op ? 0 : i;
if (!assignability_context.IsAssignable(
argument_types[i], call_type_instance.param_types[i])) {
argument_types[i], call_type_instance.param_types[param_index])) {
is_match = false;
break;
}
Expand Down
Loading