Author: revane Date: Mon Apr 1 13:15:06 2013 New Revision: 178485 URL: http://llvm.org/viewvc/llvm-project?rev=178485&view=rev Log: Improve loop convert's variable aliasing
Loop convert's variable name aliasing may cause issues if the variable is declared as a value (copy). The converted loop will declare the variable as a reference which may inadvertently cause modifications to the container if it were used and modified as a temporary copy. This is fixed by preserving the reference or value qualifiers of the aliased variable. That is, if the variable was declared as a value the loop variable will also be declared as a value and similarly for references. Fixes: PR15600 Author: Jack Yang <[email protected]> Modified: clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/naming-alias.cpp Modified: clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp?rev=178485&r1=178484&r2=178485&view=diff ============================================================================== --- clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp (original) +++ clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopActions.cpp Mon Apr 1 13:15:06 2013 @@ -737,10 +737,14 @@ void LoopFixer::doConversion(ASTContext bool ContainerNeedsDereference, bool DerefByValue) { std::string VarName; + bool VarNameFromAlias = Usages.size() == 1 && AliasDecl; + bool AliasVarIsRef = false; - if (Usages.size() == 1 && AliasDecl) { + if (VarNameFromAlias) { const VarDecl *AliasVar = cast<VarDecl>(AliasDecl->getSingleDecl()); VarName = AliasVar->getName().str(); + AliasVarIsRef = AliasVar->getType()->isReferenceType(); + // We keep along the entire DeclStmt to keep the correct range here. const SourceRange &ReplaceRange = AliasDecl->getSourceRange(); Replace->insert( @@ -770,13 +774,18 @@ void LoopFixer::doConversion(ASTContext QualType AutoRefType = Context->getAutoDeductType(); - // If an iterator's operator*() returns a 'T&' we can bind that to 'auto&'. - // If operator*() returns 'T' we can bind that to 'auto&&' which will deduce - // to 'T&&'. - if (DerefByValue) - AutoRefType = Context->getRValueReferenceType(AutoRefType); - else - AutoRefType = Context->getLValueReferenceType(AutoRefType); + // 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. + if (!VarNameFromAlias || AliasVarIsRef) { + // If an iterator's operator*() returns a 'T&' we can bind that to 'auto&'. + // If operator*() returns 'T' we can bind that to 'auto&&' which will deduce + // to 'T&&'. + if (DerefByValue) + AutoRefType = Context->getRValueReferenceType(AutoRefType); + else + AutoRefType = Context->getLValueReferenceType(AutoRefType); + } std::string MaybeDereference = ContainerNeedsDereference ? "*" : ""; std::string TypeString = AutoRefType.getAsString(); Modified: clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/naming-alias.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/naming-alias.cpp?rev=178485&r1=178484&r2=178485&view=diff ============================================================================== --- clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/naming-alias.cpp (original) +++ clang-tools-extra/trunk/test/cpp11-migrate/LoopConvert/naming-alias.cpp Mon Apr 1 13:15:06 2013 @@ -57,3 +57,40 @@ void aliasing() { // CHECK-NEXT: Val &t = func(elem); // CHECK-NEXT: int y = t.x; } + +void refs_and_vals() { + // The following tests check that the transform correctly preserves the + // reference or value qualifiers of the aliased variable. That is, if the + // variable was declared as a value, the loop variable will be declared as a + // value and vice versa for references. + + S s; + const S s_const = s; + + for (S::const_iterator it = s_const.begin(); it != s_const.end(); ++it) { + MutableVal alias = *it; { } + alias.x = 0; + } + // CHECK: for (auto alias : s_const) + // CHECK-NOT: MutableVal {{[a-z_]+}} = + // CHECK-NEXT: { } + // CHECK-NEXT: alias.x = 0; + + for (S::iterator it = s.begin(), e = s.end(); it != e; ++it) { + MutableVal alias = *it; { } + alias.x = 0; + } + // CHECK: for (auto alias : s) + // CHECK-NOT: MutableVal {{[a-z_]+}} = + // CHECK-NEXT: { } + // CHECK-NEXT: alias.x = 0; + + for (S::iterator it = s.begin(), e = s.end(); it != e; ++it) { + MutableVal &alias = *it; { } + alias.x = 0; + } + // CHECK: for (auto & alias : s) + // CHECK-NOT: MutableVal &{{[a-z_]+}} = + // CHECK-NEXT: { } + // CHECK-NEXT: alias.x = 0; +} _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
