Author: angelgarcia Date: Mon Oct 5 06:15:39 2015 New Revision: 249300 URL: http://llvm.org/viewvc/llvm-project?rev=249300&view=rev Log: Document a bug in loop-convert and fix one of its subcases.
Summary: Now that we prioritize copying trivial types over using const-references where possible, I found some cases where, after the transformation, the loop was using the address of the local copy instead of the original object. Reviewers: klimek Subscribers: alexfh, cfe-commits Differential Revision: http://reviews.llvm.org/D13431 Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp?rev=249300&r1=249299&r2=249300&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp Mon Oct 5 06:15:39 2015 @@ -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) { Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp?rev=249300&r1=249299&r2=249300&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp Mon Oct 5 06:15:39 2015 @@ -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 { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits