Skip to content

Commit 3566024

Browse files
committed
[clang-tidy] Fix readability-simplify-boolean-expr when Ifs have an init statement or condition variable
Fixes llvm#55553. Reviewed By: LegalizeAdulthood Differential Revision: https://reviews.llvm.org/D125874
1 parent 4ac0589 commit 3566024

File tree

3 files changed

+50
-9
lines changed

3 files changed

+50
-9
lines changed

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

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,9 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
351351
}
352352

353353
bool VisitIfStmt(IfStmt *If) {
354+
// Skip any if's that have a condition var or an init statement.
355+
if (If->hasInitStorage() || If->hasVarStorage())
356+
return true;
354357
/*
355358
* if (true) ThenStmt(); -> ThenStmt();
356359
* if (false) ThenStmt(); -> <Empty>;
@@ -461,14 +464,17 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
461464
* if (Cond) return false; return true; -> return !Cond;
462465
*/
463466
auto *If = cast<IfStmt>(*First);
464-
ExprAndBool ThenReturnBool =
465-
checkSingleStatement(If->getThen(), parseReturnLiteralBool);
466-
if (ThenReturnBool && ThenReturnBool.Bool != TrailingReturnBool.Bool) {
467-
if (Check->ChainedConditionalReturn ||
468-
(!PrevIf && If->getElse() == nullptr)) {
469-
Check->replaceCompoundReturnWithCondition(
470-
Context, cast<ReturnStmt>(*Second), TrailingReturnBool.Bool, If,
471-
ThenReturnBool.Item);
467+
if (!If->hasInitStorage() && !If->hasVarStorage()) {
468+
ExprAndBool ThenReturnBool =
469+
checkSingleStatement(If->getThen(), parseReturnLiteralBool);
470+
if (ThenReturnBool &&
471+
ThenReturnBool.Bool != TrailingReturnBool.Bool) {
472+
if (Check->ChainedConditionalReturn ||
473+
(!PrevIf && If->getElse() == nullptr)) {
474+
Check->replaceCompoundReturnWithCondition(
475+
Context, cast<ReturnStmt>(*Second), TrailingReturnBool.Bool,
476+
If, ThenReturnBool.Item);
477+
}
472478
}
473479
}
474480
} else if (isa<LabelStmt, CaseStmt, DefaultStmt>(*First)) {
@@ -481,7 +487,8 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
481487
: isa<CaseStmt>(*First) ? cast<CaseStmt>(*First)->getSubStmt()
482488
: cast<DefaultStmt>(*First)->getSubStmt();
483489
auto *SubIf = dyn_cast<IfStmt>(SubStmt);
484-
if (SubIf && !SubIf->getElse()) {
490+
if (SubIf && !SubIf->getElse() && !SubIf->hasInitStorage() &&
491+
!SubIf->hasVarStorage()) {
485492
ExprAndBool ThenReturnBool =
486493
checkSingleStatement(SubIf->getThen(), parseReturnLiteralBool);
487494
if (ThenReturnBool &&
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// RUN: clang-tidy %s -checks='-*,readability-simplify-boolean-expr' -- -std=c++17 | count 0
2+
struct RAII {};
3+
bool foo(bool Cond) {
4+
bool Result;
5+
6+
if (RAII Object; Cond)
7+
Result = true;
8+
else
9+
Result = false;
10+
11+
if (bool X = Cond; X)
12+
Result = true;
13+
else
14+
Result = false;
15+
16+
if (bool X = Cond; X)
17+
return true;
18+
return false;
19+
}

clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,14 @@ void lambda_conditional_return_statements() {
478478
// CHECK-FIXES-NEXT: {{^}} };{{$}}
479479
}
480480

481+
bool condition_variable_return_stmt(int i) {
482+
// Unchanged: condition variable.
483+
if (bool Res = i == 0)
484+
return true;
485+
else
486+
return false;
487+
}
488+
481489
void simple_conditional_assignment_statements(int i) {
482490
bool b;
483491
if (i > 10)
@@ -594,6 +602,13 @@ void complex_conditional_assignment_statements(int i) {
594602
h = true;
595603
} else
596604
h = false;
605+
606+
// Unchanged: condition variable.
607+
bool k;
608+
if (bool Res = j > 10)
609+
k = true;
610+
else
611+
k = false;
597612
}
598613

599614
// Unchanged: chained return statements, but ChainedConditionalReturn not set.

0 commit comments

Comments
 (0)