Skip to content

Commit

Permalink
[Clang] Fix the for statement disappearing in AST when an error occur…
Browse files Browse the repository at this point in the history
…s in the conditional expression of the for statement (#65381)

Consider:
```
constexpr int f() {
    int sum = 0;
    for (int i = 0; undefined_var; ++i) {
        sum += i;
    }
    return sum;
}

static_assert(f());
```

The AST before this patch:
```
|-FunctionDecl <line:1:1, line:7:1> line:1:15 used constexpr f 'int ()' implicit-inline
| `-CompoundStmt <col:19, line:7:1>
|   |-DeclStmt <line:2:5, col:16>
|   | `-VarDecl <col:5, col:15> col:9 used sum 'int' cinit
|   |   `-IntegerLiteral <col:15> 'int' 0
|   `-ReturnStmt <line:6:5, col:12>
|     `-ImplicitCastExpr <col:12> 'int' <LValueToRValue>
|       `-DeclRefExpr <col:12> 'int' lvalue Var 0xb870518 'sum' 'int'
```

The AST after this patch:
```
|-FunctionDecl 0x11d0f63f8 <./main.cpp:1:1, line:7:1> line:1:15 used constexpr f 'int ()' implicit-inline
| `-CompoundStmt 0x11d110880 <col:19, line:7:1>
|   |-DeclStmt 0x11d0f65c8 <line:2:5, col:16>
|   | `-VarDecl 0x11d0f6528 <col:5, col:15> col:9 used sum 'int' cinit
|   |   `-IntegerLiteral 0x11d0f6590 <col:15> 'int' 0
|   |-ForStmt 0x11d110800 <line:3:5, line:5:5>
|   | |-DeclStmt 0x11d0f66a0 <line:3:10, col:19>
|   | | `-VarDecl 0x11d0f6600 <col:10, col:18> col:14 used i 'int' cinit
|   | |   `-IntegerLiteral 0x11d0f6668 <col:18> 'int' 0
|   | |-<<<NULL>>>
|   | |-RecoveryExpr 0x11d0f66e8 <col:21> 'bool' contains-errors
|   | |-UnaryOperator 0x11d0f6728 <col:36, col:38> 'int' lvalue prefix '++'
|   | | `-DeclRefExpr 0x11d0f6708 <col:38> 'int' lvalue Var 0x11d0f6600 'i' 'int'
|   | `-CompoundStmt 0x11d0f67c8 <col:41, line:5:5>
|   |   `-CompoundAssignOperator 0x11d0f6798 <line:4:9, col:16> 'int' lvalue '+=' ComputeLHSTy='int' ComputeResultTy='int'
|   |     |-DeclRefExpr 0x11d0f6740 <col:9> 'int' lvalue Var 0x11d0f6528 'sum' 'int'
|   |     `-ImplicitCastExpr 0x11d0f6780 <col:16> 'int' <LValueToRValue>
|   |       `-DeclRefExpr 0x11d0f6760 <col:16> 'int' lvalue Var 0x11d0f6600 'i' 'int'
|   `-ReturnStmt 0x11d110870 <line:6:5, col:12>
|     `-ImplicitCastExpr 0x11d110858 <col:12> 'int' <LValueToRValue>
|       `-DeclRefExpr 0x11d110838 <col:12> 'int' lvalue Var 0x11d0f6528 'sum' 'int'
```

---------

Co-authored-by: Shafik Yaghmour <shafik@users.noreply.github.com>
  • Loading branch information
yronglin and shafik committed Sep 8, 2023
1 parent 123bf08 commit 3cfdef3
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 2 deletions.
19 changes: 17 additions & 2 deletions clang/lib/Parse/ParseStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2158,11 +2158,13 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
// for-range-declaration next.
bool MightBeForRangeStmt = !ForRangeInfo.ParsedForRangeDecl();
ColonProtectionRAIIObject ColonProtection(*this, MightBeForRangeStmt);
SourceLocation SecondPartStart = Tok.getLocation();
Sema::ConditionKind CK = Sema::ConditionKind::Boolean;
SecondPart = ParseCXXCondition(
nullptr, ForLoc, Sema::ConditionKind::Boolean,
/*InitStmt=*/nullptr, ForLoc, CK,
// FIXME: recovery if we don't see another semi!
/*MissingOK=*/true, MightBeForRangeStmt ? &ForRangeInfo : nullptr,
/*EnterForConditionScope*/ true);
/*EnterForConditionScope=*/true);

if (ForRangeInfo.ParsedForRangeDecl()) {
Diag(FirstPart.get() ? FirstPart.get()->getBeginLoc()
Expand All @@ -2178,6 +2180,19 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
<< FixItHint::CreateRemoval(EmptyInitStmtSemiLoc);
}
}

if (SecondPart.isInvalid()) {
ExprResult CondExpr = Actions.CreateRecoveryExpr(
SecondPartStart,
Tok.getLocation() == SecondPartStart ? SecondPartStart
: PrevTokLocation,
{}, Actions.PreferredConditionType(CK));
if (!CondExpr.isInvalid())
SecondPart = Actions.ActOnCondition(getCurScope(), ForLoc,
CondExpr.get(), CK,
/*MissingOK=*/false);
}

} else {
// We permit 'continue' and 'break' in the condition of a for loop.
getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);
Expand Down
15 changes: 15 additions & 0 deletions clang/test/AST/ast-dump-recovery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -432,3 +432,18 @@ void RecoveryToDoWhileStmtCond() {
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 10
do {} while (some_invalid_val + 1 < 10);
}

void RecoveryForStmtCond() {
// CHECK:FunctionDecl {{.*}} RecoveryForStmtCond
// CHECK-NEXT:`-CompoundStmt {{.*}}
// CHECK-NEXT: `-ForStmt {{.*}}
// CHECK-NEXT: |-DeclStmt {{.*}}
// CHECK-NEXT: | `-VarDecl {{.*}}
// CHECK-NEXT: | `-IntegerLiteral {{.*}} <col:16> 'int' 0
// CHECK-NEXT: |-<<<NULL>>>
// CHECK-NEXT: |-RecoveryExpr {{.*}} 'bool' contains-errors
// CHECK-NEXT: |-UnaryOperator {{.*}} 'int' lvalue prefix '++'
// CHECK-NEXT: | `-DeclRefExpr {{.*}} 'int' lvalue Var {{.*}} 'i' 'int'
// CHECK-NEXT: `-CompoundStmt {{.*}}
for (int i = 0; i < invalid; ++i) {}
}
1 change: 1 addition & 0 deletions clang/test/SemaCXX/constexpr-function-recovery-crash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,4 @@ TEST_EVALUATE(ForCond, for (; !!;){};);// expected-error + {{}}
TEST_EVALUATE(ForInc, for (;; !!){};);// expected-error + {{}}
// expected-note@-1 + {{infinite loop}}
// expected-note@-2 {{in call}}
TEST_EVALUATE(ForCondUnDef, for (;some_cond;){};); // expected-error + {{}}

0 comments on commit 3cfdef3

Please sign in to comment.