Skip to content

Commit 2d149d1

Browse files
committed
[clang-tidy] Fix crashes on if consteval in readability checks
The `readability-braces-around-statements` check tries to look at the closing parens of the if condition to determine where to insert braces, however, "consteval if" statements don't have a condition, and always have braces regardless, so the skip can be checked. The `readability-simplify-boolean-expr` check looks at the condition of the if statement to determine what could be simplified, but as "consteval if" statements do not have a condition that could be simplified, they can also be skipped here. There may still be more checks that try to look at the conditions of `if`s that aren't included here Fixes llvm#57568 Reviewed By: njames93, aaron.ballman Differential Revision: https://reviews.llvm.org/D133413
1 parent d330731 commit 2d149d1

File tree

5 files changed

+83
-4
lines changed

5 files changed

+83
-4
lines changed

clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,10 @@ void BracesAroundStatementsCheck::check(
131131
return;
132132
checkStmt(Result, S->getBody(), StartLoc);
133133
} else if (const auto *S = Result.Nodes.getNodeAs<IfStmt>("if")) {
134+
// "if consteval" always has braces.
135+
if (S->isConsteval())
136+
return;
137+
134138
SourceLocation StartLoc = findRParenLoc(S, SM, Context);
135139
if (StartLoc.isInvalid())
136140
return;

clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -354,8 +354,9 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
354354
}
355355

356356
bool VisitIfStmt(IfStmt *If) {
357-
// Skip any if's that have a condition var or an init statement.
358-
if (If->hasInitStorage() || If->hasVarStorage())
357+
// Skip any if's that have a condition var or an init statement, or are
358+
// "if consteval" statements.
359+
if (If->hasInitStorage() || If->hasVarStorage() || If->isConsteval())
359360
return true;
360361
/*
361362
* if (true) ThenStmt(); -> ThenStmt();
@@ -467,7 +468,8 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
467468
* if (Cond) return false; return true; -> return !Cond;
468469
*/
469470
auto *If = cast<IfStmt>(*First);
470-
if (!If->hasInitStorage() && !If->hasVarStorage()) {
471+
if (!If->hasInitStorage() && !If->hasVarStorage() &&
472+
!If->isConsteval()) {
471473
ExprAndBool ThenReturnBool =
472474
checkSingleStatement(If->getThen(), parseReturnLiteralBool);
473475
if (ThenReturnBool &&
@@ -491,7 +493,7 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
491493
: cast<DefaultStmt>(*First)->getSubStmt();
492494
auto *SubIf = dyn_cast<IfStmt>(SubStmt);
493495
if (SubIf && !SubIf->getElse() && !SubIf->hasInitStorage() &&
494-
!SubIf->hasVarStorage()) {
496+
!SubIf->hasVarStorage() && !SubIf->isConsteval()) {
495497
ExprAndBool ThenReturnBool =
496498
checkSingleStatement(SubIf->getThen(), parseReturnLiteralBool);
497499
if (ThenReturnBool &&

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,11 @@ Changes in existing checks
159159
<clang-tidy/checks/readability/avoid-const-params-in-decls>` to not
160160
warn about `const` value parameters of declarations inside macros.
161161

162+
- Fixed crashes in :doc:`readability-braces-around-statements
163+
<clang-tidy/checks/readability/braces-around-statements>` and
164+
:doc:`readability-simplify-boolean-expr <clang-tidy/checks/readability/simplify-boolean-expr>`
165+
when using a C++23 ``if consteval`` statement.
166+
162167
Removed checks
163168
^^^^^^^^^^^^^^
164169

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// RUN: clang-tidy %s -checks='-*,readability-braces-around-statements' -- -std=c++2b | count 0
2+
3+
constexpr void handle(bool) {}
4+
5+
constexpr void shouldPass() {
6+
if consteval {
7+
handle(true);
8+
} else {
9+
handle(false);
10+
}
11+
}
12+
13+
constexpr void shouldPassNegated() {
14+
if !consteval {
15+
handle(false);
16+
} else {
17+
handle(true);
18+
}
19+
}
20+
21+
constexpr void shouldPassSimple() {
22+
if consteval {
23+
handle(true);
24+
}
25+
}
26+
27+
void run() {
28+
shouldPass();
29+
shouldPassNegated();
30+
shouldPassSimple();
31+
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// RUN: clang-tidy %s -checks='-*,readability-simplify-boolean-expr' -- -std=c++2b | count 0
2+
template <bool Cond>
3+
constexpr int testIf() {
4+
if consteval {
5+
if constexpr (Cond) {
6+
return 0;
7+
} else {
8+
return 1;
9+
}
10+
} else {
11+
return 2;
12+
}
13+
}
14+
15+
constexpr bool testCompound() {
16+
if consteval {
17+
return true;
18+
}
19+
return false;
20+
}
21+
22+
constexpr bool testCase(int I) {
23+
switch (I) {
24+
case 0: {
25+
if consteval {
26+
return true;
27+
}
28+
return false;
29+
}
30+
default: {
31+
if consteval {
32+
return false;
33+
}
34+
return true;
35+
}
36+
}
37+
}

0 commit comments

Comments
 (0)