Iteratively parse nested parentheses in PrattParserWorker

parser/internal/BUILD

@@ -161,7 +161,6 @@ cc_library(
         "//parser:parser_interface",
         "@com_google_absl//absl/base:core_headers",
         "@com_google_absl//absl/base:nullability",
-        "@com_google_absl//absl/cleanup",
         "@com_google_absl//absl/container:flat_hash_map",
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",

parser/internal/pratt_parser_benchmark.cc

@@ -222,9 +222,46 @@ void BM_Antlr_ParseNestedParentheses(benchmark::State& state) {
   BM_ParseNestedParentheses(state, ParserImplType::kAntlr);
 }
 
-BENCHMARK(BM_Pratt_ParseNestedParentheses)->Arg(10)->Arg(50);
+BENCHMARK(BM_Pratt_ParseNestedParentheses)->Arg(10)->Arg(50)->Arg(200);
 BENCHMARK(BM_Antlr_ParseNestedParentheses)->Arg(10)->Arg(50);
 
+// -----------------------------------------------------------------------------
+// Workload 5b: Deeply Nested Left Parentheses with Calc ("((((a + 1) + 1))")
+// -----------------------------------------------------------------------------
+std::string BuildNestedLeftParenthesesCalc(int depth) {
+  std::string expr(depth, '(');
+  absl::StrAppend(&expr, "1 + 2");
+  for (int i = 0; i < depth; ++i) {
+    absl::StrAppend(&expr, ") + 1");
+  }
+  return expr;
+}
+
+void BM_ParseNestedLeftParenthesesCalc(benchmark::State& state,
+                                       ParserImplType type) {
+  cel::ParserOptions options;
+  auto parser = CreateParser(type, options);
+  std::string expr = BuildNestedLeftParenthesesCalc(state.range(0));
+
+  for (auto _ : state) {
+    auto source = cel::NewSource(expr);
+    ABSL_DCHECK_OK(source.status());
+    auto ast = parser->Parse(**source);
+    ABSL_DCHECK_OK(ast.status());
+    benchmark::DoNotOptimize(ast);
+  }
+}
+
+void BM_Pratt_ParseNestedLeftParenthesesCalc(benchmark::State& state) {
+  BM_ParseNestedLeftParenthesesCalc(state, ParserImplType::kPratt);
+}
+void BM_Antlr_ParseNestedLeftParenthesesCalc(benchmark::State& state) {
+  BM_ParseNestedLeftParenthesesCalc(state, ParserImplType::kAntlr);
+}
+
+BENCHMARK(BM_Pratt_ParseNestedLeftParenthesesCalc)->Arg(10)->Arg(50)->Arg(200);
+BENCHMARK(BM_Antlr_ParseNestedLeftParenthesesCalc)->Arg(10)->Arg(50);
+
 // -----------------------------------------------------------------------------
 // Workload 6: Common Representative Expressions with Syntax Errors
 // -----------------------------------------------------------------------------

parser/internal/pratt_parser_test.cc

@@ -1698,22 +1698,25 @@ TEST(PrattParserRecursionDepthTest, ParseRecursionDepth) {
               StatusIs(absl::StatusCode::kCancelled));
 }
 
-TEST(PrattParserRecursionDepthTest, ParseRecursionDepthIgnoreExtraParens) {
-  cel::ParserOptions options;
-  options.max_recursion_depth = 1;
-  EXPECT_THAT(Parse("((((1))))", options), IsOkAndHolds(NotNull()));
-}
-
+// Parentheses are unwound iteratively, so nesting them does not grow the C++
+// call stack. The recursion limit is raised above the nesting level here to
+// exercise that: each '(' still consumes one unit of the recursion budget.
 TEST(PrattParserRecursionDepthTest, DeeplyNestedParens) {
   cel::ParserOptions options;
-  options.max_recursion_depth = 1;
+  options.max_recursion_depth = 2100;
   std::string literal_expr =
       std::string(1000, '(') + "42" + std::string(1000, ')');
   EXPECT_THAT(Parse(literal_expr, options), IsOkAndHolds(NotNull()));
 
   std::string binary_expr =
       std::string(1000, '(') + "1 + 2" + std::string(1000, ')');
   EXPECT_THAT(Parse(binary_expr, options), IsOkAndHolds(NotNull()));
+
+  std::string left_nested_calc_expr = std::string(1000, '(') + "1 + 2";
+  for (int i = 0; i < 1000; ++i) {
+    left_nested_calc_expr += ") + 1";
+  }
+  EXPECT_THAT(Parse(left_nested_calc_expr, options), IsOkAndHolds(NotNull()));
 }
 
 TEST(PrattParserRecursionDepthTest, NestedAndGroupingParensCombinations) {
@@ -1724,6 +1727,21 @@ TEST(PrattParserRecursionDepthTest, NestedAndGroupingParensCombinations) {
   EXPECT_THAT("f((((1))), (((2))))", AstEq("f(1, 2)"));
   EXPECT_THAT("[{((1)): ((2))}]", AstEq("[{1: 2}]"));
   EXPECT_THAT("(((a))).b[0]", AstEq("a.b[0]"));
+  EXPECT_THAT("((((a).b[0]) + 1) ? 2 : 3)", AstEq("(a.b[0] + 1) ? 2 : 3"));
+  EXPECT_THAT("a ? ((((b)))) : c", AstEq("a ? b : c"));
+  EXPECT_THAT("((((7) + 1) + 1) + 1)", AstEq("7 + 1 + 1 + 1"));
+  EXPECT_THAT("(true) || (true || (true || false))",
+              AstEq("true || (true || (true || false))"));
+  EXPECT_THAT("(1 + 1 + 1) + 1", AstEq("1 + 1 + 1 + 1"));
+  EXPECT_THAT("((1 + 1) + 1) + 1", AstEq("1 + 1 + 1 + 1"));
+  EXPECT_THAT("(((1 + 1 + 1))) + 1", AstEq("1 + 1 + 1 + 1"));
+  EXPECT_THAT("1 + (1 + 1 + 1)", AstEq("1 + (1 + 1 + 1)"));
+  EXPECT_THAT("((1 + ((7))))", AstEq("1 + 7"));
+  EXPECT_THAT("(a.b.c).d.e.f", AstEq("a.b.c.d.e.f"));
+  EXPECT_THAT("((a && b.c.d).e)", AstEq("(a && b.c.d).e"));
+  EXPECT_THAT("((a && b && c.d.e).f)", AstEq("(a && b && c.d.e).f"));
+  EXPECT_THAT("((a ? b : c).d[0] ? (e ? f : g) : h) + ((x).y)",
+              AstEq("((a ? b : c).d[0] ? (e ? f : g) : h) + x.y"));
 }
 
 TEST(PrattParserRecursionDepthTest, MismatchedParensStillReportErrors) {
@@ -1744,13 +1762,18 @@ TEST(PrattParserRecursionDepthTest, SequentialScopesDoNotAccumulateDepth) {
 
 TEST(PrattParserRecursionDepthTest, DeeplyNestedTernary) {
   cel::ParserOptions options;
-  options.max_recursion_depth = 4;
+  options.max_recursion_depth = 5;
   EXPECT_THAT(Parse("a ? b : a ? b : a ? b : a ? b : c", options),
               IsOkAndHolds(NotNull()));
   EXPECT_THAT(Parse("a ? b : a ? b : a ? b : a ? b : a ? b : c", options),
               StatusIs(absl::StatusCode::kCancelled));
 }
 
+TEST(PrattParserRecursionDepthTest, NestedParensWithTernaryAndSelectors) {
+  EXPECT_THAT(Parse("((a ? b : c).d[0] ? (e ? f : g) : h) + ((x).y)"),
+              IsOkAndHolds(NotNull()));
+}
+
 class TestParserWorker : public ParserWorker {
   // Expose the protected constructor and methods for testing.
  public:

parser/internal/pratt_parser_worker.cc

@@ -123,7 +123,7 @@ std::string ParserWorker::GetTokenText(const Token& tok) const {
   return "";
 }
 
-Token ParserWorker::NextSignificantToken(bool report_error) {
+Token ParserWorker::NextSignificantToken() {
   if (is_recovery_limit_exceeded()) {
     return Token{.type = TokenType::kEnd, .start = 0, .end = 0};
   }
@@ -132,7 +132,7 @@ Token ParserWorker::NextSignificantToken(bool report_error) {
     if (tok.type == TokenType::kWhitespace || tok.type == TokenType::kComment) {
       continue;
     }
-    if (tok.type == TokenType::kError && report_error) {
+    if (tok.type == TokenType::kError) {
       ReportSyntaxError(tok, lexer_.GetError().message);
       if (is_recovery_limit_exceeded()) {
         return Token{.type = TokenType::kEnd, .start = 0, .end = 0};
@@ -159,7 +159,7 @@ bool ParserWorker::Expect(TokenType type, absl::string_view msg) {
     NextToken();
     return true;
   }
-  if (is_recovery_limit_exceeded()) {
+  if (recursion_limit_exceeded_ || is_recovery_limit_exceeded()) {
     return false;
   }
   if (peek_token_.type != TokenType::kError) {
@@ -184,7 +184,7 @@ bool ParserWorker::Expect(TokenType type, absl::string_view msg) {
 }
 
 void ParserWorker::SynchronizeOnDelimiter() {
-  if (is_recovery_limit_exceeded()) {
+  if (recursion_limit_exceeded_ || is_recovery_limit_exceeded()) {
     peek_token_ = Token{.type = TokenType::kEnd, .start = 0, .end = 0};
     return;
   }