From c56757268726cc48cb437d75f79912ee09cfda69 Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Fri, 18 Sep 2026 18:58:30 -0700 Subject: [PATCH] Iteratively parse nested parentheses in PrattParser Replace the lookahead-based grouping parentheses counter (`countGroupingParentheses`) with a bottom-up iterative approach in `PrattParser.parsePrimary()` to eliminate quadratic $O(N^2)$ lookahead scans while keeping $O(1)$ call-stack frames on deeply nested parenthesized expressions. ### Benchmarks (`CelParserBenchmark`, `-c opt`, `parserType=PRATT`, `parseOnly`) | Benchmark Case | Before (Mean) | After (Mean) | Change / Speedup | | :--- | ---: | ---: | ---: | | `NESTED_PARENS` (N=200) | 8,528.69 ns | 5,119.12 ns | -40.0% (1.67x faster) | | `NESTED_LEFT_PARENS_CALC` (N=200) | 951,027.39 ns | 27,998.44 ns | -97.1% (34.0x faster) | | Benchmark Case (Allocations) | Before (Bytes / Objs) | After (Bytes / Objs) | Change | | :--- | ---: | ---: | ---: | | `NESTED_PARENS` (N=200) | 29,200 B / 855 objs | 16,336 B / 453 objs | -44.1% B / -47.0% objs | | `NESTED_LEFT_PARENS_CALC` (N=200) | ~2,690,000 B / ~84,500 objs | 127,016 B / 4,577 objs | -95.3% B / -94.6% objs | PiperOrigin-RevId: 984184714 --- .../main/java/dev/cel/parser/PrattParser.java | 120 ++++++------------ .../dev/cel/parser/CelParserImplTest.java | 13 +- .../src/test/resources/parser_errors.baseline | 9 -- .../resources/pratt_parser_errors.baseline | 6 - 4 files changed, 50 insertions(+), 98 deletions(-) diff --git a/parser/src/main/java/dev/cel/parser/PrattParser.java b/parser/src/main/java/dev/cel/parser/PrattParser.java index 829ac1dcc..15b59b269 100644 --- a/parser/src/main/java/dev/cel/parser/PrattParser.java +++ b/parser/src/main/java/dev/cel/parser/PrattParser.java @@ -253,7 +253,7 @@ private boolean expect(Lexer.TokenType type, String msg) { nextToken(); return true; } - if (isRecoveryLimitExceeded()) { + if (recursionLimitExceeded || isRecoveryLimitExceeded()) { return false; } if (peekToken.type != Lexer.TokenType.ERROR) { @@ -274,7 +274,7 @@ private boolean expect(Lexer.TokenType type, String msg) { // Find the next delimiter to prevent a cascade of spurious secondary errors. private void synchronizeOnDelimiter() { - if (isRecoveryLimitExceeded()) { + if (recursionLimitExceeded || isRecoveryLimitExceeded()) { peekToken = END_TOKEN; return; } @@ -412,6 +412,7 @@ private void reportRecursionLimit(int position) { LOCALE, "Expression recursion limit exceeded. limit: %d", options.maxParseRecursionDepth())); + peekToken = END_TOKEN; } } @@ -429,11 +430,14 @@ private CelExpr parseExpr() { return expr; } - @SuppressWarnings("EnumOrdinal") // Using ordinal for O(1) binary operator lookup table private CelExpr parseBinaryAndTernary(int minPrec) { - CelExpr lhs = parseSelectorChain(); + return parseBinaryAndTernaryFromLhs(parseSelectorChain(), minPrec); + } + + @SuppressWarnings("EnumOrdinal") // Using ordinal for O(1) binary operator lookup table + private CelExpr parseBinaryAndTernaryFromLhs(CelExpr lhs, int minPrec) { int chainDepth = currentLhsDepth; - while (true) { + while (!recursionLimitExceeded && !isRecoveryLimitExceeded()) { Lexer.TokenType tok = peekToken.type; if (tok == Lexer.TokenType.QUESTION && minPrec <= 0) { lhs = parseTernary(lhs); @@ -771,16 +775,39 @@ private CelExpr parsePrimary() { switch (peekToken.type) { case LEFT_PAREN: { - int groupingParenCount = countGroupingParentheses(); - if (checkRecursion(groupingParenCount, peekToken)) { + if (recursionLimitExceeded || isRecoveryLimitExceeded()) { return ERROR; } - for (int i = 0; i < groupingParenCount; ++i) { + // To avoid deep call-stack recursion on heavily nested parentheses (e.g. "((((a))))" or + // "((((a + 1) + 1) + 1))"), consume all consecutive leading '(' tokens upfront, parse the + // innermost expression once, and then iteratively unwind each enclosing '(' from + // innermost to outermost. After consuming each matching ')', if more enclosing '(' remain + // open and the next token is not another ')', continue parsing any trailing selectors or + // binary/ternary operators belonging to that enclosing parenthesized level using the + // already-parsed inner expression as the LHS. + int openParens = 0; + while (peekToken.type == Lexer.TokenType.LEFT_PAREN) { + openParens++; + if (checkRecursion(openParens, peekToken)) { + return ERROR; + } nextToken(); } - CelExpr expr = parseExpr(); - for (int i = 0; i < groupingParenCount; ++i) { + recursionDepth += openParens; + CelExpr expr = parseBinaryAndTernary(0); + for (int i = 0; i < openParens; ++i) { expect(Lexer.TokenType.RIGHT_PAREN, ""); + recursionDepth--; + if (i < openParens - 1 && peekToken.type != Lexer.TokenType.RIGHT_PAREN) { + currentLhsDepth = 0; + Lexer.TokenType tok = peekToken.type; + if (tok == Lexer.TokenType.DOT + || tok == Lexer.TokenType.LEFT_BRACKET + || tok == Lexer.TokenType.LEFT_BRACE) { + expr = parseSelectorChainTail(expr); + } + expr = parseBinaryAndTernaryFromLhs(expr, 0); + } } return expr; } @@ -1098,8 +1125,7 @@ private Optional tryExpandMacro( return Optional.empty(); } if (nodeLimitExceeded) { - reportError( - getPosition(exprId), "could not expand macro: expression node limit exceeded"); + reportError(getPosition(exprId), "could not expand macro: expression node limit exceeded"); return Optional.empty(); } @@ -1178,76 +1204,6 @@ private CelExpr buildMacroCallArgs(CelExpr expr) { return expr; } - private int countGroupingParentheses() { - if (peekToken.type != Lexer.TokenType.LEFT_PAREN) { - return 0; - } - - // Fast path: if the next non-whitespace character is not '(', leading open parens is 1. - int pos = peekToken.end; - int size = content.size(); - while (pos < size) { - int c = content.get(pos); - if (c != ' ' && c != '\t' && c != '\n' && c != '\r' && c != '\f' && c != 11) { - if (c == '/') { - // A comment might precede another '('. - break; - } - if (c == '(') { - break; - } - // Next significant token is definitely not '('. - return 1; - } - pos++; - } - - int savedPos = lexer.savePosition(); - try { - int leadingOpenParens = 1; - Lexer.Token tok = nextSignificantToken(/* reportError= */ false); - while (tok.type == Lexer.TokenType.LEFT_PAREN) { - leadingOpenParens++; - tok = nextSignificantToken(/* reportError= */ false); - } - if (leadingOpenParens == 1) { - return 1; - } - - int openParens = leadingOpenParens; - int consecutiveLeadingClosed = 0; - - while (openParens > 0) { - if (tok.type == Lexer.TokenType.END || tok.type == Lexer.TokenType.ERROR) { - return 1; - } - - if (tok.type == Lexer.TokenType.LEFT_PAREN) { - openParens++; - consecutiveLeadingClosed = 0; - } else if (tok.type == Lexer.TokenType.RIGHT_PAREN) { - if (leadingOpenParens == openParens) { - leadingOpenParens--; - consecutiveLeadingClosed++; - } else { - consecutiveLeadingClosed = 0; - } - openParens--; - } else { - consecutiveLeadingClosed = 0; - } - - if (openParens > 0) { - tok = nextSignificantToken(/* reportError= */ false); - } - } - - return Math.max(1, consecutiveLeadingClosed); - } finally { - lexer.restorePosition(savedPos); - } - } - private final class PrattMacroExprFactory extends CelMacroExprFactory { private final ArrayDeque macroPositions = new ArrayDeque<>(1); diff --git a/parser/src/test/java/dev/cel/parser/CelParserImplTest.java b/parser/src/test/java/dev/cel/parser/CelParserImplTest.java index 09d578a36..6d1556466 100644 --- a/parser/src/test/java/dev/cel/parser/CelParserImplTest.java +++ b/parser/src/test/java/dev/cel/parser/CelParserImplTest.java @@ -229,7 +229,18 @@ private enum MaxParseRecursionDepthTestCase { + " a[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20] !=" + " a[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20] !=" + " a[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20] !=" - + " a[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20]"); + + " a[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20]"), + TERNARY( + "a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b :" + + " a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b :" + + " a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b :" + + " a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : c"), + TERNARY_TRUE_BRANCH_PARENS( + "a ? ((((((((((((((((((((((((((((((((b)))))))))))))))))))))))))))))))) : c"), + NESTED_LEFT_PARENS_WITH_CALC( + "((((((((((((((((((((((((((((((((7) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1)" + + " + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) +" + + " 1) + 1) + 1) + 1)"); static final int MAX_RECURSION_LIMIT = 32; final String source; diff --git a/parser/src/test/resources/parser_errors.baseline b/parser/src/test/resources/parser_errors.baseline index cbd9f087b..4f23e4a9c 100644 --- a/parser/src/test/resources/parser_errors.baseline +++ b/parser/src/test/resources/parser_errors.baseline @@ -1098,9 +1098,6 @@ E/A: Expression recursion limit exceeded. limit: 250 E/P: ERROR: :1:251: Expression recursion limit exceeded. limit: 250 | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['too many']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]] | ..........................................................................................................................................................................................................................................................^ -ERROR: :1:251: Syntax error: expected ']' - | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['too many']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]] - | ..........................................................................................................................................................................................................................................................^ I: [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[ »»»[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['too many']]]]]]]]]]]]]]]]]]]]]]]]]]]] @@ -1110,9 +1107,6 @@ E/A: Expression recursion limit exceeded. limit: 32 E/P: ERROR: :1:33: Expression recursion limit exceeded. limit: 32 | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[ | ................................^ -ERROR: :1:33: Syntax error: expected ']' - | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[ - | ................................^ I: [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]] =====> @@ -1120,9 +1114,6 @@ E/A: Expression recursion limit exceeded. limit: 32 E/P: ERROR: :1:33: Expression recursion limit exceeded. limit: 32 | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]] | ................................^ -ERROR: :1:33: Syntax error: expected ']' - | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]] - | ................................^ I: a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z.A.B.C.D.E.F.G.H =====> diff --git a/parser/src/test/resources/pratt_parser_errors.baseline b/parser/src/test/resources/pratt_parser_errors.baseline index d21e65ab3..541f9e14f 100644 --- a/parser/src/test/resources/pratt_parser_errors.baseline +++ b/parser/src/test/resources/pratt_parser_errors.baseline @@ -398,18 +398,12 @@ I: [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[ E: ERROR: :1:33: Expression recursion limit exceeded. limit: 32 | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[ | ................................^ -ERROR: :1:33: Syntax error: expected ']' - | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[ - | ................................^ I: [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]] =====> E: ERROR: :1:33: Expression recursion limit exceeded. limit: 32 | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]] | ................................^ -ERROR: :1:33: Syntax error: expected ']' - | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]] - | ................................^ I: a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z.A.B.C.D.E.F.G.H =====>