[clang-tools-extra] [clang-tidy] Correctly handle attributes in readability-inconsistent-ifelse-braces (PR #184095)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 5 06:25:14 PST 2026
https://github.com/zeyi2 updated https://github.com/llvm/llvm-project/pull/184095
>From 8b3e1e7d334a86aa649fed91ff928d480f7a874f Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Mon, 2 Mar 2026 18:49:43 +0800
Subject: [PATCH 1/4] [clang-tidy] Correctly handle attributes in
readability-inconsistent-ifelse-braces
---
.../InconsistentIfElseBracesCheck.cpp | 21 ++++++---
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++
.../inconsistent-ifelse-braces-attributes.cpp | 46 +++++++++++++++++++
3 files changed, 66 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp b/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp
index 6cc1b203470f1..5cf52088917a9 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,13 +70,14 @@ 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 auto *NestedIf = dyn_cast<const IfStmt>(Else))
+ const Stmt *UnwrappedElse = ignoreAttributed(Else);
+ if (const auto *NestedIf = dyn_cast<const IfStmt>(UnwrappedElse))
checkIfStmt(Result, NestedIf);
- else if (!isa<CompoundStmt>(Else))
+ else if (!isa<CompoundStmt>(UnwrappedElse))
emitDiagnostic(Result, If->getElse(), If->getElseLoc());
}
}
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index cf8dd0dba9f12..65a24f1311754 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -253,6 +253,11 @@ Changes in existing checks
now uses separate note diagnostics for each uninitialized enumerator, making
it easier to see which specific enumerators need explicit initialization.
+- Improved :doc:`readability-inconsistent-ifelse-braces
+ <clang-tidy/checks/readability/inconsistent-ifelse-braces>` check by fixing
+ false positives when ``[[likely]]`` or ``[[unlikely]]`` attributes are placed
+ between the ``if`` or ``else`` keyword and the opening brace.
+
- Improved :doc:`readability-non-const-parameter
<clang-tidy/checks/readability/non-const-parameter>` check by avoiding false
positives on parameters used in dependent expressions (e.g. inside generic
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..8e44440669d40 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
@@ -14,6 +14,20 @@ void f(bool b) {
return;
// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces]
// CHECK-FIXES: } else { {{[[][[]}}unlikely{{[]][]]}}
+
+ if (b) [[likely]] {
+ } else [[unlikely]]
+ return;
+ // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces]
+ // CHECK-FIXES: } else { {{[[][[]}}unlikely{{[]][]]}}
+
+ if (b) [[likely]]
+ return;
+ else [[unlikely]] {
+ }
+ // CHECK-MESSAGES: :[[@LINE-4]]:9: 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;
+ }
}
>From 030dca199c59130b1e25644f18c9830ef6ee48de Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Tue, 3 Mar 2026 08:45:37 +0800
Subject: [PATCH 2/4] better?? may fail CI though.
---
.../InconsistentIfElseBracesCheck.cpp | 8 ++++----
.../clang-tidy/utils/BracesAroundStatement.cpp | 18 +++++++++++++++---
clang-tools-extra/docs/ReleaseNotes.rst | 5 -----
.../inconsistent-ifelse-braces-attributes.cpp | 16 ++++++++--------
4 files changed, 27 insertions(+), 20 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp b/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp
index 5cf52088917a9..fc22e36a67c54 100644
--- a/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/InconsistentIfElseBracesCheck.cpp
@@ -73,11 +73,11 @@ void InconsistentIfElseBracesCheck::checkIfStmt(
emitDiagnostic(Result, If->getThen(), If->getRParenLoc(), If->getElseLoc());
}
- if (const Stmt *const Else = If->getElse()) {
- const Stmt *UnwrappedElse = ignoreAttributed(Else);
- if (const auto *NestedIf = dyn_cast<const IfStmt>(UnwrappedElse))
+ 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>(UnwrappedElse))
+ else if (!isa<CompoundStmt>(Else))
emitDiagnostic(Result, If->getElse(), If->getElseLoc());
}
}
diff --git a/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp b/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp
index 5f804793044e1..4034de854667f 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 = tidy::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/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 65a24f1311754..cf8dd0dba9f12 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -253,11 +253,6 @@ Changes in existing checks
now uses separate note diagnostics for each uninitialized enumerator, making
it easier to see which specific enumerators need explicit initialization.
-- Improved :doc:`readability-inconsistent-ifelse-braces
- <clang-tidy/checks/readability/inconsistent-ifelse-braces>` check by fixing
- false positives when ``[[likely]]`` or ``[[unlikely]]`` attributes are placed
- between the ``if`` or ``else`` keyword and the opening brace.
-
- Improved :doc:`readability-non-const-parameter
<clang-tidy/checks/readability/non-const-parameter>` check by avoiding false
positives on parameters used in dependent expressions (e.g. inside generic
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 8e44440669d40..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,28 +5,28 @@ 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]]: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]]
return;
else [[unlikely]] {
}
- // CHECK-MESSAGES: :[[@LINE-4]]:9: warning: statement should have braces [readability-inconsistent-ifelse-braces]
- // CHECK-FIXES: if (b) { {{[[][[]}}likely{{[]][]]}}
+ // CHECK-MESSAGES: :[[@LINE-4]]:20: warning: statement should have braces [readability-inconsistent-ifelse-braces]
+ // CHECK-FIXES: if (b) {{[[][[]}}likely{{[]][]]}} {
// CHECK-FIXES: } else {{[[][[]}}unlikely{{[]][]]}} {
}
>From a452b90d9be021c9e32bf5581b6ee2795f769949 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Tue, 3 Mar 2026 09:00:42 +0800
Subject: [PATCH 3/4] unnecessary namespace
---
clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp b/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp
index 4034de854667f..d51bfd9362eb7 100644
--- a/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp
+++ b/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp
@@ -133,7 +133,7 @@ BraceInsertionHints getBraceInsertionsHints(const Stmt *const S,
SourceLocation InsertLoc = StartLoc;
if (S != InnerS) {
- if (std::optional<Token> Tok = tidy::utils::lexer::getPreviousToken(
+ if (std::optional<Token> Tok = utils::lexer::getPreviousToken(
InnerS->getBeginLoc(), SM, LangOpts, /*SkipComments=*/true)) {
InsertLoc = Tok->getLocation();
}
>From b8cf194e4b14342afb14cd8a7d5aa248e9825d0c Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Thu, 5 Mar 2026 22:22:58 +0800
Subject: [PATCH 4/4] fixup fixup fixup fixup fixup
---
.../clang-tidy/readability/BracesAroundStatementsCheck.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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(
More information about the cfe-commits
mailing list