https://github.com/gxyd created https://github.com/llvm/llvm-project/pull/202015
Fixes #105508 >From 4b091596e9dfc7f2dd050c2b6acea83e9b27e2a1 Mon Sep 17 00:00:00 2001 From: Gaurav Dhingra <[email protected]> Date: Fri, 5 Jun 2026 13:56:47 +0530 Subject: [PATCH 1/2] [clang-tidy] Fix modernize-loop-convert by introducing space Introduce leading space for a dereference operator without leading space, which follows a keyword in loop conversion body Fixes #105508 --- .../clang-tidy/modernize/LoopConvertCheck.cpp | 19 +++ .../Inputs/loop-convert/structures.h | 2 +- .../checkers/modernize/loop-convert-basic.cpp | 130 ++++++++++++++++++ 3 files changed, 150 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp index 1ba209fed5a27..1962c83db5aa8 100644 --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp @@ -703,6 +703,25 @@ void LoopConvertCheck::doConversion( ReplaceText = Usage.Kind == Usage::UK_MemberThroughArrow ? VarNameOrStructuredBinding + "." : VarNameOrStructuredBinding; + // In cases like `delete*it`, we can't just change it to `deleteit`, + // we need to introduce space after `delete` to make it `delete it`. + Token StarToken; + if (Usage.Kind == Usage::UK_Default && + !Lexer::getRawToken(Usage.Range.getBegin(), StarToken, + Context->getSourceManager(), getLangOpts(), + false) && + StarToken.is(tok::star)) { + std::optional<Token> PrevToken = Lexer::findPreviousToken( + Usage.Range.getBegin(), Context->getSourceManager(), + getLangOpts(), true); + if (PrevToken) { + // Check whether StarToken has leading space or not + const bool StarTokenHasNoLeadingSpace = + PrevToken->getEndLoc() == StarToken.getLocation(); + if (PrevToken->isAnyIdentifier() && StarTokenHasNoLeadingSpace) + ReplaceText = " " + ReplaceText; + } + } const DynTypedNodeList Parents = Context->getParents(*Usage.Expression); if (Parents.size() == 1) { if (const auto *Paren = Parents[0].get<ParenExpr>()) { diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h index 839511da30631..64ea452c0062f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h @@ -13,7 +13,7 @@ extern "C" { extern int printf(const char *restrict, ...); } -struct Val {int X; void g(); }; +struct Val {int X; void g(); int** f(); }; struct MutableVal { void constFun(int) const; diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp index 2f744eb8f1c9b..3f9c4d8200ad1 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp @@ -1009,3 +1009,133 @@ void test() { } } } // namespace GH109083 + +namespace GH105508 { +void test() { + Nested<int*> v1; + for (auto it=v1.begin(); it != v1.end(); ++it) { + delete* it; + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead [modernize-loop-convert] + // CHECK-FIXES: for (auto & it : v1) { + // CHECK-FIXES-NEXT: delete it; + + Nested<int*> v2; + for (auto it=v2.begin(); it != v2.end(); ++it) { + delete* it; + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead [modernize-loop-convert] + // CHECK-FIXES: for (auto & it : v2) { + // CHECK-FIXES-NEXT: delete it; + + Nested<int*> v3; + for (auto it=v3.begin(); it != v3.end(); ++it) { + delete/* random comment */*it; + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead [modernize-loop-convert] + // CHECK-FIXES: for (auto & it : v3) { + // CHECK-FIXES-NEXT: delete/* random comment */it; + + Nested<int*> v4; + for (auto it=v4.begin(); it != v4.end(); ++it) { + sizeof* it; + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead [modernize-loop-convert] + // CHECK-FIXES: for (auto & it : v4) { + // CHECK-FIXES-NEXT: sizeof it; + + Nested<int*> v5; + for (auto it=v5.begin(); it != v5.end(); ++it) { + sizeof* it; + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead [modernize-loop-convert] + // CHECK-FIXES: for (auto & it : v5) { + // CHECK-FIXES-NEXT: sizeof it; + + Nested<int*> v6; + for (auto it=v6.begin(); it != v6.end(); ++it) { + sizeof/* random comment */*it; + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead [modernize-loop-convert] + // CHECK-FIXES: for (auto & it : v6) { + // CHECK-FIXES-NEXT: sizeof/* random comment */it; + + Nested<Val> v7; + for (auto it=v7.begin(); it != v7.end(); ++it) { + delete *it->f(); + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead [modernize-loop-convert] + // CHECK-FIXES: for (auto & it : v7) { + // CHECK-FIXES-NEXT: delete *it.f(); + + Nested<Val> v8; + for (auto it=v8.begin(); it != v8.end(); ++it) { + delete*it->f(); + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead [modernize-loop-convert] + // CHECK-FIXES: for (auto & it : v8) { + // CHECK-FIXES-NEXT: delete*it.f(); + + Nested<Val> v9; + for (auto it=v9.begin(); it != v9.end(); ++it) { + sizeof *it->f(); + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead [modernize-loop-convert] + // CHECK-FIXES: for (auto & it : v9) { + // CHECK-FIXES-NEXT: sizeof *it.f(); + + Nested<Val> v10; + for (auto it=v10.begin(); it != v10.end(); ++it) { + sizeof*it->f(); + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead [modernize-loop-convert] + // CHECK-FIXES: for (auto & it : v10) { + // CHECK-FIXES-NEXT: sizeof*it.f(); + + Nested<int*> v11; + for (auto it = v11.begin(); it != v11.end(); ++it) { + auto H1 = [&it]() { delete*it; }; + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead [modernize-loop-convert] + // CHECK-FIXES: for (auto & it : v11) { + // CHECK-FIXES-NEXT: auto H1 = [&it]() { delete it; }; + + Nested<int*> v12; + for (auto it = v12.begin(); it != v12.end(); ++it) { + auto H1 = [&it]() { return*it; }; + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead [modernize-loop-convert] + // CHECK-FIXES: for (auto & it : v12) { + // CHECK-FIXES-NEXT: auto H1 = [&it]() { return it; }; +} + +int* test2() { + Nested<int*> v; + for (auto it=v.begin(); it != v.end(); ++it) { + return*it; + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead [modernize-loop-convert] + // CHECK-FIXES: for (auto & it : v) { + // CHECK-FIXES-NEXT: return it; +} + +int* test3() { + Nested<int*> v; + for (auto it=v.begin(); it != v.end(); ++it) { + return* it; + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead [modernize-loop-convert] + // CHECK-FIXES: for (auto & it : v) { + // CHECK-FIXES-NEXT: return it; +} + +int* test4() { + Nested<int*> v; + for (auto it=v.begin(); it != v.end(); ++it) { + return *it; + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead [modernize-loop-convert] + // CHECK-FIXES: for (auto & it : v) { + // CHECK-FIXES-NEXT: return it; +} +} // namespace GH105508 >From eed48446eb4a14bace6e206edcff4e9137857ad6 Mon Sep 17 00:00:00 2001 From: Gaurav Dhingra <[email protected]> Date: Sat, 6 Jun 2026 12:30:44 +0530 Subject: [PATCH 2/2] add release notes entry --- clang-tools-extra/docs/ReleaseNotes.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index d2db3dc2b3e78..2aec70baf2d71 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -416,6 +416,12 @@ Changes in existing checks positives on project headers that use the same name as a standard library header. +- Improve :doc:`modernize-loop-convert + <clang-tidy/checks/modernize/loop-convert>` checks to insert a space when + replacing ``*it`` with the loop variable in expressions like ``delete*it`` + , where the missing space would cause the keyword and the new variable to + merge into a single identifier. + - Improved :doc:`modernize-pass-by-value <clang-tidy/checks/modernize/pass-by-value>` check by adding `IgnoreMacros` option to suppress warnings in macros. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
