[clang-tools-extra] [clang-tidy] Fix modernize-loop-convert by introducing space (PR #202015)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Jun 6 00:03:12 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Gaurav Dhingra (gxyd)
<details>
<summary>Changes</summary>
Fixes #<!-- -->105508
---
Full diff: https://github.com/llvm/llvm-project/pull/202015.diff
4 Files Affected:
- (modified) clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp (+19)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6)
- (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h (+1-1)
- (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp (+130)
``````````diff
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/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.
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
``````````
</details>
https://github.com/llvm/llvm-project/pull/202015
More information about the cfe-commits
mailing list