replica
/
hanchenye-llvm-project
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix loop-convert for trivially copyable types. 

Previously, we would rewrite:
void f(const vector<int> &v) {
  for (size_t i = 0; i < v.size(); ++i) {
to
  for (const auto &elem : v) {

Now we rewrite it to:
  for (auto elem : v) {
(and similarly for iterator based loops).

llvm-svn: 248438
This commit is contained in:
Manuel Klimek 2015-09-23 22:28:14 +00:00
parent beb5bccf88
commit b457b68f8d
3 changed files with 56 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

63
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
View File

@ -500,7 +500,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.
if (!VarNameFromAlias || AliasVarIsRef) {
bool UseCopy = (VarNameFromAlias && !AliasVarIsRef) ||
(Descriptor.DerefByConstRef && Descriptor.IsTriviallyCopyable);
if (!UseCopy) {
if (Descriptor.DerefByConstRef) {
AutoType =
Context->getLValueReferenceType(Context->getConstType(AutoType));
@ -547,35 +550,29 @@ void LoopConvertCheck::getArrayLoopQualifiers(ASTContext *Context,
RangeDescriptor &Descriptor) {
// On arrays and pseudoarrays, we must figure out the qualifiers from the
// usages.
if (usagesAreConst(Usages)) {
if (usagesAreConst(Usages) ||
containerIsConst(ContainerExpr, Descriptor.ContainerNeedsDereference)) {
Descriptor.DerefByConstRef = true;
} else {
if (usagesReturnRValues(Usages)) {
// If the index usages (dereference, subscript, at, ...) return rvalues,
// then we should not use a reference, because we need to keep the code
// correct if it mutates the returned objects.
Descriptor.DerefByValue = true;
// Try to find the type of the elements on the container, to check if
// they are trivially copyable.
for (const Usage &U : Usages) {
if (!U.Expression || U.Expression->getType().isNull())
continue;
QualType Type = U.Expression->getType().getCanonicalType();
if (U.Kind == Usage::UK_MemberThroughArrow) {
if (!Type->isPointerType())
continue;
Type = Type->getPointeeType();
}
Descriptor.IsTriviallyCopyable = Type.isTriviallyCopyableType(*Context);
break;
}
if (usagesReturnRValues(Usages)) {
// If the index usages (dereference, subscript, at, ...) return rvalues,
// then we should not use a reference, because we need to keep the code
// correct if it mutates the returned objects.
Descriptor.DerefByValue = true;
}
// Try to find the type of the elements on the container, to check if
// they are trivially copyable.
for (const Usage &U : Usages) {
if (!U.Expression || U.Expression->getType().isNull())
continue;
QualType Type = U.Expression->getType().getCanonicalType();
if (U.Kind == Usage::UK_MemberThroughArrow) {
if (!Type->isPointerType()) {
continue;
}
} else if (containerIsConst(ContainerExpr,
Descriptor.ContainerNeedsDereference)) {
// The usages are lvalue references, we add a 'const' if the container
// is 'const'. This will always be the case with const arrays (unless
// usagesAreConst() returned true).
Descriptor.DerefByConstRef = true;
Type = Type->getPointeeType();
}
Descriptor.IsTriviallyCopyable = Type.isTriviallyCopyableType(*Context);
}
}
@ -604,10 +601,11 @@ void LoopConvertCheck::getIteratorLoopQualifiers(ASTContext *Context,
// A node will only be bound with DerefByRefResultName if we're dealing
// with a user-defined iterator type. Test the const qualification of
// the reference type.
Descriptor.DerefByConstRef = (*DerefType)
->getAs<ReferenceType>()
->getPointeeType()
.isConstQualified();
auto ValueType = (*DerefType)->getAs<ReferenceType>()->getPointeeType();
Descriptor.DerefByConstRef = ValueType.isConstQualified();
Descriptor.IsTriviallyCopyable =
ValueType.isTriviallyCopyableType(*Context);
} else {
// By nature of the matcher this case is triggered only for built-in
// iterator types (i.e. pointers).
@ -617,6 +615,9 @@ void LoopConvertCheck::getIteratorLoopQualifiers(ASTContext *Context,
// We test for const qualification of the pointed-at type.
Descriptor.DerefByConstRef =
CanonicalInitVarType->getPointeeType().isConstQualified();
Descriptor.IsTriviallyCopyable =
CanonicalInitVarType->getPointeeType().isTriviallyCopyableType(
*Context);
}
}
}

29
clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp
View File

@ -102,7 +102,7 @@ void constArray() {
printf("2 * %d = %d\n", constArr[i], constArr[i] + constArr[i]);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : constArr)
// CHECK-FIXES: for (auto elem : constArr)
// CHECK-FIXES-NEXT: printf("2 * %d = %d\n", elem, elem + elem);
}
@ -333,7 +333,7 @@ void different_type() {
printf("s has value %d\n", (*it).x);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : s)
// CHECK-FIXES: for (auto elem : s)
// CHECK-FIXES-NEXT: printf("s has value %d\n", (elem).x);
S *ps;
@ -341,7 +341,7 @@ void different_type() {
printf("s has value %d\n", (*it).x);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & p : *ps)
// CHECK-FIXES: for (auto p : *ps)
// CHECK-FIXES-NEXT: printf("s has value %d\n", (p).x);
// v.begin() returns a user-defined type 'iterator' which, since it's
@ -406,12 +406,12 @@ public:
for (const_iterator I = begin(), E = end(); I != E; ++I)
(void) *I;
// CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : *this)
// CHECK-FIXES: for (auto elem : *this)
for (const_iterator I = C::begin(), E = C::end(); I != E; ++I)
(void) *I;
// CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : *this)
// CHECK-FIXES: for (auto elem : *this)
for (const_iterator I = begin(), E = end(); I != E; ++I) {
(void) *I;
@ -508,7 +508,7 @@ void constness() {
sum += constv[i] + 2;
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : constv)
// CHECK-FIXES: for (auto elem : constv)
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
// CHECK-FIXES-NEXT: sum += elem + 2;
@ -517,7 +517,7 @@ void constness() {
sum += constv.at(i) + 2;
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : constv)
// CHECK-FIXES: for (auto elem : constv)
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
// CHECK-FIXES-NEXT: sum += elem + 2;
@ -526,7 +526,7 @@ void constness() {
sum += pconstv->at(i) + 2;
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : *pconstv)
// CHECK-FIXES: for (auto elem : *pconstv)
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
// CHECK-FIXES-NEXT: sum += elem + 2;
@ -539,7 +539,7 @@ void constness() {
sum += (*pconstv)[i] + 2;
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : *pconstv)
// CHECK-FIXES: for (auto elem : *pconstv)
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
// CHECK-FIXES-NEXT: sum += elem + 2;
}
@ -552,7 +552,14 @@ void ConstRef(const dependent<int>& ConstVRef) {
sum += ConstVRef[i];
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : ConstVRef)
// CHECK-FIXES: for (auto elem : ConstVRef)
// CHECK-FIXES-NEXT: sum += elem;
for (auto I = ConstVRef.begin(), E = ConstVRef.end(); I != E; ++I) {
sum += *I;
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto elem : ConstVRef)
// CHECK-FIXES-NEXT: sum += elem;
}
@ -611,7 +618,7 @@ void NoBeginEndTest() {
for (unsigned i = 0, e = const_CBE.size(); i < e; ++i)
printf("%d\n", const_CBE[i]);
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : const_CBE)
// CHECK-FIXES: for (auto elem : const_CBE)
// CHECK-FIXES-NEXT: printf("%d\n", elem);
}

12
clang-tools-extra/test/clang-tidy/modernize-loop-convert-extra.cpp
View File

@ -275,7 +275,7 @@ void macroConflict() {
printf("Max of 3 and 5: %d\n", MAX(3, 5));
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & MAXs_it : MAXs)
// CHECK-FIXES: for (auto MAXs_it : MAXs)
// CHECK-FIXES-NEXT: printf("s has value %d\n", (MAXs_it).x);
// CHECK-FIXES-NEXT: printf("Max of 3 and 5: %d\n", MAX(3, 5));
@ -468,7 +468,7 @@ void f() {
}
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : NestS)
// CHECK-FIXES: for (auto elem : NestS)
// CHECK-FIXES-NEXT: for (S::const_iterator SI = (elem).begin(), SE = (elem).end(); SI != SE; ++SI)
// CHECK-FIXES-NEXT: printf("%d", *SI);
@ -480,7 +480,7 @@ void f() {
}
}
// CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & s : NestS)
// CHECK-FIXES: for (auto s : NestS)
for (Nested<S>::iterator I = NestS.begin(), E = NestS.end(); I != E; ++I) {
S &s = *I;
@ -501,7 +501,7 @@ void f() {
}
}
// CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & s : NestS)
// CHECK-FIXES: for (auto s : NestS)
for (Nested<S>::iterator I = NestS.begin(), E = NestS.end(); I != E; ++I) {
S &s = *I;
@ -655,7 +655,7 @@ void different_type() {
printf("s has value %d\n", (*it).x);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : s)
// CHECK-FIXES: for (auto elem : s)
// CHECK-FIXES-NEXT: printf("s has value %d\n", (elem).x);
S *ps;
@ -663,7 +663,7 @@ void different_type() {
printf("s has value %d\n", (*it).x);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & p : *ps)
// CHECK-FIXES: for (auto p : *ps)
// CHECK-FIXES-NEXT: printf("s has value %d\n", (p).x);
// v.begin() returns a user-defined type 'iterator' which, since it's