diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp index 1a9c161068030..8e67e98530fcf 100644 --- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp @@ -359,7 +359,7 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor { bool VisitIfStmt(IfStmt *If) { // 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()) + if (If->hasVarStorage() || If->isConsteval()) return true; /* * if (true) ThenStmt(); -> ThenStmt(); @@ -720,18 +720,40 @@ bool SimplifyBooleanExprCheck::issueDiag(const ASTContext &Context, void SimplifyBooleanExprCheck::replaceWithThenStatement( const ASTContext &Context, const IfStmt *IfStatement, const Expr *BoolLiteral) { + std::string Replacement = getText(Context, *IfStatement->getThen()).str(); + + // Fix: Check if the statement ends with ; or } (ignoring trailing whitespace) + StringRef Trimmed = StringRef(Replacement).trim(); + if (!Trimmed.empty() && Trimmed.back() != ';' && Trimmed.back() != '}') + Replacement += ";"; + + if (const Stmt *Init = IfStatement->getInit()) { + Replacement = + (Twine("{ ") + getText(Context, *Init) + " " + Replacement + " }") + .str(); + } issueDiag(Context, BoolLiteral->getBeginLoc(), SimplifyConditionDiagnostic, - IfStatement->getSourceRange(), - getText(Context, *IfStatement->getThen())); + IfStatement->getSourceRange(), Replacement); } void SimplifyBooleanExprCheck::replaceWithElseStatement( const ASTContext &Context, const IfStmt *IfStatement, const Expr *BoolLiteral) { const Stmt *ElseStatement = IfStatement->getElse(); + std::string Replacement = + ElseStatement ? getText(Context, *ElseStatement).str() : ""; + + StringRef Trimmed = StringRef(Replacement).trim(); + if (!Trimmed.empty() && Trimmed.back() != ';' && Trimmed.back() != '}') + Replacement += ";"; + + if (const Stmt *Init = IfStatement->getInit()) { + Replacement = + (Twine("{ ") + getText(Context, *Init) + " " + Replacement + " }") + .str(); + } issueDiag(Context, BoolLiteral->getBeginLoc(), SimplifyConditionDiagnostic, - IfStatement->getSourceRange(), - ElseStatement ? getText(Context, *ElseStatement) : ""); + IfStatement->getSourceRange(), Replacement); } void SimplifyBooleanExprCheck::replaceWithCondition( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index d2b7642bdf01e..f935875198b5f 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -616,6 +616,11 @@ Changes in existing checks where the check would sometimes suggest deleting not only a redundant ``return`` or ``continue``, but also unrelated lines preceding it. +- Improved :doc:`readability-simplify-boolean-expr + ` check to avoid + invalid code generation when the condition contains an initialization + statement. + - Improved :doc:`readability-uppercase-literal-suffix ` check to recognize literal suffixes added in C++23 and C23. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-case.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-case.cpp index 2b3bf2e46d4c2..6a7bbaae44f28 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-case.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-case.cpp @@ -6,60 +6,73 @@ bool switch_stmt(int i, int j, bool b) { if (b == true) j = 10; break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(b\)}} - // CHECK-FIXES-NEXT: {{j = 10;}} + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: redundant boolean literal supplied to boolean operator [readability-simplify-boolean-expr] + // CHECK-FIXES: case 0: + // CHECK-FIXES-NEXT: if (b) + // CHECK-FIXES-NEXT: j = 10; + // CHECK-FIXES-NEXT: break; case 1: if (b == false) j = -20; break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(!b\)}} - // CHECK-FIXES-NEXT: {{j = -20;}} + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: redundant boolean literal supplied to boolean operator [readability-simplify-boolean-expr] + // CHECK-FIXES: case 1: + // CHECK-FIXES-NEXT: if (!b) + // CHECK-FIXES-NEXT: j = -20; + // CHECK-FIXES-NEXT: break; case 2: if (b && true) j = 10; break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(b\)}} - // CHECK-FIXES-NEXT: {{j = 10;}} + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: redundant boolean literal supplied to boolean operator [readability-simplify-boolean-expr] + // CHECK-FIXES: case 2: + // CHECK-FIXES-NEXT: if (b) + // CHECK-FIXES-NEXT: j = 10; + // CHECK-FIXES-NEXT: break; case 3: if (b && false) j = -20; break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(false\)}} - // CHECK-FIXES-NEXT: {{j = -20;}} + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: redundant boolean literal supplied to boolean operator [readability-simplify-boolean-expr] + // CHECK-FIXES: case 3: + // CHECK-FIXES-NEXT: if (false) + // CHECK-FIXES-NEXT: j = -20; + // CHECK-FIXES-NEXT: break; case 4: if (b || true) j = 10; break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(true\)}} - // CHECK-FIXES-NEXT: {{j = 10;}} - // CHECK-FIXES-NEXT: {{break;}} + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: redundant boolean literal supplied to boolean operator [readability-simplify-boolean-expr] + // CHECK-FIXES: case 4: + // CHECK-FIXES-NEXT: if (true) + // CHECK-FIXES-NEXT: j = 10; + // CHECK-FIXES-NEXT: break; case 5: if (b || false) j = -20; break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(b\)}} - // CHECK-FIXES-NEXT: {{j = -20;}} + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: redundant boolean literal supplied to boolean operator [readability-simplify-boolean-expr] + // CHECK-FIXES: case 5: + // CHECK-FIXES-NEXT: if (b) + // CHECK-FIXES-NEXT: j = -20; + // CHECK-FIXES-NEXT: break; case 6: return i > 0 ? true : false; - // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}} in ternary expression result - // CHECK-FIXES: {{return i > 0;}} + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: redundant boolean literal in ternary expression result [readability-simplify-boolean-expr] + // CHECK-FIXES: case 6: + // CHECK-FIXES-NEXT: return i > 0; case 7: return i > 0 ? false : true; - // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}} in ternary expression result - // CHECK-FIXES: {{return i <= 0;}} + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: redundant boolean literal in ternary expression result [readability-simplify-boolean-expr] + // CHECK-FIXES: case 7: + // CHECK-FIXES-NEXT: return i <= 0; case 8: if (true) @@ -67,9 +80,10 @@ bool switch_stmt(int i, int j, bool b) { else j = -20; break; - // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in if statement condition - // CHECK-FIXES: {{j = 10;$}} - // CHECK-FIXES-NEXT: {{break;$}} + // CHECK-MESSAGES: :[[@LINE-5]]:{{[0-9]+}}: warning: redundant boolean literal in if statement condition [readability-simplify-boolean-expr] + // CHECK-FIXES: case 8: + // CHECK-FIXES-NEXT: j = 10;; + // CHECK-FIXES-NEXT: break; case 9: if (false) @@ -77,25 +91,28 @@ bool switch_stmt(int i, int j, bool b) { else j = 10; break; - // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in if statement condition - // CHECK-FIXES: {{j = 10;}} - // CHECK-FIXES-NEXT: {{break;}} + // CHECK-MESSAGES: :[[@LINE-5]]:{{[0-9]+}}: warning: redundant boolean literal in if statement condition [readability-simplify-boolean-expr] + // CHECK-FIXES: case 9: + // CHECK-FIXES-NEXT: j = 10;; + // CHECK-FIXES-NEXT: break; case 10: if (j > 10) return true; else return false; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} in conditional return statement - // CHECK-FIXES: {{return j > 10;}} + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: redundant boolean literal in conditional return statement [readability-simplify-boolean-expr] + // CHECK-FIXES: case 10: + // CHECK-FIXES-NEXT: return j > 10; case 11: if (j > 10) return false; else return true; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} in conditional return statement - // CHECK-FIXES: {{return j <= 10;}} + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: redundant boolean literal in conditional return statement [readability-simplify-boolean-expr] + // CHECK-FIXES: case 11: + // CHECK-FIXES-NEXT: return j <= 10; case 12: if (j > 10) @@ -103,9 +120,10 @@ bool switch_stmt(int i, int j, bool b) { else b = false; return b; - // CHECK-MESSAGES: :[[@LINE-4]]:11: warning: {{.*}} in conditional assignment - // CHECK-FIXES: {{b = j > 10;}} - // CHECK-FIXES-NEXT: {{return b;}} + // CHECK-MESSAGES: :[[@LINE-4]]:{{[0-9]+}}: warning: redundant boolean literal in conditional assignment [readability-simplify-boolean-expr] + // CHECK-FIXES: case 12: + // CHECK-FIXES-NEXT: b = j > 10; + // CHECK-FIXES-NEXT: return b; case 13: if (j > 10) @@ -113,550 +131,121 @@ bool switch_stmt(int i, int j, bool b) { else b = true; return b; - // CHECK-MESSAGES: :[[@LINE-4]]:11: warning: {{.*}} in conditional assignment - // CHECK-FIXES: {{b = j <= 10;}} - // CHECK-FIXES-NEXT: {{return b;}} + // CHECK-MESSAGES: :[[@LINE-4]]:{{[0-9]+}}: warning: redundant boolean literal in conditional assignment [readability-simplify-boolean-expr] + // CHECK-FIXES: case 13: + // CHECK-FIXES-NEXT: b = j <= 10; + // CHECK-FIXES-NEXT: return b; case 14: if (j > 10) return true; return false; - // CHECK-MESSAGES: :[[@LINE-2]]:14: warning: {{.*}} in conditional return - // FIXES: {{return j > 10;}} + // CHECK-MESSAGES: :[[@LINE-2]]:{{[0-9]+}}: warning: redundant boolean literal in conditional return statement [readability-simplify-boolean-expr] + // CHECK-FIXES: case 14: + // CHECK-FIXES-NEXT: return j > 10; case 15: if (j > 10) return false; return true; - // CHECK-MESSAGES: :[[@LINE-2]]:14: warning: {{.*}} in conditional return - // FIXES: {{return j <= 10;}} - - case 16: - if (j > 10) - return true; - return true; - return false; - - case 17: - if (j > 10) - return false; - return false; - return true; - - case 100: { - if (b == true) - j = 10; - break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(b\)}} - // CHECK-FIXES-NEXT: {{j = 10;}} - } - - case 101: { - if (b == false) - j = -20; - break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(!b\)}} - // CHECK-FIXES-NEXT: {{j = -20;}} - } - - case 102: { - if (b && true) - j = 10; - break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(b\)}} - // CHECK-FIXES-NEXT: {{j = 10;}} - } - - case 103: { - if (b && false) - j = -20; - break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(false\)}} - // CHECK-FIXES-NEXT: {{j = -20;}} - } - - case 104: { - if (b || true) - j = 10; - break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(true\)}} - // CHECK-FIXES-NEXT: {{j = 10;}} - // CHECK-FIXES-NEXT: {{break;}} - } - - case 105: { - if (b || false) - j = -20; - break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(b\)}} - // CHECK-FIXES-NEXT: {{j = -20;}} - } - - case 106: { - return i > 0 ? true : false; - // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}} in ternary expression result - // CHECK-FIXES: {{return i > 0;}} - } - - case 107: { - return i > 0 ? false : true; - // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}} in ternary expression result - // CHECK-FIXES: {{return i <= 0;}} - } - - case 108: { - if (true) - j = 10; - else - j = -20; - break; - // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in if statement condition - // CHECK-FIXES: {{j = 10;$}} - // CHECK-FIXES-NEXT: {{break;$}} - } - - case 109: { - if (false) - j = -20; - else - j = 10; - break; - // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in if statement condition - // CHECK-FIXES: {{j = 10;}} - // CHECK-FIXES-NEXT: {{break;}} - } - - case 110: { - if (j > 10) - return true; - else - return false; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} in conditional return statement - // CHECK-FIXES: {{return j > 10;}} - } - - case 111: { - if (j > 10) - return false; - else - return true; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} in conditional return statement - // CHECK-FIXES: {{return j <= 10;}} - } - - case 112: { - if (j > 10) - b = true; - else - b = false; - return b; - // CHECK-MESSAGES: :[[@LINE-4]]:11: warning: {{.*}} in conditional assignment - // CHECK-FIXES: {{b = j > 10;}} - // CHECK-FIXES-NEXT: {{return b;}} - } - - case 113: { - if (j > 10) - b = false; - else - b = true; - return b; - // CHECK-MESSAGES: :[[@LINE-4]]:11: warning: {{.*}} in conditional assignment - // CHECK-FIXES: {{b = j <= 10;}} - // CHECK-FIXES-NEXT: {{return b;}} - } - - case 114: { - if (j > 10) - return true; - return false; - // CHECK-MESSAGES: :[[@LINE-2]]:14: warning: {{.*}} in conditional return - // CHECK-FIXES: {{return j > 10;}} - } - - case 115: { - if (j > 10) - return false; - return true; - // CHECK-MESSAGES: :[[@LINE-2]]:14: warning: {{.*}} in conditional return - // CHECK-FIXES: {{return j <= 10;}} - } - - case 116: { - return false; - if (j > 10) - return true; - } - - case 117: { - return true; - if (j > 10) - return false; - } - } - - return j > 0; -} - -bool default_stmt0(int i, int j, bool b) { - switch (i) { - case 0: - return true; + // CHECK-MESSAGES: :[[@LINE-2]]:{{[0-9]+}}: warning: redundant boolean literal in conditional return statement [readability-simplify-boolean-expr] + // CHECK-FIXES: case 15: + // CHECK-FIXES-NEXT: return j <= 10; default: if (b == true) j = 10; break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(b\)}} - // CHECK-FIXES-NEXT: {{j = 10;}} - } - return false; -} - -bool default_stmt1(int i, int j, bool b) { - switch (i) { - case 0: - return true; - - default: - if (b == false) - j = -20; - break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(!b\)}} - // CHECK-FIXES-NEXT: {{j = -20;}} - } - return false; -} - -bool default_stmt2(int i, int j, bool b) { - switch (i) { - case 0: - return true; - - default: - if (b && true) - j = 10; - break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(b\)}} - // CHECK-FIXES-NEXT: {{j = 10;}} - } - return false; -} - -bool default_stmt3(int i, int j, bool b) { - switch (i) { - case 0: - return true; - - default: - if (b && false) - j = -20; - break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(false\)}} - // CHECK-FIXES-NEXT: {{j = -20;}} - } - return false; -} - -bool default_stmt4(int i, int j, bool b) { - switch (i) { - case 0: - return true; - - default: - if (b || true) - j = 10; - break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(true\)}} - // CHECK-FIXES-NEXT: {{j = 10;}} - // CHECK-FIXES-NEXT: {{break;}} - } - return false; -} - -bool default_stmt5(int i, int j, bool b) { - switch (i) { - case 0: - return true; - - default: - if (b || false) - j = -20; - break; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(b\)}} - // CHECK-FIXES-NEXT: {{j = -20;}} - } - return false; -} - -bool default_stmt6(int i, int j, bool b) { - switch (i) { - case 0: - return true; - - default: - return i > 0 ? true : false; - // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}} in ternary expression result - // CHECK-FIXES: {{return i > 0;}} - } - return false; -} - -bool default_stmt7(int i, int j, bool b) { - switch (i) { - case 0: - return true; - - default: - return i > 0 ? false : true; - // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}} in ternary expression result - // CHECK-FIXES: {{return i <= 0;}} - } - return false; -} - -bool default_stmt8(int i, int j, bool b) { - switch (i) { - case 0: - return true; - - default: - if (true) - j = 10; - else - j = -20; - break; - // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in if statement condition - // CHECK-FIXES: {{j = 10;$}} - // CHECK-FIXES-NEXT: {{break;$}} - } - return false; -} - -bool default_stmt9(int i, int j, bool b) { - switch (i) { - case 0: - return true; - - default: - if (false) - j = -20; - else - j = 10; - break; - // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in if statement condition - // CHECK-FIXES: {{j = 10;}} - // CHECK-FIXES-NEXT: {{break;}} - } - return false; -} - -bool default_stmt10(int i, int j, bool b) { - switch (i) { - case 0: - return true; - - default: - if (j > 10) - return true; - else - return false; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} in conditional return statement - // CHECK-FIXES: {{return j > 10;}} - } - return false; -} - -bool default_stmt11(int i, int j, bool b) { - switch (i) { - case 0: - return true; - - default: - if (j > 10) - return false; - else - return true; - // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} in conditional return statement - // CHECK-FIXES: {{return j <= 10;}} - } - return false; -} - -bool default_stmt12(int i, int j, bool b) { - switch (i) { - case 0: - return true; - - default: - if (j > 10) - b = true; - else - b = false; - return b; - // CHECK-MESSAGES: :[[@LINE-4]]:11: warning: {{.*}} in conditional assignment - // CHECK-FIXES: {{b = j > 10;}} - // CHECK-FIXES-NEXT: {{return b;}} - } - return false; -} - -bool default_stmt13(int i, int j, bool b) { - switch (i) { - case 0: - return true; - - default: - if (j > 10) - b = false; - else - b = true; - return b; - // CHECK-MESSAGES: :[[@LINE-4]]:11: warning: {{.*}} in conditional assignment - // CHECK-FIXES: {{b = j <= 10;}} - // CHECK-FIXES-NEXT: {{return b;}} - } - return false; -} - -bool default_stmt14(int i, int j, bool b) { - switch (i) { - case 0: - return true; - - default: - if (j > 10) - return true; - return false; - // CHECK-MESSAGES: :[[@LINE-2]]:14: warning: {{.*}} in conditional return - // FIXES: {{return j > 10;}} + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: redundant boolean literal supplied to boolean operator [readability-simplify-boolean-expr] + // CHECK-FIXES: default: + // CHECK-FIXES-NEXT: if (b) + // CHECK-FIXES-NEXT: j = 10; + // CHECK-FIXES-NEXT: break; } - return false; -} - -bool default_stmt15(int i, int j, bool b) { - switch (i) { - case 0: - return true; - - default: - if (j > 10) - return false; - return true; - // CHECK-MESSAGES: :[[@LINE-2]]:14: warning: {{.*}} in conditional return - // FIXES: {{return j <= 10;}} - } - return false; -} - -bool default_stmt16(int i, int j, bool b) { - switch (i) { - case 0: - return false; - - default: - if (j > 10) - return true; - } - return false; -} - -bool default_stmt17(int i, int j, bool b) { - switch (i) { - case 0: - return true; - - default: - if (j > 10) - return false; - } - return false; } bool label_stmt0(int i, int j, bool b) { label: if (b == true) j = 10; - // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(b\)}} - // CHECK-FIXES-NEXT: {{j = 10;}} - return false; + // CHECK-MESSAGES: :[[@LINE-2]]:{{[0-9]+}}: warning: redundant boolean literal supplied to boolean operator [readability-simplify-boolean-expr] + // CHECK-FIXES: if (b) + // CHECK-FIXES-NEXT: j = 10; + // + // } bool label_stmt1(int i, int j, bool b) { label: if (b == false) j = -20; - // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(!b\)}} - // CHECK-FIXES-NEXT: {{j = -20;}} - return false; + // CHECK-MESSAGES: :[[@LINE-2]]:{{[0-9]+}}: warning: redundant boolean literal supplied to boolean operator [readability-simplify-boolean-expr] + // CHECK-FIXES: if (!b) + // CHECK-FIXES-NEXT: j = -20; + // + // } bool label_stmt2(int i, int j, bool b) { label: if (b && true) j = 10; - // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(b\)}} - // CHECK-FIXES-NEXT: {{j = 10;}} - return false; + // CHECK-MESSAGES: :[[@LINE-2]]:{{[0-9]+}}: warning: redundant boolean literal supplied to boolean operator [readability-simplify-boolean-expr] + // CHECK-FIXES: if (b) + // CHECK-FIXES-NEXT: j = 10; + // + // } bool label_stmt3(int i, int j, bool b) { label: if (b && false) j = -20; - // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(false\)}} - // CHECK-FIXES-NEXT: {{j = -20;}} - return false; + // CHECK-MESSAGES: :[[@LINE-2]]:{{[0-9]+}}: warning: redundant boolean literal supplied to boolean operator [readability-simplify-boolean-expr] + // CHECK-FIXES: if (false) + // CHECK-FIXES-NEXT: j = -20; + // + // } bool label_stmt4(int i, int j, bool b) { label: if (b || true) j = 10; - // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(true\)}} - // CHECK-FIXES-NEXT: {{j = 10;}} - return false; + // CHECK-MESSAGES: :[[@LINE-2]]:{{[0-9]+}}: warning: redundant boolean literal supplied to boolean operator [readability-simplify-boolean-expr] + // CHECK-FIXES: if (true) + // CHECK-FIXES-NEXT: j = 10; + // + // } bool label_stmt5(int i, int j, bool b) { label: if (b || false) j = -20; - // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: {{.*}} to boolean operator - // CHECK-FIXES: {{if \(b\)}} - // CHECK-FIXES-NEXT: {{j = -20;}} - return false; + // CHECK-MESSAGES: :[[@LINE-2]]:{{[0-9]+}}: warning: redundant boolean literal supplied to boolean operator [readability-simplify-boolean-expr] + // CHECK-FIXES: if (b) + // CHECK-FIXES-NEXT: j = -20; + // + // } bool label_stmt6(int i, int j, bool b) { label: return i > 0 ? true : false; - // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: {{.*}} in ternary expression result - // CHECK-FIXES: {{return i > 0;}} + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: redundant boolean literal in ternary expression result [readability-simplify-boolean-expr] + // CHECK-FIXES: return i > 0; + // + // } bool label_stmt7(int i, int j, bool b) { label: return i > 0 ? false : true; - // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: {{.*}} in ternary expression result - // CHECK-FIXES: {{return i <= 0;}} + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: redundant boolean literal in ternary expression result [readability-simplify-boolean-expr] + // CHECK-FIXES: return i <= 0; + // + // } bool label_stmt8(int i, int j, bool b) { @@ -665,8 +254,10 @@ bool label_stmt8(int i, int j, bool b) { j = 10; else j = -20; - // CHECK-MESSAGES: :[[@LINE-4]]:7: warning: {{.*}} in if statement condition - // CHECK-FIXES: {{j = 10;$}} + // CHECK-MESSAGES: :[[@LINE-4]]:{{[0-9]+}}: warning: redundant boolean literal in if statement condition [readability-simplify-boolean-expr] + // CHECK-FIXES: j = 10;; + // + // return false; } @@ -676,8 +267,10 @@ bool label_stmt9(int i, int j, bool b) { j = -20; else j = 10; - // CHECK-MESSAGES: :[[@LINE-4]]:7: warning: {{.*}} in if statement condition - // CHECK-FIXES: {{j = 10;}} + // CHECK-MESSAGES: :[[@LINE-4]]:{{[0-9]+}}: warning: redundant boolean literal in if statement condition [readability-simplify-boolean-expr] + // CHECK-FIXES: j = 10;; + // + // return false; } @@ -687,8 +280,10 @@ bool label_stmt10(int i, int j, bool b) { return true; else return false; - // CHECK-MESSAGES: :[[@LINE-3]]:12: warning: {{.*}} in conditional return statement - // CHECK-FIXES: {{return j > 10;}} + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: redundant boolean literal in conditional return statement [readability-simplify-boolean-expr] + // CHECK-FIXES: return j > 10; + // + // } bool label_stmt11(int i, int j, bool b) { @@ -697,8 +292,10 @@ bool label_stmt11(int i, int j, bool b) { return false; else return true; - // CHECK-MESSAGES: :[[@LINE-3]]:12: warning: {{.*}} in conditional return statement - // CHECK-FIXES: {{return j <= 10;}} + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: redundant boolean literal in conditional return statement [readability-simplify-boolean-expr] + // CHECK-FIXES: return j <= 10; + // + // } bool label_stmt12(int i, int j, bool b) { @@ -708,9 +305,11 @@ bool label_stmt12(int i, int j, bool b) { else b = false; return b; - // CHECK-MESSAGES: :[[@LINE-4]]:9: warning: {{.*}} in conditional assignment - // CHECK-FIXES: {{b = j > 10;}} - // CHECK-FIXES-NEXT: {{return b;}} + // CHECK-MESSAGES: :[[@LINE-4]]:{{[0-9]+}}: warning: redundant boolean literal in conditional assignment [readability-simplify-boolean-expr] + // CHECK-FIXES: b = j > 10; + // CHECK-FIXES-NEXT: return b; + // + // } bool label_stmt13(int i, int j, bool b) { @@ -720,9 +319,11 @@ bool label_stmt13(int i, int j, bool b) { else b = true; return b; - // CHECK-MESSAGES: :[[@LINE-4]]:9: warning: {{.*}} in conditional assignment - // CHECK-FIXES: {{b = j <= 10;}} - // CHECK-FIXES-NEXT: {{return b;}} + // CHECK-MESSAGES: :[[@LINE-4]]:{{[0-9]+}}: warning: redundant boolean literal in conditional assignment [readability-simplify-boolean-expr] + // CHECK-FIXES: b = j <= 10; + // CHECK-FIXES-NEXT: return b; + // + // } bool label_stmt14(int i, int j, bool b) { @@ -730,8 +331,8 @@ bool label_stmt14(int i, int j, bool b) { if (j > 10) return true; return false; - // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: {{.*}} in conditional return - // FIXES: {{return j > 10;}} + // CHECK-MESSAGES: :[[@LINE-2]]:{{[0-9]+}}: warning: redundant boolean literal in conditional return statement [readability-simplify-boolean-expr] + // CHECK-FIXES: return j > 10; } bool label_stmt15(int i, int j, bool b) { @@ -739,6 +340,6 @@ bool label_stmt15(int i, int j, bool b) { if (j > 10) return false; return true; - // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: {{.*}} in conditional return - // FIXES: {{return j <= 10;}} + // CHECK-MESSAGES: :[[@LINE-2]]:{{[0-9]+}}: warning: redundant boolean literal in conditional return statement [readability-simplify-boolean-expr] + // CHECK-FIXES: return j <= 10; } diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr.cpp index 0b99cb89262cd..cafc3c9c8fd22 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr.cpp @@ -1035,3 +1035,36 @@ void instantiate() { ignoreInstantiations(); ignoreInstantiations(); } + +void test_init_stmt_true() { + void foo(int i); + if (int i = 0; true) + foo(i); + // CHECK-MESSAGES: :[[@LINE-2]]:18: warning: redundant boolean literal in if statement condition [readability-simplify-boolean-expr] + // CHECK-FIXES: { int i = 0; foo(i); }; +} + +void if_with_init_statement() { + bool x = true; + if (bool y = x; y == true) { + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: redundant boolean literal supplied to boolean operator [readability-simplify-boolean-expr] + // CHECK-FIXES: if (bool y = x; y) { + } +} + +// This matches the "RAII" and "Cond" logic from the deleted C++17 file +// to ensure we don't regress or crash on these complex cases. +void test_cxx17_raii_and_complex() { + struct RAII {}; + bool Cond = true; + // Case 1: Init statement with non-boolean type + if (RAII Object; Cond) { + // No warning expected here for now, just verifying no crash. + } + // Case 2: Init statement declaring the condition variable + if (bool X = Cond; X) { + // No warning expected, verifying no crash. + } +} + +