https://github.com/localspook created https://github.com/llvm/llvm-project/pull/187437
Fixes #57258. >From 341387f8d36a7bfea4eee0c359484a804d194752 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> Date: Thu, 19 Mar 2026 04:49:33 +0000 Subject: [PATCH] [clang-tidy] Fix `readability-else-after-return` breaking code by deleting too many characters --- .../readability/ElseAfterReturnCheck.cpp | 28 +++---------------- clang-tools-extra/docs/ReleaseNotes.rst | 3 ++ .../readability/else-after-return.cpp | 20 +++++++++++++ 3 files changed, 27 insertions(+), 24 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp index 8e1162ff8b073..568096d0d048f 100644 --- a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp @@ -136,33 +136,13 @@ static void removeElseAndBrackets(DiagnosticBuilder &Diag, ASTContext &Context, auto Remap = [&](SourceLocation Loc) { return Context.getSourceManager().getExpansionLoc(Loc); }; - auto TokLen = [&](SourceLocation Loc) { - return Lexer::MeasureTokenLength(Loc, Context.getSourceManager(), - Context.getLangOpts()); - }; if (const auto *CS = dyn_cast<CompoundStmt>(Else)) { - Diag << tooling::fixit::createRemoval(ElseLoc); - const SourceLocation LBrace = CS->getLBracLoc(); - const SourceLocation RBrace = CS->getRBracLoc(); - const SourceLocation RangeStart = - Remap(LBrace).getLocWithOffset(TokLen(LBrace) + 1); - const SourceLocation RangeEnd = Remap(RBrace).getLocWithOffset(-1); - - const StringRef Repl = Lexer::getSourceText( - CharSourceRange::getTokenRange(RangeStart, RangeEnd), - Context.getSourceManager(), Context.getLangOpts()); - Diag << tooling::fixit::createReplacement(CS->getSourceRange(), Repl); + Diag << tooling::fixit::createRemoval(ElseLoc) + << tooling::fixit::createRemoval(Remap(CS->getLBracLoc())) + << tooling::fixit::createRemoval(Remap(CS->getRBracLoc())); } else { - const SourceLocation ElseExpandedLoc = Remap(ElseLoc); - const SourceLocation EndLoc = Remap(Else->getEndLoc()); - - const StringRef Repl = Lexer::getSourceText( - CharSourceRange::getTokenRange( - ElseExpandedLoc.getLocWithOffset(TokLen(ElseLoc) + 1), EndLoc), - Context.getSourceManager(), Context.getLangOpts()); - Diag << tooling::fixit::createReplacement( - SourceRange(ElseExpandedLoc, EndLoc), Repl); + Diag << tooling::fixit::createRemoval(Remap(ElseLoc)); } } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index c9a170a9e8660..0f9f2f0afa8ac 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -352,6 +352,9 @@ Changes in existing checks - Fixed a false positive involving ``if`` statements which contain a ``return``, ``break``, etc., jumped over by a ``goto``. + - Fixed the check potentially breaking code by deleting one too many + characters following an ``else`` or a curly brace. + - Added support for handling attributed ``if`` then-branches such as ``[[likely]]`` and ``[[unlikely]]``. 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 1bbbdbc2a5683..85117b718de83 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 @@ -502,3 +502,23 @@ void testNoReturn() { f(0); } } + +void testFixitsPreserveComments() { + if (true) { + return; + } else {// aaaa + // bbbb + }// cccc + // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: do not use 'else' after 'return' + // CHECK-FIXES: } // aaaa + // CHECK-FIXES-NEXT: // bbbb + // CHECK-FIXES-NEXT: // cccc + + if (true) + return; + else// dddd + ;// eeee + // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: do not use 'else' after 'return' + // CHECK-FIXES: // dddd + // CHECK-FIXES-NEXT: ;// eeee +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
