Skip to content

Commit ea2debf

Browse files
dmitriplotnikovcopybara-github
authored andcommitted
[Pratt Parser] Optimize call stack depth via in-place AST mutation and lean dispatch
- In-place AST mutation in binary/ternary operations: - Updated some Parse* functions to accept ExprNode& lhs and mutate lhs in place instead of returning ExprNode by value. - Eliminated some intermediate named temporary rhs stack variables. - Lean ParseUnary and ParsePrimary dispatch: - Extracted prefix operator parsing (!, -) into ParseUnaryOps(), making ParseUnary() a fast single branch for standard non-prefix expressions. PiperOrigin-RevId: 963785455
1 parent 9bcb00a commit ea2debf

7 files changed

Lines changed: 291 additions & 88 deletions

File tree

parser/internal/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ cc_library(
9393
"//parser:parser_interface",
9494
"@com_google_absl//absl/base:core_headers",
9595
"@com_google_absl//absl/base:nullability",
96+
"@com_google_absl//absl/cleanup",
9697
"@com_google_absl//absl/container:flat_hash_map",
9798
"@com_google_absl//absl/status:statusor",
9899
"@com_google_absl//absl/strings",

parser/internal/lexer.h

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,13 @@ class Lexer final {
140140
std::numeric_limits<int32_t>::max()));
141141
}
142142

143+
struct Position final {
144+
int32_t position = 0;
145+
bool at_end = false;
146+
bool done = false;
147+
LexerError error;
148+
};
149+
143150
Lexer(const Lexer&) = delete;
144151
Lexer(Lexer&&) = delete;
145152
Lexer& operator=(const Lexer&) = delete;
@@ -158,6 +165,17 @@ class Lexer final {
158165

159166
[[nodiscard]] int32_t GetPosition() const { return position_; }
160167

168+
[[nodiscard]] Position SavePosition() const {
169+
return Position{position_, at_end_, done_, error_};
170+
}
171+
172+
void RestorePosition(const Position& position) {
173+
position_ = position.position;
174+
at_end_ = position.at_end;
175+
done_ = position.done;
176+
error_ = position.error;
177+
}
178+
161179
private:
162180
[[nodiscard]] bool Match(char32_t c) const {
163181
return position_ < content_.size() && content_.at(position_) == c;

parser/internal/lexer_test.cc

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,5 +495,42 @@ TEST(LexerErrorRecoveryTest, ResumesAfterError) {
495495
EXPECT_EQ(token.end, 6);
496496
}
497497

498+
TEST(LexerPositionTest, SaveAndRestorePosition) {
499+
ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource("foo + bar * 42"));
500+
Lexer lexer(*source);
501+
502+
Token tok1 = lexer.Lex();
503+
EXPECT_EQ(tok1.type, TokenType::kIdent);
504+
505+
Token tok2 = lexer.Lex();
506+
EXPECT_EQ(tok2.type, TokenType::kWhitespace);
507+
508+
// Save position before '+'
509+
Lexer::Position saved = lexer.SavePosition();
510+
511+
Token tok3 = lexer.Lex();
512+
EXPECT_EQ(tok3.type, TokenType::kPlus);
513+
514+
Token tok4 = lexer.Lex();
515+
EXPECT_EQ(tok4.type, TokenType::kWhitespace);
516+
517+
Token tok5 = lexer.Lex();
518+
EXPECT_EQ(tok5.type, TokenType::kIdent);
519+
520+
// Restore position to before '+'
521+
lexer.RestorePosition(saved);
522+
523+
Token tok3_restored = lexer.Lex();
524+
EXPECT_EQ(tok3_restored.type, TokenType::kPlus);
525+
EXPECT_EQ(tok3_restored.start, tok3.start);
526+
EXPECT_EQ(tok3_restored.end, tok3.end);
527+
528+
Token tok4_restored = lexer.Lex();
529+
EXPECT_EQ(tok4_restored.type, TokenType::kWhitespace);
530+
531+
Token tok5_restored = lexer.Lex();
532+
EXPECT_EQ(tok5_restored.type, TokenType::kIdent);
533+
}
534+
498535
} // namespace
499536
} // namespace cel::parser_internal

parser/internal/pratt_parser_test.cc

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,32 @@ MATCHER_P(AstIs, expected_ast, "") {
180180
return false;
181181
}
182182

183+
MATCHER_P(AstEq, expected_expr, "") {
184+
KindAndIdAdorner kind_and_id_adorner;
185+
cel::ExprPrinter printer(kind_and_id_adorner);
186+
ParserOptions options;
187+
auto actual_ast = Parse(arg, options);
188+
if (!actual_ast.ok()) {
189+
*result_listener << "\n Actual expression failed to parse: "
190+
<< actual_ast.status();
191+
return false;
192+
}
193+
std::string actual = Unindent(printer.Print((*actual_ast)->root_expr()));
194+
auto expected_ast = Parse(expected_expr, options);
195+
if (!expected_ast.ok()) {
196+
*result_listener << "\n Expected expression failed to parse: "
197+
<< expected_ast.status();
198+
return false;
199+
}
200+
std::string expected = Unindent(printer.Print((*expected_ast)->root_expr()));
201+
if (actual == expected) {
202+
return true;
203+
}
204+
*result_listener << "\n Actual: " << actual
205+
<< "\n Expected: " << expected;
206+
return false;
207+
}
208+
183209
TEST_P(PrattParserTest, Parse) {
184210
const TestCase& test_case = GetParam();
185211
cel::ParserOptions options;
@@ -1472,6 +1498,44 @@ TEST(PrattParserRecursionDepthTest, ParseRecursionDepth) {
14721498
StatusIs(absl::StatusCode::kCancelled));
14731499
}
14741500

1501+
TEST(PrattParserRecursionDepthTest, ParseRecursionDepthIgnoreExtraParens) {
1502+
cel::ParserOptions options;
1503+
options.max_recursion_depth = 1;
1504+
EXPECT_THAT(Parse("((((1))))", options), IsOkAndHolds(NotNull()));
1505+
}
1506+
1507+
TEST(PrattParserRecursionDepthTest, DeeplyNestedParens) {
1508+
cel::ParserOptions options;
1509+
options.max_recursion_depth = 1;
1510+
std::string literal_expr =
1511+
std::string(1000, '(') + "42" + std::string(1000, ')');
1512+
EXPECT_THAT(Parse(literal_expr, options), IsOkAndHolds(NotNull()));
1513+
1514+
std::string binary_expr =
1515+
std::string(1000, '(') + "1 + 2" + std::string(1000, ')');
1516+
EXPECT_THAT(Parse(binary_expr, options), IsOkAndHolds(NotNull()));
1517+
}
1518+
1519+
TEST(PrattParserRecursionDepthTest, NestedAndGroupingParensCombinations) {
1520+
EXPECT_THAT("(( (1) + 2 ))", AstEq("1 + 2"));
1521+
EXPECT_THAT("((1 + 2) * (3 + 4))", AstEq("(1 + 2) * (3 + 4)"));
1522+
EXPECT_THAT("((((1)) + ((2))))", AstEq("1 + 2"));
1523+
EXPECT_THAT("(((1 + 2) * 3) + 4)", AstEq("(1 + 2) * 3 + 4"));
1524+
EXPECT_THAT("f((((1))), (((2))))", AstEq("f(1, 2)"));
1525+
EXPECT_THAT("[{((1)): ((2))}]", AstEq("[{1: 2}]"));
1526+
EXPECT_THAT("(((a))).b[0]", AstEq("a.b[0]"));
1527+
}
1528+
1529+
TEST(PrattParserRecursionDepthTest, MismatchedParensStillReportErrors) {
1530+
cel::ParserOptions options;
1531+
EXPECT_THAT(Parse("((((1))", options),
1532+
StatusIs(absl::StatusCode::kInvalidArgument));
1533+
EXPECT_THAT(Parse("(((1 + 2]", options),
1534+
StatusIs(absl::StatusCode::kInvalidArgument));
1535+
EXPECT_THAT(Parse("(( [ 1 ) ] ))", options),
1536+
StatusIs(absl::StatusCode::kInvalidArgument));
1537+
}
1538+
14751539
TEST(PrattParserRecursionDepthTest, SequentialScopesDoNotAccumulateDepth) {
14761540
cel::ParserOptions options;
14771541
options.max_recursion_depth = 2;

parser/internal/pratt_parser_worker.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ std::string ParserWorker::GetTokenText(const Token& tok) const {
123123
return "";
124124
}
125125

126-
Token ParserWorker::NextSignificantToken() {
126+
Token ParserWorker::NextSignificantToken(bool report_error) {
127127
if (is_recovery_limit_exceeded()) {
128128
return Token{.type = TokenType::kEnd, .start = 0, .end = 0};
129129
}
@@ -132,7 +132,7 @@ Token ParserWorker::NextSignificantToken() {
132132
if (tok.type == TokenType::kWhitespace || tok.type == TokenType::kComment) {
133133
continue;
134134
}
135-
if (tok.type == TokenType::kError) {
135+
if (tok.type == TokenType::kError && report_error) {
136136
ReportError(tok, lexer_.GetError().message);
137137
if (is_recovery_limit_exceeded()) {
138138
return Token{.type = TokenType::kEnd, .start = 0, .end = 0};

0 commit comments

Comments
 (0)