[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
This commit is contained in:
mitchell 2026-03-07 12:17:16 +08:00 committed by GitHub
parent 69902769c7
commit ca1eefdfc0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 80 additions and 13 deletions

View File

@ -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(

View File

@ -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))

View File

@ -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 {};

View File

@ -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;
}
}