https://github.com/berkaysahiin updated https://github.com/llvm/llvm-project/pull/181878
>From 2d48ff7dd6a55bd3b2d5a9c4f755a9e7be6a5936 Mon Sep 17 00:00:00 2001 From: Berkay <[email protected]> Date: Tue, 17 Feb 2026 21:05:55 +0300 Subject: [PATCH 1/3] [clang-tidy] Fix readability-else-after-return for if statements appear in unbraced switch case labels --- .../readability/ElseAfterReturnCheck.cpp | 16 ++++++++++------ clang-tools-extra/docs/ReleaseNotes.rst | 4 ++++ .../checkers/readability/else-after-return.cpp | 16 ++++++++++++++++ 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp index fccda912947eb..cb3818d93cc81 100644 --- a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp @@ -169,14 +169,18 @@ void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) { const auto InterruptsControlFlow = stmt(anyOf( returnStmt().bind(InterruptingStr), continueStmt().bind(InterruptingStr), breakStmt().bind(InterruptingStr), cxxThrowExpr().bind(InterruptingStr))); + + const auto IfWithInterruptingThenElse = + ifStmt(unless(isConstexpr()), unless(isConsteval()), + hasThen(stmt(anyOf(InterruptsControlFlow, + compoundStmt(has(InterruptsControlFlow))))), + hasElse(stmt().bind("else"))) + .bind("if"); + Finder->addMatcher( compoundStmt( - forEach(ifStmt(unless(isConstexpr()), unless(isConsteval()), - hasThen(stmt( - anyOf(InterruptsControlFlow, - compoundStmt(has(InterruptsControlFlow))))), - hasElse(stmt().bind("else"))) - .bind("if"))) + forEach(stmt(anyOf(IfWithInterruptingThenElse, + switchCase(has(IfWithInterruptingThenElse)))))) .bind("cs"), this); } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 2bb4800df47c9..6de47ce1a23f1 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -231,6 +231,10 @@ Changes in existing checks <clang-tidy/checks/readability/container-size-empty>` check by fixing a crash when a member expression has a non-identifier name. +- Improved :doc:`readability-else-after-return + <clang-tidy/checks/readability/else-after-return>` check by fixing missed + diagnostics when ``if`` statements appear in unbraced ``switch`` case labels + - Improved :doc:`readability-enum-initial-value <clang-tidy/checks/readability/enum-initial-value>` check: the warning message now uses separate note diagnostics for each uninitialized enumerator, making diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp index 220c7ba19fed0..63ece00dfde5f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp @@ -312,3 +312,19 @@ void testPPConditionals() { } #endif } + +void testSwitchCaseWithoutInnerCompound(int i, bool b) { + switch (i) { + case 0: + if (b) { + return; + } else { + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'return' + // CHECK-FIXES: {{^}} } + f(0); + } + break; + default: + break; + } +} >From c8d3ab5470f129e76c27fdb8ad386f6a0c2d72cd Mon Sep 17 00:00:00 2001 From: Berkay <[email protected]> Date: Sun, 22 Feb 2026 15:54:07 +0300 Subject: [PATCH 2/3] [clang-tidy] Add support for switch cases fallthrough and goto label statements to readability-else-after-return --- .../readability/ElseAfterReturnCheck.cpp | 17 ++-- clang-tools-extra/docs/ReleaseNotes.rst | 2 +- .../readability/else-after-return.cpp | 81 ++++++++++++++++++- 3 files changed, 90 insertions(+), 10 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp index cb3818d93cc81..f894350368f1b 100644 --- a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp @@ -177,12 +177,17 @@ void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) { hasElse(stmt().bind("else"))) .bind("if"); - Finder->addMatcher( - compoundStmt( - forEach(stmt(anyOf(IfWithInterruptingThenElse, - switchCase(has(IfWithInterruptingThenElse)))))) - .bind("cs"), - this); + const auto SwitchCaseWithTargetIf = switchCase(anyOf( + switchCase(has(IfWithInterruptingThenElse)), + switchCase(hasDescendant(switchCase(has(IfWithInterruptingThenElse)))))); + + const auto LabelWithTargetIf = labelStmt(has(IfWithInterruptingThenElse)); + + Finder->addMatcher(compoundStmt(forEach(stmt(anyOf(IfWithInterruptingThenElse, + SwitchCaseWithTargetIf, + LabelWithTargetIf)))) + .bind("cs"), + this); } static bool hasPreprocessorBranchEndBetweenLocations( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6de47ce1a23f1..7f649b166db9e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -233,7 +233,7 @@ Changes in existing checks - Improved :doc:`readability-else-after-return <clang-tidy/checks/readability/else-after-return>` check by fixing missed - diagnostics when ``if`` statements appear in unbraced ``switch`` case labels + diagnostics when ``if`` statements appear in unbraced ``switch`` case labels. - Improved :doc:`readability-enum-initial-value <clang-tidy/checks/readability/enum-initial-value>` check: the warning message diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp index 63ece00dfde5f..6b9dec3b77ad2 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp @@ -313,18 +313,93 @@ void testPPConditionals() { #endif } -void testSwitchCaseWithoutInnerCompound(int i, bool b) { +void testSwitchCases(int i, bool b, bool b2) { + // Case statement without braces. switch (i) { case 0: if (b) { return; - } else { + } else { // comment-18 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'return' - // CHECK-FIXES: {{^}} } + // CHECK-FIXES: {{^}} } // comment-18 + f(1); + } + } + + // Fallthrough. + switch (i) { + case 0: + case 1: + case 2: + if (b) + return; + else // comment-19 + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return' + // CHECK-FIXES: {{^}} // comment-19 + return; + } + + switch (i) { + case 1: + case 2: + if (b) f(0); + else if (b2) + return; + else // comment-20 + // CHECK-FIXES-NOT: {{^}} // comment-20 + f(1); + } + + switch (i) { + case 0: + if (b) { + if (b2) + return; + else // comment-21 + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'return' + // CHECK-FIXES: {{^}} // comment-21 + f(0); + } else { + if (b && b2) + return; + else // comment-22 + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'return' + // CHECK-FIXES: {{^}} // comment-22 + f(0); + } + } + + // Nested switch. + switch (i) { + case 0: + case 1: + switch (3) { + case 0: + if (b) { + return; + } else { // comment-23 + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: do not use 'else' after 'return' + // CHECK-FIXES: {{^}} } // comment-23 + f(0); + } + break; + default: + break; } break; default: break; } } + +void testLabels(bool b) { + goto LABEL; +LABEL: + if (b) + return; + else // comment-25 + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return' + // CHECK-FIXES: {{^}} // comment-25 + return; +} >From bf591705f820d913b756eabfd92812eb07a432a7 Mon Sep 17 00:00:00 2001 From: Berkay <[email protected]> Date: Sun, 1 Mar 2026 19:45:46 +0300 Subject: [PATCH 3/3] [clang-tidy] Introduce custom matcher for label like statements --- .../readability/ElseAfterReturnCheck.cpp | 17 ++++--- .../readability/else-after-return.cpp | 45 ++++++++++++++++++- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp index f894350368f1b..7e93d619e2a9f 100644 --- a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp @@ -39,6 +39,12 @@ class PPConditionalCollector : public PPCallbacks { const SourceManager &SM; }; +AST_MATCHER_P(Stmt, stripLabelLikeStatements, + ast_matchers::internal::Matcher<Stmt>, InnerMatcher) { + const Stmt *S = Node.stripLabelLikeStatements(); + return InnerMatcher.matches(*S, Finder, Builder); +} + } // namespace static constexpr char InterruptingStr[] = "interrupting"; @@ -177,15 +183,8 @@ void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) { hasElse(stmt().bind("else"))) .bind("if"); - const auto SwitchCaseWithTargetIf = switchCase(anyOf( - switchCase(has(IfWithInterruptingThenElse)), - switchCase(hasDescendant(switchCase(has(IfWithInterruptingThenElse)))))); - - const auto LabelWithTargetIf = labelStmt(has(IfWithInterruptingThenElse)); - - Finder->addMatcher(compoundStmt(forEach(stmt(anyOf(IfWithInterruptingThenElse, - SwitchCaseWithTargetIf, - LabelWithTargetIf)))) + Finder->addMatcher(compoundStmt(forEach(stripLabelLikeStatements( + IfWithInterruptingThenElse))) .bind("cs"), this); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp index 6b9dec3b77ad2..06df0e60ef620 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp @@ -391,15 +391,56 @@ void testSwitchCases(int i, bool b, bool b2) { default: break; } + + switch (i) { + case 1: + if (b) + return; + else // comment-24 + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return' + // CHECK-FIXES-NOT: {{^}} // comment-24 + int _ = 20; + break; + case 2: + break; + } + + switch (i) { + case 1: + case 2: + [[clang::annotate("TestWithAttributedStmt")]] + case 3: + if (b) { + return; + } else { // comment-25 + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'return' + // CHECK-FIXES: {{^}} } // comment-25 + f(0); + } + } } void testLabels(bool b) { goto LABEL; + goto LABEL2; + LABEL: if (b) return; - else // comment-25 + else // comment-26 // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return' - // CHECK-FIXES: {{^}} // comment-25 + // CHECK-FIXES: {{^}} // comment-26 return; + + switch ((int)b) { + case 1: + case 2: + LABEL2: + if (0) + return; + else // comment-27 + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return' + // CHECK-FIXES: {{^}} // comment-27 + f(0); + } } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
