Skip to content
Merged
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
4 changes: 2 additions & 2 deletions conformance/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ _TESTS_TO_SKIP_LEGACY_DASHBOARD = [
]

# Generates a bunch of `cc_test` whose names follow the pattern
# `conformance_(...)_{arena|refcount}_{optimized|unoptimized}_{recursive|iterative}`.
# `conformance_(...)_{pratt|antlr}_{optimized|unoptimized}_{recursive|iterative}`.
gen_conformance_tests(
name = "conformance_parse_only",
data = _ALL_TESTS,
Expand Down Expand Up @@ -317,7 +317,7 @@ gen_conformance_tests(
)

# Generates a bunch of `cc_test` whose names follow the pattern
# `conformance_dashboard_..._{arena|refcount}_{optimized|unoptimized}_{recursive|iterative}`.
# `conformance_dashboard_..._{pratt|antlr}_{optimized|unoptimized}_{recursive|iterative}`.
gen_conformance_tests(
name = "conformance_dashboard_parse_only",
dashboard = True,
Expand Down
51 changes: 29 additions & 22 deletions conformance/run.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,17 @@ def _expand_tests_to_skip(tests_to_skip):
result.append(test_to_skip[0:slash] + part)
return result

def _conformance_test_name(name, optimize, recursive):
def _conformance_test_name(name, pratt, optimize, recursive):
return "_".join(
[
name,
"pratt" if pratt else "antlr",
"optimized" if optimize else "unoptimized",
"recursive" if recursive else "iterative",
],
)

def _conformance_test_args(modern, optimize, recursive, select_opt, skip_check, dashboard, enable_variadic_logical_operators):
def _conformance_test_args(modern, optimize, recursive, select_opt, skip_check, dashboard, enable_variadic_logical_operators, pratt):
args = []
if modern:
args.append("--modern")
Expand All @@ -74,12 +75,16 @@ def _conformance_test_args(modern, optimize, recursive, select_opt, skip_check,
args.append("--dashboard")
if enable_variadic_logical_operators:
args.append("--enable_variadic_logical_operators")
if pratt:
args.append("--enable_pratt_parser")
else:
args.append("--noenable_pratt_parser")
return args

def _conformance_test(name, data, modern, optimize, recursive, select_opt, skip_check, skip_tests, tags, dashboard, enable_variadic_logical_operators):
def _conformance_test(name, data, modern, optimize, recursive, select_opt, skip_check, skip_tests, tags, dashboard, enable_variadic_logical_operators, pratt):
cc_test(
name = _conformance_test_name(name, optimize, recursive),
args = _conformance_test_args(modern, optimize, recursive, select_opt, skip_check, dashboard, enable_variadic_logical_operators) + ["$(rlocationpath {})".format(test) for test in data],
name = _conformance_test_name(name, pratt, optimize, recursive),
args = _conformance_test_args(modern, optimize, recursive, select_opt, skip_check, dashboard, enable_variadic_logical_operators, pratt) + ["$(rlocationpath {})".format(test) for test in data],
env = select(
{
"@platforms//os:windows": {"CEL_SKIP_TESTS": ",".join(skip_tests + _TESTS_TO_SKIP_WINDOWS)},
Expand Down Expand Up @@ -108,23 +113,25 @@ def gen_conformance_tests(name, data, modern = False, checked = False, select_op
"""
skip_check = not checked
tests = []
for optimize in (True, False):
for recursive in (True, False):
test_name = _conformance_test_name(name, optimize, recursive)
tests.append(test_name)
_conformance_test(
name,
data,
modern = modern,
optimize = optimize,
recursive = recursive,
select_opt = select_opt,
skip_check = skip_check,
skip_tests = _expand_tests_to_skip(skip_tests),
tags = tags,
dashboard = dashboard,
enable_variadic_logical_operators = enable_variadic_logical_operators,
)
for pratt in (True, False):
for optimize in (True, False):
for recursive in (True, False):
test_name = _conformance_test_name(name, pratt, optimize, recursive)
tests.append(test_name)
_conformance_test(
name,
data,
modern = modern,
optimize = optimize,
recursive = recursive,
select_opt = select_opt,
skip_check = skip_check,
skip_tests = _expand_tests_to_skip(skip_tests),
tags = tags,
dashboard = dashboard,
enable_variadic_logical_operators = enable_variadic_logical_operators,
pratt = pratt,
)
native.test_suite(
name = name,
tests = tests,
Expand Down
3 changes: 3 additions & 0 deletions conformance/run.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ ABSL_FLAG(bool, select_optimization, false, "Enable select optimization.");
ABSL_FLAG(bool, enable_variadic_logical_operators, false,
"Enable parsing logical AND & OR operators as a single flat variadic "
"call.");
ABSL_FLAG(bool, enable_pratt_parser, true,
"Enable manual (Pratt) parser instead of ANTLR parser.");

namespace {

Expand Down Expand Up @@ -266,6 +268,7 @@ NewConformanceServiceFromFlags() {
.select_optimization = absl::GetFlag(FLAGS_select_optimization),
.enable_variadic_logical_operators =
absl::GetFlag(FLAGS_enable_variadic_logical_operators),
.enable_pratt_parser = absl::GetFlag(FLAGS_enable_pratt_parser),
});
ABSL_CHECK_OK(status_or_service);
return std::shared_ptr<cel_conformance::ConformanceServiceInterface>(
Expand Down
39 changes: 24 additions & 15 deletions conformance/service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,16 @@ cel::expr::Expr ExtractExpr(
absl::Status LegacyParse(const conformance::v1alpha1::ParseRequest& request,
conformance::v1alpha1::ParseResponse& response,
bool enable_optional_syntax,
bool enable_variadic_logical_operators) {
bool enable_variadic_logical_operators,
bool enable_pratt_parser) {
if (request.cel_source().empty()) {
return absl::InvalidArgumentError("no source code");
}
cel::ParserOptions options;
options.enable_optional_syntax = enable_optional_syntax;
options.enable_quoted_identifiers = true;
options.enable_variadic_logical_operators = enable_variadic_logical_operators;
options.enable_pratt_parser = enable_pratt_parser;
cel::MacroRegistry macros;
CEL_RETURN_IF_ERROR(cel::RegisterStandardMacros(macros, options));
CEL_RETURN_IF_ERROR(
Expand Down Expand Up @@ -240,7 +242,7 @@ class LegacyConformanceServiceImpl : public ConformanceServiceInterface {
public:
static absl::StatusOr<std::unique_ptr<LegacyConformanceServiceImpl>> Create(
bool optimize, bool recursive, bool select_optimization,
bool enable_variadic_logical_operators) {
bool enable_variadic_logical_operators, bool enable_pratt_parser) {
static auto* constant_arena = new Arena();

google::protobuf::LinkMessageReflection<
Expand Down Expand Up @@ -319,14 +321,15 @@ class LegacyConformanceServiceImpl : public ConformanceServiceInterface {
builder->GetRegistry(), options));

return absl::WrapUnique(new LegacyConformanceServiceImpl(
std::move(builder), enable_variadic_logical_operators));
std::move(builder), enable_variadic_logical_operators,
enable_pratt_parser));
}

void Parse(const conformance::v1alpha1::ParseRequest& request,
conformance::v1alpha1::ParseResponse& response) override {
auto status =
LegacyParse(request, response, /*enable_optional_syntax=*/false,
enable_variadic_logical_operators_);
enable_variadic_logical_operators_, enable_pratt_parser_);
if (!status.ok()) {
auto* issue = response.add_issues();
issue->set_code(ToGrpcCode(status.code()));
Expand Down Expand Up @@ -425,19 +428,22 @@ class LegacyConformanceServiceImpl : public ConformanceServiceInterface {

private:
LegacyConformanceServiceImpl(std::unique_ptr<CelExpressionBuilder> builder,
bool enable_variadic_logical_operators)
bool enable_variadic_logical_operators,
bool enable_pratt_parser)
: builder_(std::move(builder)),
enable_variadic_logical_operators_(enable_variadic_logical_operators) {}
enable_variadic_logical_operators_(enable_variadic_logical_operators),
enable_pratt_parser_(enable_pratt_parser) {}

std::unique_ptr<CelExpressionBuilder> builder_;
bool enable_variadic_logical_operators_;
bool enable_pratt_parser_;
};

class ModernConformanceServiceImpl : public ConformanceServiceInterface {
public:
static absl::StatusOr<std::unique_ptr<ModernConformanceServiceImpl>> Create(
bool optimize, bool recursive, bool select_optimization,
bool enable_variadic_logical_operators) {
bool enable_variadic_logical_operators, bool enable_pratt_parser) {
google::protobuf::LinkMessageReflection<
cel::expr::conformance::proto3::TestAllTypes>();
google::protobuf::LinkMessageReflection<
Expand Down Expand Up @@ -479,9 +485,9 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface {
options.max_recursion_depth = 48;
}

return absl::WrapUnique(
new ModernConformanceServiceImpl(options, optimize, select_optimization,
enable_variadic_logical_operators));
return absl::WrapUnique(new ModernConformanceServiceImpl(
options, optimize, select_optimization,
enable_variadic_logical_operators, enable_pratt_parser));
}

absl::StatusOr<std::unique_ptr<const cel::Runtime>> Setup(
Expand Down Expand Up @@ -538,7 +544,7 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface {
conformance::v1alpha1::ParseResponse& response) override {
auto status =
LegacyParse(request, response, /*enable_optional_syntax=*/true,
enable_variadic_logical_operators_);
enable_variadic_logical_operators_, enable_pratt_parser_);
if (!status.ok()) {
auto* issue = response.add_issues();
issue->set_code(ToGrpcCode(status.code()));
Expand Down Expand Up @@ -630,11 +636,13 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface {
ModernConformanceServiceImpl(const RuntimeOptions& options,
bool enable_optimizations,
bool enable_select_optimization,
bool enable_variadic_logical_operators)
bool enable_variadic_logical_operators,
bool enable_pratt_parser)
: options_(options),
enable_optimizations_(enable_optimizations),
enable_select_optimization_(enable_select_optimization),
enable_variadic_logical_operators_(enable_variadic_logical_operators) {}
enable_variadic_logical_operators_(enable_variadic_logical_operators),
enable_pratt_parser_(enable_pratt_parser) {}

static absl::StatusOr<std::unique_ptr<cel::TraceableProgram>> Plan(
const cel::Runtime& runtime,
Expand Down Expand Up @@ -666,6 +674,7 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface {
bool enable_optimizations_;
bool enable_select_optimization_;
bool enable_variadic_logical_operators_;
bool enable_pratt_parser_;
};

} // namespace
Expand All @@ -679,11 +688,11 @@ NewConformanceService(const ConformanceServiceOptions& options) {
if (options.modern) {
return google::api::expr::runtime::ModernConformanceServiceImpl::Create(
options.optimize, options.recursive, options.select_optimization,
options.enable_variadic_logical_operators);
options.enable_variadic_logical_operators, options.enable_pratt_parser);
} else {
return google::api::expr::runtime::LegacyConformanceServiceImpl::Create(
options.optimize, options.recursive, options.select_optimization,
options.enable_variadic_logical_operators);
options.enable_variadic_logical_operators, options.enable_pratt_parser);
}
}

Expand Down
1 change: 1 addition & 0 deletions conformance/service.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ struct ConformanceServiceOptions {
bool recursive;
bool select_optimization;
bool enable_variadic_logical_operators = false;
bool enable_pratt_parser = true;
};

absl::StatusOr<std::unique_ptr<ConformanceServiceInterface>>
Expand Down
4 changes: 3 additions & 1 deletion parser/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ cc_library(
":source_factory",
"//common:ast",
"//common:constant",
"//common:expr",
"//common:expr_factory",
"//common:operators",
"//common:source",
Expand All @@ -50,8 +51,8 @@ cc_library(
"//internal:lexis",
"//internal:status_macros",
"//internal:strings",
"//internal:utf8",
"//parser/internal:cel_cc_parser",
"//parser/internal:pratt_parser",
"@antlr4-cpp-runtime",
"@com_google_absl//absl/base:core_headers",
"@com_google_absl//absl/cleanup",
Expand All @@ -60,6 +61,7 @@ cc_library(
"@com_google_absl//absl/container:flat_hash_set",
"@com_google_absl//absl/functional:overload",
"@com_google_absl//absl/log:absl_check",
"@com_google_absl//absl/log:check",
"@com_google_absl//absl/memory",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
Expand Down
1 change: 1 addition & 0 deletions parser/internal/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ cc_library(
"//parser:macro_registry",
"//parser:options",
"//parser:parser_interface",
"@com_google_absl//absl/algorithm:container",
"@com_google_absl//absl/base:nullability",
"@com_google_absl//absl/cleanup",
"@com_google_absl//absl/container:flat_hash_map",
Expand Down
4 changes: 2 additions & 2 deletions parser/internal/ast_factory_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,8 @@ class TestMacroExprExpanderSupport
public:
int64_t NextId() override { return 42; }
int64_t CopyId(int64_t id) override { return id; }
cel::Expr ReportError(std::string_view) override { return cel::Expr(); }
cel::Expr ReportErrorAt(const cel::Expr&, std::string_view) override {
cel::Expr ReportError(absl::string_view) override { return cel::Expr(); }
cel::Expr ReportErrorAt(const cel::Expr&, absl::string_view) override {
return cel::Expr();
}
};
Expand Down
27 changes: 21 additions & 6 deletions parser/internal/pratt_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@

#include "parser/internal/pratt_parser.h"

#include <algorithm>
#include <cstdint>
#include <memory>
#include <string>
#include <string_view>
#include <utility>
#include <vector>

#include "absl/algorithm/container.h"
#include "absl/base/nullability.h"
#include "absl/cleanup/cleanup.h"
#include "absl/container/flat_hash_map.h"
Expand Down Expand Up @@ -158,21 +160,34 @@ template class PrattParserWorker<cel::Expr>;
absl::StatusOr<std::unique_ptr<cel::Ast>> PrattParserImpl::ParseImpl(
const cel::Source& source,
std::vector<cel::ParseIssue>* absl_nullable parse_issues) const {
if (source.content().size() > options_.expression_size_codepoint_limit) {
return PrattParseImpl(source, macro_registry_, options_, parse_issues);
}

absl::StatusOr<std::unique_ptr<cel::Ast>> PrattParseImpl(
const cel::Source& source, const cel::MacroRegistry& registry,
const ParserOptions& options, std::vector<cel::ParseIssue>* parse_issues) {
if (source.content().size() > options.expression_size_codepoint_limit) {
return absl::InvalidArgumentError(absl::StrFormat(
"expression size exceeds codepoint limit: %zu > %d",
source.content().size(), options_.expression_size_codepoint_limit));
"expression size exceeds codepoint limit. input size: %zu, limit: %d",
source.content().size(), options.expression_size_codepoint_limit));
}
std::vector<cel::ParseIssue> issues;
AstFactory factory(&macro_registry_);
PrattParserWorker<cel::Expr> worker(source, options_, &issues, factory);
AstFactory factory(&registry);
PrattParserWorker<cel::Expr> worker(source, options, &issues, factory);
Expr expr = worker.Parse();
if (worker.is_recursion_limit_exceeded()) {
return absl::CancelledError(
absl::StrFormat("Expression recursion limit exceeded. limit: %d",
options_.max_recursion_depth));
options.max_recursion_depth));
}
if (worker.has_errors()) {
absl::c_stable_sort(
issues, [](const cel::ParseIssue& lhs, const cel::ParseIssue& rhs) {
if (lhs.location().line != rhs.location().line) {
return lhs.location().line < rhs.location().line;
}
return lhs.location().column < rhs.location().column;
});
std::string err_msg = FormatIssues(source, issues);
if (parse_issues != nullptr) {
parse_issues->swap(issues);
Expand Down
5 changes: 5 additions & 0 deletions parser/internal/pratt_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ class PrattParserImpl final : public cel::Parser {
absl::flat_hash_set<std::string> library_ids_;
};

absl::StatusOr<std::unique_ptr<cel::Ast>> PrattParseImpl(
const cel::Source& source, const cel::MacroRegistry& registry,
const ParserOptions& options,
std::vector<cel::ParseIssue>* parse_issues = nullptr);

class PrattParserBuilderImpl final : public cel::ParserBuilder {
public:
explicit PrattParserBuilderImpl(const cel::ParserOptions& options)
Expand Down
4 changes: 2 additions & 2 deletions parser/internal/pratt_parser_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1810,10 +1810,10 @@ TEST(PrattParserErrorRecoveryTest, ErrorRecoveryLimitOne) {
EXPECT_THAT(result, StatusIs(absl::StatusCode::kInvalidArgument));
ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource("......"));
EXPECT_EQ(FormatIssues(*source, issues),
"ERROR: <input>:-1:0: Error recovery limit (1) exceeded\n"
"ERROR: <input>:1:2: expected identifier\n"
" | ......\n"
" | .^\n"
"ERROR: <input>:-1:0: Error recovery limit (1) exceeded");
" | .^");
}

} // namespace
Expand Down
Loading
Loading