diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp index f0791da143ad9..3229e302eb432 100644 --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp @@ -706,13 +706,17 @@ void LoopConvertCheck::doConversion( ReplaceText = Usage.Kind == Usage::UK_MemberThroughArrow ? VarNameOrStructuredBinding + "." : VarNameOrStructuredBinding; - auto Parents = Context->getParents(*Usage.Expression); + const DynTypedNodeList Parents = Context->getParents(*Usage.Expression); if (Parents.size() == 1) { if (const auto *Paren = Parents[0].get()) { // Usage.Expression will be replaced with the new index variable, // and parenthesis around a simple DeclRefExpr can always be - // removed. - Range = Paren->getSourceRange(); + // removed except in case of a `sizeof` operator call. + const DynTypedNodeList GrandParents = Context->getParents(*Paren); + if (GrandParents.size() != 1 || + GrandParents[0].get() == nullptr) { + Range = Paren->getSourceRange(); + } } else if (const auto *UOP = Parents[0].get()) { // If we are taking the address of the loop variable, then we must // not use a copy, as it would mean taking the address of the loop's diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 7ca7037e2a6a4..58629426216ba 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -170,6 +170,11 @@ Changes in existing checks `AllowStringArrays` option, enabling the exclusion of array types with deduced length initialized from string literals. +- Improved :doc:`modernize-loop-convert + ` check by ensuring that fix-its + don't remove parentheses used in ``sizeof`` calls when they have array index + accesses as arguments. + - Improved :doc:`modernize-use-override ` check to also remove any trailing whitespace when deleting the ``virtual`` keyword. 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 c29fbc9f9b23b..695925a5d877c 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 @@ -88,6 +88,19 @@ void f() { // CHECK-FIXES-NEXT: printf("Fibonacci number %d has address %p\n", I, &I); // CHECK-FIXES-NEXT: Sum += I + 2; + int Matrix[N][12]; + unsigned size = 0; + for (int I = 0; I < N; ++I) { + size += sizeof(Matrix[I]); + size += sizeof Matrix[I]; + size += sizeof((Matrix[I])); + } + // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (auto & I : Matrix) + // CHECK-FIXES-NEXT: size += sizeof(I); + // CHECK-FIXES-NEXT: size += sizeof I; + // CHECK-FIXES-NEXT: size += sizeof(I); + Val Teas[N]; for (int I = 0; I < N; ++I) { Teas[I].g();