llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Gaurav Dhingra (gxyd) <details> <summary>Changes</summary> Fixes #<!-- -->194694 --- Full diff: https://github.com/llvm/llvm-project/pull/202869.diff 3 Files Affected: - (modified) clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp (+4-11) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) - (added) clang-tools-extra/test/clang-tidy/checkers/readability/inconsistent-ifelse-labels.cpp (+126) ``````````diff diff --git a/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp b/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp index fc22e36a67c54..732b5f199230f 100644 --- a/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp @@ -18,21 +18,14 @@ using namespace clang::ast_matchers; namespace clang::tidy::readability { -/// Look through AttributedStmt wrappers to find the underlying statement. -static const Stmt *ignoreAttributed(const Stmt *S) { - if (const auto *AS = dyn_cast<AttributedStmt>(S)) - return AS->getSubStmt(); - return S; -} - /// Check that at least one branch of the \p If statement is a \c CompoundStmt. static bool shouldHaveBraces(const IfStmt *If) { - const Stmt *const Then = ignoreAttributed(If->getThen()); + const Stmt *const Then = If->getThen()->stripLabelLikeStatements(); if (isa<CompoundStmt>(Then)) return true; if (const Stmt *Else = If->getElse()) { - Else = ignoreAttributed(Else); + Else = Else->stripLabelLikeStatements(); if (const auto *NestedIf = dyn_cast<const IfStmt>(Else)) return shouldHaveBraces(NestedIf); @@ -61,7 +54,7 @@ void InconsistentIfElseBracesCheck::check( void InconsistentIfElseBracesCheck::checkIfStmt( const MatchFinder::MatchResult &Result, const IfStmt *If) { - const Stmt *Then = ignoreAttributed(If->getThen()); + const Stmt *Then = If->getThen()->stripLabelLikeStatements(); if (const auto *NestedIf = dyn_cast<const IfStmt>(Then)) { // If the then-branch is a nested IfStmt, first we need to add braces to // it, then we need to check the inner IfStmt. @@ -74,7 +67,7 @@ void InconsistentIfElseBracesCheck::checkIfStmt( } if (const Stmt *Else = If->getElse()) { - Else = ignoreAttributed(Else); + Else = Else->stripLabelLikeStatements(); if (const auto *NestedIf = dyn_cast<const IfStmt>(Else)) checkIfStmt(Result, NestedIf); else if (!isa<CompoundStmt>(Else)) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index c369b1fd8b373..003b622e6821a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -780,6 +780,10 @@ Changes in existing checks - Fixed a false positive where ``bool`` conditions in C conditional operators were diagnosed as implicit conversions to ``int``. +- Improved :doc:`readability-inconsistent-ifelse-braces + <clang-tidy/checks/readability/inconsistent-ifelse-braces>` check to + correctly handle labeled statements of ``if``/``else`` bodies. + - Improved :doc:`readability-non-const-parameter <clang-tidy/checks/readability/non-const-parameter>` check: diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/inconsistent-ifelse-labels.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/inconsistent-ifelse-labels.cpp new file mode 100644 index 0000000000000..fcf9b1112a122 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/inconsistent-ifelse-labels.cpp @@ -0,0 +1,126 @@ +// RUN: %check_clang_tidy -std=c++98-or-later %s readability-inconsistent-ifelse-braces %t + +// Positive tests. +void f(bool b) { + if (b) goo: + return; + else too: { + return; + } + // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces] + // CHECK-FIXES: if (b) { goo: + // CHECK-FIXES-NEXT: return; + // CHECK-FIXES-NEXT: } else too: { + + if (b) xoo: { + return; + } else yoo: + return; + // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces] + // CHECK-FIXES: } else { yoo: + // CHECK-FIXES-NEXT: return; + // CHECK-FIXES-NEXT: } + + if (b) hoo: { + return; + } else loo: [[unlikely]] + return; + // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces] + // CHECK-FIXES: } else { loo: {{[[][[]}}unlikely{{[]][]]}} + // CHECK-FIXES-NEXT: return; + // CHECK-FIXES-NEXT: } + + if (b) + return; + else coo: [[unlikely]] { + return; + } + // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces] + // CHECK-FIXES: if (b) { + // CHECK-FIXES-NEXT: return; + // CHECK-FIXES-NEXT: } else coo: {{[[][[]}}unlikely{{[]][]]}} { + + if (b) aoo: + return; + else boo: [[unlikely]] { + return; + } + // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces] + // CHECK-FIXES: if (b) { aoo: + // CHECK-FIXES-NEXT: return; + // CHECK-FIXES-NEXT: } else boo: {{[[][[]}}unlikely{{[]][]]}} { + + if (b) moo: [[unlikely]] + return; + else noo: [[unlikely]] { + return; + } + // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces] + // CHECK-FIXES: if (b) { moo: {{[[][[]}}unlikely{{[]][]]}} + // CHECK-FIXES-NEXT: return; + // CHECK-FIXES-NEXT: } else noo: {{[[][[]}}unlikely{{[]][]]}} { + + if (b) poo: [[likely]] { + return; + } else qoo: [[unlikely]] + return; + // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces] + // CHECK-FIXES: } else { qoo: {{[[][[]}}unlikely{{[]][]]}} + // CHECK-FIXES-NEXT: return; + // CHECK-FIXES-NEXT: } +} + + +// Negative tests. +void g(bool b) { + if (b) { + return; + } else foo: { + return; + } + + if (b) { + return; + } else goo: [[unlikely]] { + return; + } + + if (b) { + return; + } else hoo: { + return; + } + + if (b) joo: { + return; + } else { + return; + } + + if (b) roo: [[unlikely]] { + return; + } else { + return; + } + + if (b) koo: { + return; + } else loo: { + return; + } + + if (b) moo: + return; + else noo: + return; + + if (b) + return; + else ooo: + return; + + if (b) poo: [[likely]] + return; + else qoo: [[unlikely]] + return; +} `````````` </details> https://github.com/llvm/llvm-project/pull/202869 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
