diff options
| -rw-r--r-- | clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp | 17 | ||||
| -rw-r--r-- | clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp | 12 |
2 files changed, 26 insertions, 3 deletions
diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp index 33a2d6da96a..cd2a6679ff4 100644 --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp @@ -477,6 +477,7 @@ void LoopConvertCheck::doConversion( std::string VarName; bool VarNameFromAlias = (Usages.size() == 1) && AliasDecl; bool AliasVarIsRef = false; + bool CanCopy = true; if (VarNameFromAlias) { const auto *AliasVar = cast<VarDecl>(AliasDecl->getSingleDecl()); @@ -525,6 +526,16 @@ void LoopConvertCheck::doConversion( // and parenthesis around a simple DeclRefExpr can always be // removed. Range = Paren->getSourceRange(); + } else if (const auto *UOP = Parents[0].get<UnaryOperator>()) { + // 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 + // local index instead. + // FIXME: This won't catch cases where the address is taken outside + // of the loop's body (for instance, in a function that got the + // loop's index as a const reference parameter), or where we take + // the address of a member (like "&Arr[i].A.B.C"). + if (UOP->getOpcode() == UO_AddrOf) + CanCopy = false; } } } else { @@ -548,8 +559,10 @@ void LoopConvertCheck::doConversion( // If the new variable name is from the aliased variable, then the reference // type for the new variable should only be used if the aliased variable was // declared as a reference. - bool UseCopy = (VarNameFromAlias && !AliasVarIsRef) || - (Descriptor.DerefByConstRef && Descriptor.IsTriviallyCopyable); + bool UseCopy = + CanCopy && + ((VarNameFromAlias && !AliasVarIsRef) || + (Descriptor.DerefByConstRef && Descriptor.IsTriviallyCopyable)); if (!UseCopy) { if (Descriptor.DerefByConstRef) { diff --git a/clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp b/clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp index 42a5d5cdc37..145ea7f0be0 100644 --- a/clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp +++ b/clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp @@ -97,7 +97,7 @@ void f() { // CHECK-FIXES-NEXT: Tea.g(); } -void constArray() { +const int *constArray() { for (int I = 0; I < N; ++I) { printf("2 * %d = %d\n", ConstArr[I], ConstArr[I] + ConstArr[I]); } @@ -112,6 +112,16 @@ void constArray() { // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (const auto & Elem : NonCopy) // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", Elem.X, Elem.X + Elem.X); + + bool Something = false; + for (int I = 0; I < N; ++I) { + if (Something) + return &ConstArr[I]; + } + // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (const auto & Elem : ConstArr) + // CHECK-FIXES-NEXT: if (Something) + // CHECK-FIXES-NEXT: return &Elem; } struct HasArr { |
