Skip to content

[Pratt parser] Align Pratt parser AST node ID generation with the standard parser. #2241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
copybara-service merged 1 commit into from
Aug 31, 2026
Merged

[Pratt parser] Align Pratt parser AST node ID generation with the standard parser. #2241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 8 additions & 8 deletions parser/internal/pratt_parser_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ std::vector<TestCase> GetParserTestCases() {
TestCase{
.source = "- -1",
.expected_ast = R"(
1^#3:int64#
1^#1:int64#
)",
},
TestCase{
Expand All @@ -452,7 +452,7 @@ std::vector<TestCase> GetParserTestCases() {
.source = "---a",
.expected_ast = R"(
-_(
a^#4:Expr.Ident#
a^#2:Expr.Ident#
)^#1:Expr.Call#
)",
},
Expand Down Expand Up @@ -1082,17 +1082,17 @@ std::vector<TestCase> GetParserTestCases() {
.source = "{'key': 'value', 'num': 42}",
.expected_ast = R"(
{
"key"^#2:string#:"value"^#4:string#^#3:Expr.CreateStruct.Entry#,
"num"^#5:string#:42^#7:int64#^#6:Expr.CreateStruct.Entry#
"key"^#3:string#:"value"^#4:string#^#2:Expr.CreateStruct.Entry#,
"num"^#6:string#:42^#7:int64#^#5:Expr.CreateStruct.Entry#
}^#1:Expr.CreateMap#
)",
},
TestCase{
.source = "{?'key': 'value', 'num': 42}",
.expected_ast = R"(
{
?"key"^#2:string#:"value"^#4:string#^#3:Expr.CreateStruct.Entry#,
"num"^#5:string#:42^#7:int64#^#6:Expr.CreateStruct.Entry#
?"key"^#3:string#:"value"^#4:string#^#2:Expr.CreateStruct.Entry#,
"num"^#6:string#:42^#7:int64#^#5:Expr.CreateStruct.Entry#
}^#1:Expr.CreateMap#
)",
.enable_optional_syntax = true,
Expand All @@ -1101,8 +1101,8 @@ std::vector<TestCase> GetParserTestCases() {
.source = "{foo: 5, bar: \"xyz\"}",
.expected_ast = R"(
{
foo^#2:Expr.Ident#:5^#4:int64#^#3:Expr.CreateStruct.Entry#,
bar^#5:Expr.Ident#:"xyz"^#7:string#^#6:Expr.CreateStruct.Entry#
foo^#3:Expr.Ident#:5^#4:int64#^#2:Expr.CreateStruct.Entry#,
bar^#6:Expr.Ident#:"xyz"^#7:string#^#5:Expr.CreateStruct.Entry#
}^#1:Expr.CreateMap#
)",
},
Expand Down
8 changes: 8 additions & 0 deletions parser/internal/pratt_parser_worker.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,14 @@ int64_t ParserWorker::NextId(int32_t position) {

int64_t ParserWorker::NextId() { return NextId(-1); }

void ParserWorker::SetPosition(int64_t id, const Token& token) {
if (token.start >= 0) {
positions_[id] = token.start;
SetNodeRange(id, token.start,
token.end > token.start ? token.end - 1 : token.start);
}
}

int64_t ParserWorker::CopyId(int64_t id) {
if (id == 0) {
return 0;
Expand Down
93 changes: 49 additions & 44 deletions parser/internal/pratt_parser_worker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#define THIRD_PARTY_CEL_CPP_PARSER_INTERNAL_PRATT_PARSER_WORKER_H_

#include <algorithm>
#include <cstddef>
#include <cstdint>
#include <functional>
#include <limits>
Expand Down Expand Up @@ -83,14 +84,12 @@ class ParserWorker {
int64_t NextId(int32_t position);
int64_t NextId(const Token& token) {
int64_t id = NextId(token.start);
if (ABSL_PREDICT_FALSE(track_node_ranges_)) {
if (token.start >= 0 && token.end > token.start) {
node_ranges_[id] = {token.start, token.end - 1};
}
}
SetNodeRange(id, token.start,
token.end > token.start ? token.end - 1 : token.start);
return id;
}
int64_t NextId();
void SetPosition(int64_t id, const Token& token);
int64_t CopyId(int64_t id);
void EraseId(int64_t id);
void SetNodeRange(int64_t id, int32_t begin, int32_t end) {
Expand Down Expand Up @@ -150,6 +149,7 @@ template <typename ExprNode>
class PrattParserWorker : public ParserWorker {
public:
using ParserWorker::NextId;
using ParserWorker::SetPosition;

explicit PrattParserWorker(
const cel::Source& source, const cel::ParserOptions& options,
Expand Down Expand Up @@ -530,57 +530,61 @@ void PrattParserWorker<ExprNode>::ParseSelectorChainTail(ExprNode& lhs) {

template <typename ExprNode>
ExprNode PrattParserWorker<ExprNode>::ParseUnaryOpsChain(Token first_op) {
struct UnaryOpInfo {
TokenType type;
int64_t id;
struct UnaryOp {
Token token;
int64_t id = 0;
};
std::vector<UnaryOpInfo> ops;
ops.push_back({first_op.type, NextId(first_op)});

std::vector<UnaryOp> ops;
ops.push_back({first_op});
while (peek_token_.type == TokenType::kExclamation ||
peek_token_.type == TokenType::kMinus) {
Token op = NextToken();
ops.push_back({op.type, NextId(op)});
ops.push_back({NextToken()});
}

ExprNode operand;
if (!ops.empty() && ops.back().type == TokenType::kMinus) {
if (options_.fold_unary_operators && ops.size() > 1 &&
ops[ops.size() - 2].type == TokenType::kMinus) {
// Match the ANTLR parser behavior where `-(-)+` prefers to match as
// repeated negate operators instead of a negation of an int literal.
// ---9223372036854775808 will fail to parse.
ops.pop_back();
ops.pop_back();
operand = ParseSelectorChain();
} else if (peek_token_.type == TokenType::kInt) {
int64_t op_id = ops.back().id;
ops.pop_back();
operand = ParseNegativeIntLiteral(op_id);
ParseSelectorChainTail(operand);
} else if (peek_token_.type == TokenType::kFloat) {
int64_t op_id = ops.back().id;
ops.pop_back();
operand = ParseNegativeDoubleLiteral(op_id);
ParseSelectorChainTail(operand);
} else {
operand = ParseSelectorChain();
const bool has_solitary_trailing_minus =
!ops.empty() && ops.back().token.type == TokenType::kMinus &&
(ops.size() == 1 || ops[ops.size() - 2].token.type != TokenType::kMinus);

if (options_.fold_unary_operators) {
size_t write = 0;
for (size_t read = 0; read < ops.size();) {
size_t next = read;
while (next < ops.size() &&
ops[next].token.type == ops[read].token.type) {
next++;
}
if ((next - read) % 2 != 0) {
ops[write++] = ops[read];
}
read = next;
}
ops.resize(write);
}

for (auto& op : ops) {
op.id = NextId(op.token);
}

ExprNode operand;
// Match the ANTLR parser behavior where `-(-)+` prefers to match as
// repeated negate operators instead of a negation of an int literal.
// ---9223372036854775808 will fail to parse.
if (has_solitary_trailing_minus && (peek_token_.type == TokenType::kInt ||
peek_token_.type == TokenType::kFloat)) {
int64_t op_id = ops.back().id;
ops.pop_back();
operand = (peek_token_.type == TokenType::kInt)
? ParseNegativeIntLiteral(op_id)
: ParseNegativeDoubleLiteral(op_id);
ParseSelectorChainTail(operand);
} else {
operand = ParseSelectorChain();
}

for (int i = static_cast<int>(ops.size()) - 1; i >= 0; --i) {
std::vector<ExprNode> args;
if (options_.fold_unary_operators && i > 0) {
if (ops[i - 1].type == ops[i].type) {
i--;
continue;
}
}

args.push_back(std::move(operand));
absl::string_view op_name = (ops[i].type == TokenType::kExclamation)
absl::string_view op_name = (ops[i].token.type == TokenType::kExclamation)
? CelOperator::LOGICAL_NOT
: CelOperator::NEGATE;
operand =
Expand Down Expand Up @@ -777,12 +781,13 @@ ExprNode PrattParserWorker<ExprNode>::ParseMap() {
}
key_start = peek_token_;
}
int64_t entry_id = NextId();
ExprNode key = ParseExpr();
Token colon = peek_token_;
if (!Expect(TokenType::kColon, "expected ':' in map entry")) {
break;
}
int64_t entry_id = NextId(colon);
SetPosition(entry_id, colon);
builder.Add(entry_id, std::move(key), ParseExpr(), optional);
if (peek_token_.type == TokenType::kComma) {
NextToken();
Expand Down
Loading
Loading