From 199e5232b3f57d007bf13d803bd2bf1260e71072 Mon Sep 17 00:00:00 2001 From: Angel Garcia Gomez Date: Mon, 5 Oct 2015 11:15:39 +0000 Subject: 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 llvm-svn: 249300 --- .../clang-tidy/modernize/LoopConvertCheck.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) (limited to 'clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp') 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(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()) { + // 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) { -- cgit v1.2.3