From 155ad1e48df48c7a7ba1d33c110fd7218fa9caca Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Fri, 24 Jul 2026 14:07:53 -0700 Subject: [PATCH] [Pratt Parser] Refactor worker methods and parse unary operator chains iteratively. - Extract helper methods (`ParseTernary`, `ParseSelectorChainTail`, `ParseUnaryOpsChain`, `ParseIdentOrCall`, `ParseNegativeIntLiteral`, `ParseNegativeDoubleLiteral`, `BuildBinaryCall`) to reduce the size of the call stack frames - Process prefix unary operator chains (`!`, `-`) iteratively in `ParseUnaryOpsChain` to reduce stack growth on deep chains. PiperOrigin-RevId: 953546973 --- parser/internal/BUILD | 1 + parser/internal/pratt_parser_test.cc | 48 ++++ parser/internal/pratt_parser_worker.h | 334 +++++++++++++++++--------- 3 files changed, 267 insertions(+), 116 deletions(-) diff --git a/parser/internal/BUILD b/parser/internal/BUILD index e3f2b8ed0..bf95d66b7 100644 --- a/parser/internal/BUILD +++ b/parser/internal/BUILD @@ -91,6 +91,7 @@ cc_library( "//internal:strings", "//parser:options", "//parser:parser_interface", + "@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/status:statusor", diff --git a/parser/internal/pratt_parser_test.cc b/parser/internal/pratt_parser_test.cc index 26f849639..2956ec792 100644 --- a/parser/internal/pratt_parser_test.cc +++ b/parser/internal/pratt_parser_test.cc @@ -723,6 +723,18 @@ std::vector GetParserTestCases() { )^#4:Expr.Call# )", }, + TestCase{ + .source = "(((10 - 3) - 2))", + .expected_ast = R"( + _-_( + _-_( + 10^#1:int64#, + 3^#3:int64# + )^#2:Expr.Call#, + 2^#5:int64# + )^#4:Expr.Call# + )", + }, TestCase{ .source = "1 + 2 * 3 - 1 / 2 == 6 % 1", .expected_ast = R"( @@ -747,6 +759,30 @@ std::vector GetParserTestCases() { )^#10:Expr.Call# )", }, + TestCase{ + .source = "(1 + (2 * 3) - (1 / 2)) == (6 % 1)", + .expected_ast = R"( + _==_( + _-_( + _+_( + 1^#1:int64#, + _*_( + 2^#3:int64#, + 3^#5:int64# + )^#4:Expr.Call# + )^#2:Expr.Call#, + _/_( + 1^#7:int64#, + 2^#9:int64# + )^#8:Expr.Call# + )^#6:Expr.Call#, + _%_( + 6^#11:int64#, + 1^#13:int64# + )^#12:Expr.Call# + )^#10:Expr.Call# + )", + }, TestCase{ .source = "1 + 2 * 3 == 7 && true || false", .expected_ast = R"( @@ -1039,6 +1075,18 @@ std::vector GetParserTestCases() { )", .enable_optional_syntax = true, }, + TestCase{ + .source = "(((10 - 3) - 2))", + .expected_ast = R"( + _-_( + _-_( + 10^#1:int64#, + 3^#3:int64# + )^#2:Expr.Call#, + 2^#5:int64# + )^#4:Expr.Call# + )", + }, }; } diff --git a/parser/internal/pratt_parser_worker.h b/parser/internal/pratt_parser_worker.h index ad3bfda2d..8f44979c4 100644 --- a/parser/internal/pratt_parser_worker.h +++ b/parser/internal/pratt_parser_worker.h @@ -25,6 +25,7 @@ #include #include "absl/base/nullability.h" +#include "absl/base/optimization.h" #include "absl/container/flat_hash_map.h" #include "absl/status/statusor.h" #include "absl/strings/ascii.h" @@ -179,7 +180,6 @@ class PrattParserWorker : public ParserWorker { using CelOperator = ::google::api::expr::common::CelOperator; ExprNode ParseExpr(); - // Parses binary operator expressions and ternary conditional expressions // (`? :`) using operator-precedence (Pratt) parsing. Consumes operators from // the token stream whose binding precedence is greater than or equal to @@ -194,6 +194,10 @@ class PrattParserWorker : public ParserWorker { // and `ParseBinary(0)` for false branch `c`. ExprNode ParseBinaryAndTernary(int min_prec); + // Parses ternary conditional expressions (`condition ? true_expr : + // false_expr`). + ExprNode ParseTernary(ExprNode lhs); + // Helper method for parsing a contiguous chain of same-precedence logical // operators (`&&` or `||`) iteratively into a list of terms and operator IDs. // Constructs either a balanced binary AST (`(a && b) && c`) or a single @@ -216,6 +220,13 @@ class PrattParserWorker : public ParserWorker { // then loops iteratively through `.b`, `[0]`, and `.c(x)`. ExprNode ParseSelectorChain(); + // Handler for postfix member, index, receiver method, and + // struct initializer operations (`.field`, `[index]`, `.method(args)`, + // `Type{field: val}`). + // + // Processes continuous postfix operation chains iteratively. + ExprNode ParseSelectorChainTail(ExprNode lhs); + // Parses prefix unary operators (logical NOT `!` and negation `-`). If a // numeric literal immediately follows `-`, folds it directly into a negative // constant node (`-42`, `-3.14`). Otherwise, wraps the operand in a `_!_` or @@ -228,6 +239,9 @@ class PrattParserWorker : public ParserWorker { // wrapping `has(x.y)`. ExprNode ParseUnary(); + // Parses unary operator chains (`!`, `-`). + ExprNode ParseUnaryOpsChain(Token first_op); + // Parses primary leaf expressions (`nud` atomic atoms), including // parenthesized expressions (`(expr)`), literal constants (`null`, `true`, // `false`, numbers, strings, bytes), collection initializers (`[list]`, @@ -250,10 +264,15 @@ class PrattParserWorker : public ParserWorker { ExprNode ParseStruct(int64_t obj_id, absl::string_view struct_name); std::vector ParseArguments(TokenType close_token); ExprNode ParseIntLiteral(); + ExprNode ParseNegativeIntLiteral(int64_t node_id); ExprNode ParseUintLiteral(); ExprNode ParseDoubleLiteral(); + ExprNode ParseNegativeDoubleLiteral(int64_t node_id); ExprNode ParseStringLiteral(); ExprNode ParseBytesLiteral(); + ExprNode BuildBinaryCall(int64_t op_id, absl::string_view op_name, + ExprNode lhs, ExprNode rhs); + ExprNode ParseIdentOrCall(); std::string NormalizeIdent(const Token& tok, bool allow_quoted); std::optional ExtractStructName(const ExprNode& expr); int32_t GetLeftmostPosition(const ExprNode& expr); @@ -304,8 +323,37 @@ ExprNode PrattParserWorker::ParseExpr() { return expr; } +template +ExprNode PrattParserWorker::ParseTernary(ExprNode lhs) { + NextToken(); + int64_t op_id = NextId(); + ExprNode true_expr = ParseBinaryAndTernary(1); + if (!Expect(TokenType::kColon, "expected ':' in conditional expression")) { + return lhs; + } + ExprNode false_expr = ParseBinaryAndTernary(0); + std::vector args; + args.reserve(3); + args.push_back(std::move(lhs)); + args.push_back(std::move(true_expr)); + args.push_back(std::move(false_expr)); + return ast_factory_.NewCall(op_id, CelOperator::CONDITIONAL, std::move(args)); +} + const BinaryOpInfo& GetBinaryOpInfo(TokenType type); +template +ExprNode PrattParserWorker::BuildBinaryCall(int64_t op_id, + absl::string_view op_name, + ExprNode lhs, + ExprNode rhs) { + std::vector args; + args.reserve(2); + args.push_back(std::move(lhs)); + args.push_back(std::move(rhs)); + return ast_factory_.NewCall(op_id, std::string(op_name), std::move(args)); +} + // Parses binary operator expressions and ternary conditional expressions // (`? :`) using Pratt operator-precedence parsing (e.g., `a + b * c`). template @@ -314,21 +362,7 @@ ExprNode PrattParserWorker::ParseBinaryAndTernary(int min_prec) { while (true) { TokenType tok = peek_token_.type; if (tok == TokenType::kQuestion && min_prec <= 0) { - NextToken(); - int64_t op_id = NextId(); - ExprNode true_expr = ParseBinaryAndTernary(1); - if (!Expect(TokenType::kColon, - "expected ':' in conditional expression")) { - return lhs; - } - ExprNode false_expr = ParseBinaryAndTernary(0); - std::vector args; - args.reserve(3); - args.push_back(std::move(lhs)); - args.push_back(std::move(true_expr)); - args.push_back(std::move(false_expr)); - lhs = ast_factory_.NewCall(op_id, CelOperator::CONDITIONAL, - std::move(args)); + lhs = ParseTernary(std::move(lhs)); continue; } @@ -343,12 +377,7 @@ ExprNode PrattParserWorker::ParseBinaryAndTernary(int min_prec) { Token op_tok = NextToken(); int64_t op_id = NextId(op_tok); ExprNode rhs = ParseBinaryAndTernary(op_info.precedence + 1); - std::vector args; - args.reserve(2); - args.push_back(std::move(lhs)); - args.push_back(std::move(rhs)); - lhs = - ast_factory_.NewCall(op_id, std::string(op_info.name), std::move(args)); + lhs = BuildBinaryCall(op_id, op_info.name, std::move(lhs), std::move(rhs)); } return lhs; } @@ -371,11 +400,21 @@ ExprNode PrattParserWorker::ParseBalancedLogicalChain( options_.enable_variadic_logical_operators); } -// Parses prefix and postfix member/indexing operations iteratively -// (e.g., `!a.b[0].c(x)`). template ExprNode PrattParserWorker::ParseSelectorChain() { ExprNode lhs = ParseUnary(); + TokenType tok = peek_token_.type; + if (tok == TokenType::kDot || tok == TokenType::kLeftBracket || + tok == TokenType::kLeftBrace) { + return ParseSelectorChainTail(std::move(lhs)); + } + return lhs; +} + +// Parses prefix and postfix member/indexing operations iteratively +// (e.g., `!a.b[0].c(x)`). +template +ExprNode PrattParserWorker::ParseSelectorChainTail(ExprNode lhs) { while (true) { TokenType tok = peek_token_.type; if (tok == TokenType::kDot) { @@ -456,73 +495,124 @@ ExprNode PrattParserWorker::ParseSelectorChain() { return lhs; } -// Parses prefix unary operators (`!`, `-`) and folds negative numeric literals -// (e.g., `-42`, `!has(x.y)`). template -ExprNode PrattParserWorker::ParseUnary() { - TokenType tok = peek_token_.type; - if (tok == TokenType::kExclamation) { +ExprNode PrattParserWorker::ParseUnaryOpsChain(Token first_op) { + struct UnaryOpInfo { + TokenType type; + int64_t id; + }; + std::vector ops; + ops.push_back({first_op.type, NextId(first_op)}); + + while (peek_token_.type == TokenType::kExclamation || + peek_token_.type == TokenType::kMinus) { Token op = NextToken(); - int64_t op_id = NextId(op); - ExprNode operand = ParseSelectorChain(); + ops.push_back({op.type, NextId(op)}); + } + + ExprNode operand; + if (!ops.empty() && ops.back().type == TokenType::kMinus) { + if (peek_token_.type == TokenType::kInt) { + int64_t op_id = ops.back().id; + ops.pop_back(); + operand = ParseNegativeIntLiteral(op_id); + } else if (peek_token_.type == TokenType::kFloat) { + int64_t op_id = ops.back().id; + ops.pop_back(); + operand = ParseNegativeDoubleLiteral(op_id); + } else { + operand = ParseSelectorChain(); + } + } else { + operand = ParseSelectorChain(); + } + + for (int i = static_cast(ops.size()) - 1; i >= 0; --i) { std::vector args; args.push_back(std::move(operand)); - return ast_factory_.NewCall(op_id, std::string(CelOperator::LOGICAL_NOT), - std::move(args)); + absl::string_view op_name = (ops[i].type == TokenType::kExclamation) + ? CelOperator::LOGICAL_NOT + : CelOperator::NEGATE; + operand = + ast_factory_.NewCall(ops[i].id, std::string(op_name), std::move(args)); } - if (tok == TokenType::kMinus) { - Token op = NextToken(); + + return operand; +} + +// Parses prefix unary operators (`!`, `-`) iteratively and folds negative +// numeric literals (e.g., `-42`, `!has(x.y)`). +template +ExprNode PrattParserWorker::ParseUnary() { + TokenType tok = peek_token_.type; + if (tok != TokenType::kExclamation && tok != TokenType::kMinus) { + return ParsePrimary(); + } + + Token op = NextToken(); + TokenType op_type = op.type; + if (peek_token_.type == TokenType::kExclamation || + peek_token_.type == TokenType::kMinus) { + return ParseUnaryOpsChain(op); + } + + if (op_type == TokenType::kMinus) { if (peek_token_.type == TokenType::kInt) { - Token lit_tok = NextToken(); - std::string text = GetTokenText(lit_tok); - int64_t int_val = 0; - bool success = false; - if (absl::StartsWith(text, "0x") || absl::StartsWith(text, "0X")) { - uint64_t uint_val = 0; - if (absl::SimpleHexAtoi(text, &uint_val)) { - if (uint_val <= uint64_t{0x8000000000000000}) { - if (uint_val == uint64_t{0x8000000000000000}) { - int_val = std::numeric_limits::min(); - } else { - int_val = -static_cast(uint_val); - } - success = true; - } - } - } else { - if (absl::SimpleAtoi(text, &int_val)) { - int_val = -int_val; - success = true; - } else { - // Separately address -2^63, which is not representable as -(2^63) - std::string val = absl::StrCat("-", text); - success = absl::SimpleAtoi(val, &int_val); - } - } - if (success) { - return ast_factory_.NewIntConst(NextId(op.start), int_val); - } - ReportError(lit_tok, "invalid int literal"); - return ast_factory_.NewUnspecified(NextId(lit_tok)); + return ParseNegativeIntLiteral(NextId(op)); } if (peek_token_.type == TokenType::kFloat) { - Token lit_tok = NextToken(); - double double_val = 0.0; - if (absl::SimpleAtod(GetTokenText(lit_tok), &double_val)) { - return ast_factory_.NewDoubleConst(NextId(op.start), -double_val); - } - ReportError(lit_tok, "invalid double literal"); - return ast_factory_.NewUnspecified(NextId(lit_tok)); + return ParseNegativeDoubleLiteral(NextId(op)); } - // Regular negate call - int64_t op_id = NextId(op); - ExprNode operand = ParseSelectorChain(); - std::vector args; - args.push_back(std::move(operand)); - return ast_factory_.NewCall(op_id, std::string(CelOperator::NEGATE), - std::move(args)); } - return ParsePrimary(); + + int64_t op_id = NextId(op); + ExprNode operand = ParseSelectorChain(); + std::vector args; + args.push_back(std::move(operand)); + absl::string_view op_name = (op_type == TokenType::kExclamation) + ? CelOperator::LOGICAL_NOT + : CelOperator::NEGATE; + return ast_factory_.NewCall(op_id, std::string(op_name), std::move(args)); +} + +// Parses identifiers (e.g., `foo`, `.foo`) and global function or macro calls +// (e.g., `foo(args)`, `has(x.y)`). +template +ExprNode PrattParserWorker::ParseIdentOrCall() { + TokenType tok_type = peek_token_.type; + bool leading_dot = false; + Token first_tok = peek_token_; + if (tok_type == TokenType::kDot) { + NextToken(); + leading_dot = true; + } + Token id_tok = NextToken(); + if (id_tok.type != TokenType::kIdent && + id_tok.type != TokenType::kReservedWord) { + if (id_tok.type != TokenType::kError) { + ReportError(id_tok, "expected identifier"); + } + return ast_factory_.NewUnspecified(NextId(id_tok)); + } + std::string id_text = NormalizeIdent(id_tok, /*allow_quoted=*/false); + if (ABSL_PREDICT_FALSE(id_tok.type == TokenType::kReservedWord)) { + if (cel::internal::LexisIsReserved(id_text)) { + ReportError(id_tok, absl::StrFormat("reserved identifier: %s", id_text)); + } + } + std::string name = + leading_dot ? absl::StrCat(".", id_text) : std::string(id_text); + int64_t id = NextId(leading_dot ? first_tok : id_tok); + if (peek_token_.type == TokenType::kLeftParen) { + NextToken(); + std::vector args = ParseArguments(TokenType::kRightParen); + if (auto expanded = TryExpandMacro(id, name, nullptr, args); + expanded.has_value()) { + return std::move(*expanded); + } + return ast_factory_.NewCall(id, name, std::move(args)); + } + return ast_factory_.NewIdent(id, std::move(name)); } // Parses primary leaf expressions, including parenthesized expressions @@ -560,41 +650,7 @@ ExprNode PrattParserWorker::ParsePrimary() { expr = ParseMap(); } else if (tok_type == TokenType::kDot || tok_type == TokenType::kIdent || tok_type == TokenType::kReservedWord) { - bool leading_dot = false; - Token first_tok = peek_token_; - if (tok_type == TokenType::kDot) { - NextToken(); - leading_dot = true; - } - Token id_tok = NextToken(); - if (id_tok.type != TokenType::kIdent && - id_tok.type != TokenType::kReservedWord) { - if (id_tok.type != TokenType::kError) { - ReportError(id_tok, "expected identifier"); - } - expr = ast_factory_.NewUnspecified(NextId(id_tok)); - } else { - std::string id_text = NormalizeIdent(id_tok, /*allow_quoted=*/false); - if (cel::internal::LexisIsReserved(id_text)) { - ReportError(id_tok, - absl::StrFormat("reserved identifier: %s", id_text)); - } - std::string name = - leading_dot ? absl::StrCat(".", id_text) : std::string(id_text); - int64_t id = NextId(leading_dot ? first_tok : id_tok); - if (peek_token_.type == TokenType::kLeftParen) { - NextToken(); - std::vector args = ParseArguments(TokenType::kRightParen); - if (auto expanded = TryExpandMacro(id, name, nullptr, args); - expanded.has_value()) { - expr = std::move(*expanded); - } else { - expr = ast_factory_.NewCall(id, name, std::move(args)); - } - } else { - expr = ast_factory_.NewIdent(id, std::move(name)); - } - } + expr = ParseIdentOrCall(); } else { Token bad_tok = NextToken(); if (bad_tok.type != TokenType::kError) { @@ -757,6 +813,40 @@ ExprNode PrattParserWorker::ParseIntLiteral() { return ast_factory_.NewUnspecified(NextId(tok)); } +template +ExprNode PrattParserWorker::ParseNegativeIntLiteral(int64_t node_id) { + Token lit_tok = NextToken(); + std::string text = GetTokenText(lit_tok); + int64_t int_val = 0; + bool success = false; + if (absl::StartsWith(text, "0x") || absl::StartsWith(text, "0X")) { + uint64_t uint_val = 0; + if (absl::SimpleHexAtoi(text, &uint_val)) { + if (uint_val <= uint64_t{0x8000000000000000}) { + if (uint_val == uint64_t{0x8000000000000000}) { + int_val = std::numeric_limits::min(); + } else { + int_val = -static_cast(uint_val); + } + success = true; + } + } + } else if (absl::SimpleAtoi(text, &int_val)) { + int_val = -int_val; + success = true; + } else { + // Separately handle -2^63, which is not representable as -(2^63) + std::string val = absl::StrCat("-", text); + success = absl::SimpleAtoi(val, &int_val); + } + + if (success) { + return ast_factory_.NewIntConst(node_id, int_val); + } + ReportError(lit_tok, "invalid int literal"); + return ast_factory_.NewUnspecified(NextId(lit_tok)); +} + // Parses unsigned ints (e.g., `42u`, `0x1Au`). template ExprNode PrattParserWorker::ParseUintLiteral() { @@ -790,6 +880,18 @@ ExprNode PrattParserWorker::ParseDoubleLiteral() { return ast_factory_.NewUnspecified(NextId(tok)); } +template +ExprNode PrattParserWorker::ParseNegativeDoubleLiteral( + int64_t node_id) { + Token lit_tok = NextToken(); + double double_val = 0.0; + if (absl::SimpleAtod(GetTokenText(lit_tok), &double_val)) { + return ast_factory_.NewDoubleConst(node_id, -double_val); + } + ReportError(lit_tok, "invalid double literal"); + return ast_factory_.NewUnspecified(NextId(lit_tok)); +} + // Parses string literals (e.g., `"hello"`, `'world'`, `"""multi"""`). template ExprNode PrattParserWorker::ParseStringLiteral() {