Index: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp @@ -477,6 +477,7 @@ std::string VarName; bool VarNameFromAlias = (Usages.size() == 1) && AliasDecl; bool AliasVarIsRef = false; + bool CanCopy = true; if (VarNameFromAlias) { const auto *AliasVar = cast(AliasDecl->getSingleDecl()); @@ -525,6 +526,16 @@ // and parenthesis around a simple DeclRefExpr can always be // removed. 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 + // 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 @@ // 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) { Index: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp +++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp @@ -97,7 +97,7 @@ // 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 @@ // 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 {