Author: Berkay Sahin Date: 2026-03-04T13:57:01-07:00 New Revision: 90c6e6374b9555279d6c31016bd7f4a8ee1861b4
URL: https://github.com/llvm/llvm-project/commit/90c6e6374b9555279d6c31016bd7f4a8ee1861b4 DIFF: https://github.com/llvm/llvm-project/commit/90c6e6374b9555279d6c31016bd7f4a8ee1861b4.diff LOG: [clang-tidy] Fix readability-else-after-return for if statements appear in unbraced switch case labels (#181878) Fixes #160033. Added: Modified: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp index fccda912947eb..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"; @@ -169,16 +175,18 @@ void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) { const auto InterruptsControlFlow = stmt(anyOf( returnStmt().bind(InterruptingStr), continueStmt().bind(InterruptingStr), breakStmt().bind(InterruptingStr), cxxThrowExpr().bind(InterruptingStr))); - Finder->addMatcher( - compoundStmt( - forEach(ifStmt(unless(isConstexpr()), unless(isConsteval()), - hasThen(stmt( - anyOf(InterruptsControlFlow, - compoundStmt(has(InterruptsControlFlow))))), - hasElse(stmt().bind("else"))) - .bind("if"))) - .bind("cs"), - this); + + 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(stripLabelLikeStatements( + IfWithInterruptingThenElse))) + .bind("cs"), + this); } static bool hasPreprocessorBranchEndBetweenLocations( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 52e14d63d7b37..309b2df5c2eb4 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -280,6 +280,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 2847248b2259b..0bad90b5e537a 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 @@ -306,3 +306,135 @@ void testPPConditionals() { } #endif } + +void testSwitchCases(int i, bool b, bool b2) { + // Case statement without braces. + switch (i) { + case 0: + if (b) { + return; + } else { // comment-18 + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'return' + // 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; + } + + 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-26 + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return' + // 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
