https://github.com/zeyi2 updated https://github.com/llvm/llvm-project/pull/184095
>From 8b3e1e7d334a86aa649fed91ff928d480f7a874f Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Mon, 2 Mar 2026 18:49:43 +0800 Subject: [PATCH 1/2] [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 <[email protected]> Date: Tue, 3 Mar 2026 08:45:37 +0800 Subject: [PATCH 2/2] 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{{[]][]]}} { } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
