Skip to content

Commit

Permalink
[clang-tidy] Fix crashes on if consteval in readability checks
Browse files Browse the repository at this point in the history
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 #57568

Reviewed By: njames93, aaron.ballman

Differential Revision: https://reviews.llvm.org/D133413
  • Loading branch information
rymiel committed Oct 5, 2022
1 parent d330731 commit 2d149d1
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 4 deletions.
Expand Up @@ -131,6 +131,10 @@ void BracesAroundStatementsCheck::check(
return;
checkStmt(Result, S->getBody(), StartLoc);
} else if (const auto *S = Result.Nodes.getNodeAs<IfStmt>("if")) {
// "if consteval" always has braces.
if (S->isConsteval())
return;

SourceLocation StartLoc = findRParenLoc(S, SM, Context);
if (StartLoc.isInvalid())
return;
Expand Down
Expand Up @@ -354,8 +354,9 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
}

bool VisitIfStmt(IfStmt *If) {
// Skip any if's that have a condition var or an init statement.
if (If->hasInitStorage() || If->hasVarStorage())
// Skip any if's that have a condition var or an init statement, or are
// "if consteval" statements.
if (If->hasInitStorage() || If->hasVarStorage() || If->isConsteval())
return true;
/*
* if (true) ThenStmt(); -> ThenStmt();
Expand Down Expand Up @@ -467,7 +468,8 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
* if (Cond) return false; return true; -> return !Cond;
*/
auto *If = cast<IfStmt>(*First);
if (!If->hasInitStorage() && !If->hasVarStorage()) {
if (!If->hasInitStorage() && !If->hasVarStorage() &&
!If->isConsteval()) {
ExprAndBool ThenReturnBool =
checkSingleStatement(If->getThen(), parseReturnLiteralBool);
if (ThenReturnBool &&
Expand All @@ -491,7 +493,7 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
: cast<DefaultStmt>(*First)->getSubStmt();
auto *SubIf = dyn_cast<IfStmt>(SubStmt);
if (SubIf && !SubIf->getElse() && !SubIf->hasInitStorage() &&
!SubIf->hasVarStorage()) {
!SubIf->hasVarStorage() && !SubIf->isConsteval()) {
ExprAndBool ThenReturnBool =
checkSingleStatement(SubIf->getThen(), parseReturnLiteralBool);
if (ThenReturnBool &&
Expand Down
5 changes: 5 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Expand Up @@ -159,6 +159,11 @@ Changes in existing checks
<clang-tidy/checks/readability/avoid-const-params-in-decls>` to not
warn about `const` value parameters of declarations inside macros.

- Fixed crashes in :doc:`readability-braces-around-statements
<clang-tidy/checks/readability/braces-around-statements>` and
:doc:`readability-simplify-boolean-expr <clang-tidy/checks/readability/simplify-boolean-expr>`
when using a C++23 ``if consteval`` statement.

Removed checks
^^^^^^^^^^^^^^

Expand Down
@@ -0,0 +1,31 @@
// RUN: clang-tidy %s -checks='-*,readability-braces-around-statements' -- -std=c++2b | count 0

constexpr void handle(bool) {}

constexpr void shouldPass() {
if consteval {
handle(true);
} else {
handle(false);
}
}

constexpr void shouldPassNegated() {
if !consteval {
handle(false);
} else {
handle(true);
}
}

constexpr void shouldPassSimple() {
if consteval {
handle(true);
}
}

void run() {
shouldPass();
shouldPassNegated();
shouldPassSimple();
}
@@ -0,0 +1,37 @@
// RUN: clang-tidy %s -checks='-*,readability-simplify-boolean-expr' -- -std=c++2b | count 0
template <bool Cond>
constexpr int testIf() {
if consteval {
if constexpr (Cond) {
return 0;
} else {
return 1;
}
} else {
return 2;
}
}

constexpr bool testCompound() {
if consteval {
return true;
}
return false;
}

constexpr bool testCase(int I) {
switch (I) {
case 0: {
if consteval {
return true;
}
return false;
}
default: {
if consteval {
return false;
}
return true;
}
}
}

0 comments on commit 2d149d1

Please sign in to comment.