llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy 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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
