LibJS: Parse secondary expressions with the original forbidden token set

Instead of passing the continuously merged initial forbidden token set
(with the new additional forbidden tokens from each parsed secondary
expression) to the next call of parse_secondary_expression(), keep a
copy of the original set and use it as the base for parsing the next
secondary expression.

This bug prevented us from properly parsing the following expression:

```js
0 ?? 0 ? 0 : 0 || 0
```

...due to LogicalExpression with LogicalOp::NullishCoalescing returning
both DoubleAmpersand and DoublePipe in its forbidden token set.

The following correct AST is now generated:

Program
  (Children)
    ExpressionStatement
      ConditionalExpression
        (Test)
          LogicalExpression
            NumericLiteral 0
            ??
            NumericLiteral 0
        (Consequent)
          NumericLiteral 0
        (Alternate)
          LogicalExpression
            NumericLiteral 0
            ||
            NumericLiteral 0

An alternate solution I explored was only merging the original forbidden
token set with the one of the last parsed secondary expression which is
then passed to match_secondary_expression(); however that led to an
incorrect AST (note the alternate expression):

Program
  (Children)
    ExpressionStatement
      LogicalExpression
        ConditionalExpression
          (Test)
            LogicalExpression
              NumericLiteral 0
              ??
              NumericLiteral 0
          (Consequent)
            NumericLiteral 0
          (Alternate)
            NumericLiteral 0
        ||
        NumericLiteral 0

Truth be told, I don't know enough about the inner workings of the
parser to fully explain the difference. AFAICT this patch has no
unintended side effects in its current form though.

Fixes #18087.
This commit is contained in:
Linus Groh 2023-04-01 20:44:32 +01:00 committed by Andreas Kling
parent 85d0637058
commit 3709d11212
Notes: sideshowbarker 2024-07-17 05:02:35 +09:00
2 changed files with 15 additions and 1 deletions

View File

@ -2001,6 +2001,7 @@ NonnullRefPtr<Expression const> Parser::parse_expression(int min_precedence, Ass
expression = create_ast_node<TaggedTemplateLiteral>({ m_source_code, rule_start.position(), position() }, move(expression), move(template_literal));
}
if (should_continue_parsing) {
auto original_forbidden = forbidden;
while (match_secondary_expression(forbidden)) {
int new_precedence = g_operator_precedence.get(m_state.current_token.type());
if (new_precedence < min_precedence)
@ -2010,7 +2011,7 @@ NonnullRefPtr<Expression const> Parser::parse_expression(int min_precedence, Ass
check_for_invalid_object_property(expression);
Associativity new_associativity = operator_associativity(m_state.current_token.type());
auto result = parse_secondary_expression(move(expression), new_precedence, new_associativity, forbidden);
auto result = parse_secondary_expression(move(expression), new_precedence, new_associativity, original_forbidden);
expression = result.expression;
forbidden = forbidden.merge(result.forbidden);
while (match(TokenType::TemplateLiteralStart) && !is<UpdateExpression>(*expression)) {

View File

@ -20,6 +20,19 @@ test("mixing coalescing and logical operators with parens", () => {
expect("if (0) a || (b * c) ?? d").not.toEval();
});
test("mixing coalescing and logical operators in ternary expressions", () => {
expect("0 || 0 ? 0 : 0 ?? 0").toEval();
expect("0 ?? 0 ? 0 : 0 || 0").toEval();
expect("0 ? 0 || 0 : 0 ?? 0").toEval();
expect("0 ? 0 ?? 0 : 0 || 0").toEval();
expect("0 && 0 ? 0 ?? 0 : 0 || 0").toEval();
expect("0 ?? 0 ? 0 && 0 : 0 || 0").toEval();
expect("0 ?? 0 ? 0 || 0 : 0 && 0").toEval();
expect("0 || 0 ? 0 ?? 0 : 0 && 0").toEval();
expect("0 && 0 ? 0 || 0 : 0 ?? 0").toEval();
expect("0 || 0 ? 0 && 0 : 0 ?? 0").toEval();
});
test("mixing coalescing and logical operators when 'in' isn't allowed", () => {
expect("for (a ?? b || c in a; false;);").not.toEval();
expect("for (a ?? b && c in a; false;);").not.toEval();