[clang-tools-extra] r248438 - Fix loop-convert for trivially copyable types.
Manuel Klimek via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 23 15:28:15 PDT 2015
Author: klimek
Date: Wed Sep 23 17:28:14 2015
New Revision: 248438
URL: http://llvm.org/viewvc/llvm-project?rev=248438&view=rev
Log:
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).
Modified:
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.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=248438&r1=248437&r2=248438&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp Wed Sep 23 17:28:14 2015
@@ -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::getArrayLoopQuali
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::getIteratorLoopQu
// 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::getIteratorLoopQu
// We test for const qualification of the pointed-at type.
Descriptor.DerefByConstRef =
CanonicalInitVarType->getPointeeType().isConstQualified();
+ Descriptor.IsTriviallyCopyable =
+ CanonicalInitVarType->getPointeeType().isTriviallyCopyableType(
+ *Context);
}
}
}
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=248438&r1=248437&r2=248438&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 Wed Sep 23 17:28:14 2015
@@ -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>& Cons
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);
}
Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp?rev=248438&r1=248437&r2=248438&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp Wed Sep 23 17:28:14 2015
@@ -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
More information about the cfe-commits
mailing list