[clang-tools-extra] ca1eefd - [clang-tidy] Correctly handle attributes in readability-inconsistent-ifelse-braces (#184095)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 6 20:17:20 PST 2026
Author: mitchell
Date: 2026-03-07T12:17:16+08:00
New Revision: ca1eefdfc061fc062b6b9dc4149ca4a0e51bde08
URL: https://github.com/llvm/llvm-project/commit/ca1eefdfc061fc062b6b9dc4149ca4a0e51bde08
DIFF: https://github.com/llvm/llvm-project/commit/ca1eefdfc061fc062b6b9dc4149ca4a0e51bde08.diff
LOG: [clang-tidy] Correctly handle attributes in readability-inconsistent-ifelse-braces (#184095)
Improved the check to correctly handle `[[likely]]` and `[[unlikely]]`
attributes placed between the if/else keyword and the opening brace.
As of AI Usage: Gemini 3 is used for pre-commit reviewing.
Closes https://github.com/llvm/llvm-project/issues/184081
Added:
Modified:
clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp
clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp
clang-tools-extra/test/clang-tidy/checkers/readability/inconsistent-ifelse-braces-attributes.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp b/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
index 5207c99c10d83..1864dfcbd29c2 100644
--- a/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -147,7 +147,7 @@ BracesAroundStatementsCheck::findRParenLoc(const IfOrWhileStmt *S,
bool BracesAroundStatementsCheck::checkStmt(
const MatchFinder::MatchResult &Result, const Stmt *S,
SourceLocation StartLoc, SourceLocation EndLocHint) {
- while (const auto *AS = dyn_cast<AttributedStmt>(S))
+ if (const auto *AS = dyn_cast<AttributedStmt>(S))
S = AS->getSubStmt();
const auto BraceInsertionHints = utils::getBraceInsertionsHints(
diff --git a/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp b/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp
index 6cc1b203470f1..fc22e36a67c54 100644
--- a/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp
@@ -18,13 +18,21 @@ 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 = If->getThen();
+ const Stmt *const Then = ignoreAttributed(If->getThen());
if (isa<CompoundStmt>(Then))
return true;
- if (const Stmt *const Else = If->getElse()) {
+ if (const Stmt *Else = If->getElse()) {
+ Else = ignoreAttributed(Else);
if (const auto *NestedIf = dyn_cast<const IfStmt>(Else))
return shouldHaveBraces(NestedIf);
@@ -53,7 +61,7 @@ void InconsistentIfElseBracesCheck::check(
void InconsistentIfElseBracesCheck::checkIfStmt(
const MatchFinder::MatchResult &Result, const IfStmt *If) {
- const Stmt *Then = If->getThen();
+ const Stmt *Then = ignoreAttributed(If->getThen());
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.
@@ -62,10 +70,11 @@ void InconsistentIfElseBracesCheck::checkIfStmt(
if (shouldHaveBraces(NestedIf))
checkIfStmt(Result, NestedIf);
} else if (!isa<CompoundStmt>(Then)) {
- emitDiagnostic(Result, Then, If->getRParenLoc(), If->getElseLoc());
+ emitDiagnostic(Result, If->getThen(), If->getRParenLoc(), If->getElseLoc());
}
- if (const Stmt *const Else = If->getElse()) {
+ if (const Stmt *Else = If->getElse()) {
+ Else = ignoreAttributed(Else);
if (const auto *NestedIf = dyn_cast<const IfStmt>(Else))
checkIfStmt(Result, NestedIf);
else if (!isa<CompoundStmt>(Else))
diff --git a/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp b/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp
index 5f804793044e1..d51bfd9362eb7 100644
--- a/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp
+++ b/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp
@@ -127,12 +127,24 @@ BraceInsertionHints getBraceInsertionsHints(const Stmt *const S,
if (StartLoc.isInvalid())
return {};
+ const Stmt *InnerS = S;
+ while (const auto *AS = dyn_cast<AttributedStmt>(InnerS))
+ InnerS = AS->getSubStmt();
+
+ SourceLocation InsertLoc = StartLoc;
+ if (S != InnerS) {
+ if (std::optional<Token> Tok = utils::lexer::getPreviousToken(
+ InnerS->getBeginLoc(), SM, LangOpts, /*SkipComments=*/true)) {
+ InsertLoc = Tok->getLocation();
+ }
+ }
+
// Convert StartLoc to file location, if it's on the same macro expansion
// level as the start of the statement. We also need file locations for
// Lexer::getLocForEndOfToken working properly.
- StartLoc = Lexer::makeFileCharRange(
- CharSourceRange::getCharRange(StartLoc, S->getBeginLoc()), SM,
- LangOpts)
+ StartLoc = Lexer::makeFileCharRange(CharSourceRange::getCharRange(
+ InsertLoc, InnerS->getBeginLoc()),
+ SM, LangOpts)
.getBegin();
if (StartLoc.isInvalid())
return {};
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/inconsistent-ifelse-braces-attributes.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/inconsistent-ifelse-braces-attributes.cpp
index 8fdb574227028..01965116c3272 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/inconsistent-ifelse-braces-attributes.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/inconsistent-ifelse-braces-attributes.cpp
@@ -5,15 +5,29 @@ void f(bool b) {
if (b) [[likely]] return;
else {
}
- // CHECK-MESSAGES: :[[@LINE-3]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces]
- // CHECK-FIXES: if (b) { {{[[][[]}}likely{{[]][]]}} return;
+ // CHECK-MESSAGES: :[[@LINE-3]]:20: warning: statement should have braces [readability-inconsistent-ifelse-braces]
+ // CHECK-FIXES: if (b) {{[[][[]}}likely{{[]][]]}} { return;
// CHECK-FIXES: } else {
if (b) {
} else [[unlikely]]
return;
- // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces]
- // CHECK-FIXES: } else { {{[[][[]}}unlikely{{[]][]]}}
+ // CHECK-MESSAGES: :[[@LINE-2]]:22: warning: statement should have braces [readability-inconsistent-ifelse-braces]
+ // CHECK-FIXES: } else {{[[][[]}}unlikely{{[]][]]}} {
+
+ if (b) [[likely]] {
+ } else [[unlikely]]
+ return;
+ // CHECK-MESSAGES: :[[@LINE-2]]:22: warning: statement should have braces [readability-inconsistent-ifelse-braces]
+ // CHECK-FIXES: } else {{[[][[]}}unlikely{{[]][]]}} {
+
+ if (b) [[likely]]
+ return;
+ else [[unlikely]] {
+ }
+ // CHECK-MESSAGES: :[[@LINE-4]]:20: warning: statement should have braces [readability-inconsistent-ifelse-braces]
+ // CHECK-FIXES: if (b) {{[[][[]}}likely{{[]][]]}} {
+ // CHECK-FIXES: } else {{[[][[]}}unlikely{{[]][]]}} {
}
// Negative tests.
@@ -39,4 +53,36 @@ void g(bool b) {
return;
else [[likely]]
return;
+
+ if (b) [[likely]] {
+ return;
+ } else {
+ return;
+ }
+
+ if (b) {
+ return;
+ } else [[unlikely]] {
+ return;
+ }
+
+ if (b) [[likely]] {
+ return;
+ } else [[unlikely]] {
+ return;
+ }
+
+ if (b) [[likely]] {
+ return;
+ } else if (b) [[unlikely]] {
+ return;
+ } else {
+ return;
+ }
+
+ if (b) [[likely]] [[likely]] {
+ return;
+ } else [[unlikely]] [[unlikely]] {
+ return;
+ }
}
More information about the cfe-commits
mailing list