angelgarcia updated this revision to Diff 34428.
angelgarcia added a comment.
Comment the enumerators.
> Do we need default?
I think so. We need to set the cases that do not fall in any of these
categories to something, and I think that using one of the other three as the
default kind would be confusing. But maybe there is a better name than just
UK_Default. Any ideas?
http://reviews.llvm.org/D12734
Files:
clang-tidy/modernize/LoopConvertCheck.cpp
clang-tidy/modernize/LoopConvertCheck.h
clang-tidy/modernize/LoopConvertUtils.cpp
clang-tidy/modernize/LoopConvertUtils.h
test/clang-tidy/Inputs/modernize-loop-convert/structures.h
test/clang-tidy/modernize-loop-convert-basic.cpp
test/clang-tidy/modernize-loop-convert-extra.cpp
test/clang-tidy/modernize-loop-convert-negative.cpp
Index: test/clang-tidy/modernize-loop-convert-negative.cpp
===================================================================
--- test/clang-tidy/modernize-loop-convert-negative.cpp
+++ test/clang-tidy/modernize-loop-convert-negative.cpp
@@ -290,7 +290,6 @@
dependent<int> v;
dependent<int> *pv;
-transparent<dependent<int>> cv;
int Sum = 0;
// Checks for the Index start and end:
@@ -473,3 +472,41 @@
}
} // namespace NegativeMultiEndCall
+
+namespace NoUsages {
+
+const int N = 6;
+int arr[N] = {1, 2, 3, 4, 5, 6};
+S s;
+dependent<int> v;
+int Count = 0;
+
+void foo();
+
+void f() {
+ for (int I = 0; I < N; ++I) {}
+ for (int I = 0; I < N; ++I)
+ printf("Hello world\n");
+ for (int I = 0; I < N; ++I)
+ ++Count;
+ for (int I = 0; I < N; ++I)
+ foo();
+
+ for (S::iterator I = s.begin(), E = s.end(); I != E; ++I) {}
+ for (S::iterator I = s.begin(), E = s.end(); I != E; ++I)
+ printf("Hello world\n");
+ for (S::iterator I = s.begin(), E = s.end(); I != E; ++I)
+ ++Count;
+ for (S::iterator I = s.begin(), E = s.end(); I != E; ++I)
+ foo();
+
+ for (int I = 0; I < v.size(); ++I) {}
+ for (int I = 0; I < v.size(); ++I)
+ printf("Hello world\n");
+ for (int I = 0; I < v.size(); ++I)
+ ++Count;
+ for (int I = 0; I < v.size(); ++I)
+ foo();
+}
+
+} // namespace NoUsages
Index: test/clang-tidy/modernize-loop-convert-extra.cpp
===================================================================
--- test/clang-tidy/modernize-loop-convert-extra.cpp
+++ test/clang-tidy/modernize-loop-convert-extra.cpp
@@ -14,10 +14,9 @@
int b = arr[i][a];
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & elem : arr) {
+ // CHECK-FIXES: for (auto & elem : arr)
// CHECK-FIXES-NEXT: int a = 0;
// CHECK-FIXES-NEXT: int b = elem[a];
- // CHECK-FIXES-NEXT: }
for (int j = 0; j < M; ++j) {
int a = 0;
@@ -121,16 +120,16 @@
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto alias : IntArr)
- // CHECK-FIXES-NEXT: if (alias) {
+ // CHECK-FIXES-NEXT: if (alias)
for (unsigned i = 0; i < N; ++i) {
while (int alias = IntArr[i]) {
sideEffect(alias);
}
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto alias : IntArr)
- // CHECK-FIXES-NEXT: while (alias) {
+ // CHECK-FIXES-NEXT: while (alias)
for (unsigned i = 0; i < N; ++i) {
switch (int alias = IntArr[i]) {
@@ -140,32 +139,32 @@
}
// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto alias : IntArr)
- // CHECK-FIXES-NEXT: switch (alias) {
+ // CHECK-FIXES-NEXT: switch (alias)
for (unsigned i = 0; i < N; ++i) {
for (int alias = IntArr[i]; alias < N; ++alias) {
sideEffect(alias);
}
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto alias : IntArr)
- // CHECK-FIXES-NEXT: for (; alias < N; ++alias) {
+ // CHECK-FIXES-NEXT: for (; alias < N; ++alias)
for (unsigned i = 0; i < N; ++i) {
for (unsigned j = 0; int alias = IntArr[i]; ++j) {
sideEffect(alias);
}
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto alias : IntArr)
- // CHECK-FIXES-NEXT: for (unsigned j = 0; alias; ++j) {
+ // CHECK-FIXES-NEXT: for (unsigned j = 0; alias; ++j)
struct IntRef { IntRef(const int& i); };
for (int i = 0; i < N; ++i) {
IntRef Int(IntArr[i]);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & elem : IntArr) {
+ // CHECK-FIXES: for (auto & elem : IntArr)
// CHECK-FIXES-NEXT: IntRef Int(elem);
}
@@ -288,7 +287,7 @@
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & DEFs_it : DEFs)
- // CHECK-FIXES-NEXT: if (DEFs_it == DEF) {
+ // CHECK-FIXES-NEXT: if (DEFs_it == DEF)
// CHECK-FIXES-NEXT: printf("I found %d\n", DEFs_it);
}
@@ -315,8 +314,8 @@
T Vals;
// Using the name "Val", although it is the name of an existing struct, is
// safe in this loop since it will only exist within this scope.
- for (T::iterator it = Vals.begin(), e = Vals.end(); it != e; ++it) {
- }
+ for (T::iterator it = Vals.begin(), e = Vals.end(); it != e; ++it)
+ (void) *it;
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & Val : Vals)
@@ -333,17 +332,18 @@
U TDs;
// Naming the variable "TD" within this loop is safe because the typedef
// was never used within the loop.
- for (U::iterator it = TDs.begin(), e = TDs.end(); it != e; ++it) {
- }
+ for (U::iterator it = TDs.begin(), e = TDs.end(); it != e; ++it)
+ (void) *it;
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & TD : TDs)
// "TD" cannot be used in this loop since the typedef is being used.
for (U::iterator it = TDs.begin(), e = TDs.end(); it != e; ++it) {
TD V;
V.x = 5;
+ (void) *it;
}
- // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+ // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & TDs_it : TDs)
// CHECK-FIXES-NEXT: TD V;
// CHECK-FIXES-NEXT: V.x = 5;
@@ -456,8 +456,8 @@
}
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & elem : NestT) {
- // CHECK-FIXES-NEXT: for (T::iterator TI = (elem).begin(), TE = (elem).end(); TI != TE; ++TI) {
+ // CHECK-FIXES: for (auto & elem : NestT)
+ // CHECK-FIXES-NEXT: for (T::iterator TI = (elem).begin(), TE = (elem).end(); TI != TE; ++TI)
// CHECK-FIXES-NEXT: printf("%d", *TI);
// The inner loop is also convertible.
@@ -468,8 +468,8 @@
}
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (const auto & elem : NestS) {
- // CHECK-FIXES-NEXT: for (S::const_iterator SI = (elem).begin(), SE = (elem).end(); SI != SE; ++SI) {
+ // CHECK-FIXES: for (const 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);
for (Nested<S>::const_iterator I = NestS.begin(), E = NestS.end(); I != E; ++I) {
@@ -480,7 +480,7 @@
}
}
// CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (const auto & s : NestS) {
+ // CHECK-FIXES: for (const auto & s : NestS)
for (Nested<S>::iterator I = NestS.begin(), E = NestS.end(); I != E; ++I) {
S &s = *I;
@@ -490,7 +490,7 @@
}
}
// CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & s : NestS) {
+ // CHECK-FIXES: for (auto & s : NestS)
Foo foo;
for (Nested<S>::const_iterator I = NestS.begin(), E = NestS.end(); I != E; ++I) {
@@ -501,7 +501,7 @@
}
}
// CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (const auto & s : NestS) {
+ // CHECK-FIXES: for (const auto & s : NestS)
for (Nested<S>::iterator I = NestS.begin(), E = NestS.end(); I != E; ++I) {
S &s = *I;
@@ -511,7 +511,7 @@
}
}
// CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & s : NestS) {
+ // CHECK-FIXES: for (auto & s : NestS)
}
@@ -623,15 +623,15 @@
printf("Fibonacci number is %d\n", *it);
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & elem : v) {
+ // CHECK-FIXES: for (auto & elem : v)
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
for (dependent<int>::iterator it(v.begin());
it != v.end(); ++it) {
printf("Fibonacci number is %d\n", *it);
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & elem : v) {
+ // CHECK-FIXES: for (auto & elem : v)
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
doublyDependent<int, int> intmap;
@@ -684,20 +684,39 @@
namespace Macros {
+#define TWO_PARAM(x, y) if (x == y) {}
+#define THREE_PARAM(x, y, z) if (x == y) {z;}
+
const int N = 10;
int arr[N];
void messing_with_macros() {
for (int i = 0; i < N; ++i) {
printf("Value: %d\n", arr[i]);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & elem : arr) {
+ // CHECK-FIXES: for (auto & elem : arr)
// CHECK-FIXES-NEXT: printf("Value: %d\n", elem);
for (int i = 0; i < N; ++i) {
printf("Value: %d\n", CONT arr[i]);
}
+
+ // FIXME: Right now, clang-tidy does not allow to make insertions in several
+ // arguments of the same macro call. The following code:
+ // \code
+ // for (int i = 0; i < N; ++i) {
+ // TWO_PARAM(arr[i], arr[i]);
+ // THREE_PARAM(arr[i], arr[i], arr[i]);
+ // }
+ // \endcode
+ // Should be converted to this:
+ // \code
+ // for (auto & elem : arr) {
+ // TWO_PARAM(elem, elem);
+ // THREE_PARAM(elem, elem, elem);
+ // }
+ // \endcode
}
} // namespace Macros
@@ -708,12 +727,14 @@
void set_union(Container &container) {
for (typename Container::const_iterator SI = container.begin(),
SE = container.end(); SI != SE; ++SI) {
+ (void) *SI;
}
+
S s;
- for (S::iterator SI = s.begin(), SE = s.end(); SI != SE; ++SI) {
- }
+ for (S::iterator SI = s.begin(), SE = s.end(); SI != SE; ++SI)
+ (void) *SI;
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & elem : s) {
+ // CHECK-FIXES: for (auto & elem : s)
}
void template_instantiation() {
@@ -735,7 +756,7 @@
auto F1 = [Arr, I]() { int R1 = Arr[I] + 1; };
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : Arr)
- // CHECK-FIXES-NEXT: auto F1 = [Arr, elem]() { int R1 = elem + 1; };
+ // CHECK-FIXES-NEXT: auto F1 = [Arr, &elem]() { int R1 = elem + 1; };
for (int I = 0; I < N; ++I)
auto F2 = [Arr, &I]() { int R2 = Arr[I] + 3; };
@@ -749,8 +770,7 @@
auto F3 = [&Arr, I]() { int R3 = Arr[I]; };
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : Arr)
- // CHECK-FIXES-NEXT: auto F3 = [&Arr, elem]() { int R3 = elem; };
- // FIXME: this does two copies instead of one. Capture elem by ref?
+ // CHECK-FIXES-NEXT: auto F3 = [&Arr, &elem]() { int R3 = elem; };
for (int I = 0; I < N; ++I)
@@ -764,8 +784,7 @@
auto F5 = [&Arr, I]() { int &R5 = Arr[I]; };
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : Arr)
- // CHECK-FIXES-NEXT: auto F5 = [&Arr, elem]() { int &R5 = elem; };
- // FIXME: this does one copy instead of none. Capture elem by ref?
+ // CHECK-FIXES-NEXT: auto F5 = [&Arr, &elem]() { int &R5 = elem; };
for (int I = 0; I < N; ++I)
@@ -782,10 +801,9 @@
}
// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : Arr)
- // CHECK-FIXES-NEXT: auto F = [Arr, elem](int k) {
+ // CHECK-FIXES-NEXT: auto F = [Arr, &elem](int k)
// CHECK-FIXES-NEXT: printf("%d\n", elem + k);
- // CHECK-FIXES-NEXT: };
- // CHECK-FIXES-NEXT: F(elem);
+ // CHECK-FIXES: F(elem);
}
void implicitCapture() {
@@ -815,7 +833,7 @@
}
// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : Arr)
- // CHECK-FIXES-NEXT: auto G3 = [&]() {
+ // CHECK-FIXES-NEXT: auto G3 = [&]()
// CHECK-FIXES-NEXT: int R3 = elem;
// CHECK-FIXES-NEXT: int J3 = elem + R3;
@@ -826,7 +844,7 @@
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : Arr)
- // CHECK-FIXES-NEXT: auto G4 = [=]() {
+ // CHECK-FIXES-NEXT: auto G4 = [=]()
// CHECK-FIXES-NEXT: int R4 = elem + 5;
// Alias by value.
@@ -838,7 +856,7 @@
}
// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto R5 : Arr)
- // CHECK-FIXES-NEXT: auto G5 = [&]() {
+ // CHECK-FIXES-NEXT: auto G5 = [&]()
// CHECK-FIXES: int J5 = 8 + R5;
// Alias by reference.
@@ -850,7 +868,7 @@
}
// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & R6 : Arr)
- // CHECK-FIXES-NEXT: auto G6 = [&]() {
+ // CHECK-FIXES-NEXT: auto G6 = [&]()
// CHECK-FIXES: int J6 = -1 + R6;
}
@@ -883,6 +901,30 @@
auto H5 = [=]() { int R = *I; };
}
+void captureByValue() {
+ // When the index is captured by value, we should replace this by a capture
+ // by reference. This avoids extra copies.
+ // FIXME: this could change semantics on array or pseudoarray loops if the
+ // container is captured by copy.
+ const int N = 10;
+ int Arr[N];
+ dependent<int> Dep;
+
+ for (int I = 0; I < N; ++I) {
+ auto C1 = [&Arr, I]() { if (Arr[I] == 1); };
+ }
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & elem : Arr)
+ // CHECK-FIXES-NEXT: auto C1 = [&Arr, &elem]() { if (elem == 1); };
+
+ for (unsigned I = 0; I < Dep.size(); ++I) {
+ auto C2 = [&Dep, I]() { if (Dep[I] == 2); };
+ }
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & elem : Dep)
+ // CHECK-FIXES-NEXT: auto C2 = [&Dep, &elem]() { if (elem == 2); };
+}
+
} // namespace Lambdas
namespace InitLists {
Index: test/clang-tidy/modernize-loop-convert-basic.cpp
===================================================================
--- test/clang-tidy/modernize-loop-convert-basic.cpp
+++ test/clang-tidy/modernize-loop-convert-basic.cpp
@@ -7,6 +7,7 @@
const int N = 6;
const int NMinusOne = N - 1;
int arr[N] = {1, 2, 3, 4, 5, 6};
+const int constArr[N] = {1, 2, 3, 4, 5, 6};
int (*pArr)[N] = &arr;
void f() {
@@ -17,10 +18,9 @@
int k;
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead [modernize-loop-convert]
- // CHECK-FIXES: for (auto & elem : arr) {
+ // CHECK-FIXES: for (auto & elem : arr)
// CHECK-FIXES-NEXT: sum += elem;
// CHECK-FIXES-NEXT: int k;
- // CHECK-FIXES-NEXT: }
for (int i = 0; i < N; ++i) {
printf("Fibonacci number is %d\n", arr[i]);
@@ -53,9 +53,8 @@
arr[i] += 1;
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & elem : arr) {
+ // CHECK-FIXES: for (auto & elem : arr)
// CHECK-FIXES-NEXT: elem += 1;
- // CHECK-FIXES-NEXT: }
for (int i = 0; i < N; ++i) {
int x = arr[i] + 2;
@@ -77,9 +76,8 @@
sum += arr[i];
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & elem : arr) {
+ // CHECK-FIXES: for (auto & elem : arr)
// CHECK-FIXES-NEXT: sum += elem;
- // CHECK-FIXES-NEXT: }
for (int i = 0; i < N; ++i) {
printf("Fibonacci number %d has address %p\n", arr[i], &arr[i]);
@@ -95,9 +93,17 @@
teas[i].g();
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & tea : teas) {
+ // CHECK-FIXES: for (auto & tea : teas)
// CHECK-FIXES-NEXT: tea.g();
- // CHECK-FIXES-NEXT: }
+}
+
+void constArray() {
+ for (int i = 0; i < N; ++i) {
+ 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-NEXT: printf("2 * %d = %d\n", elem, elem + elem);
}
struct HasArr {
@@ -108,39 +114,35 @@
printf("%d", Arr[i]);
}
// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & elem : Arr) {
+ // CHECK-FIXES: for (auto & elem : Arr)
// CHECK-FIXES-NEXT: printf("%d", elem);
- // CHECK-FIXES-NEXT: }
for (int i = 0; i < N; ++i) {
printf("%d", ValArr[i].x);
}
// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & elem : ValArr) {
+ // CHECK-FIXES: for (auto & elem : ValArr)
// CHECK-FIXES-NEXT: printf("%d", elem.x);
- // CHECK-FIXES-NEXT: }
}
void explicitThis() {
for (int i = 0; i < N; ++i) {
printf("%d", this->Arr[i]);
}
// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & elem : this->Arr) {
+ // CHECK-FIXES: for (auto & elem : this->Arr)
// CHECK-FIXES-NEXT: printf("%d", elem);
- // CHECK-FIXES-NEXT: }
for (int i = 0; i < N; ++i) {
printf("%d", this->ValArr[i].x);
}
// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & elem : this->ValArr) {
+ // CHECK-FIXES: for (auto & elem : this->ValArr)
// CHECK-FIXES-NEXT: printf("%d", elem.x);
- // CHECK-FIXES-NEXT: }
}
};
-// Loops whose bounds are value-dependent shold not be converted.
+// Loops whose bounds are value-dependent should not be converted.
template <int N>
void dependentExprBound() {
for (int i = 0; i < N; ++i)
@@ -263,15 +265,15 @@
printf("Fibonacci number is %d\n", *it);
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & elem : v) {
+ // CHECK-FIXES: for (auto & elem : v)
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
for (dependent<int>::iterator it(v.begin()), e = v.end();
it != e; ++it) {
printf("Fibonacci number is %d\n", *it);
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & elem : v) {
+ // CHECK-FIXES: for (auto & elem : v)
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
doublyDependent<int, int> intmap;
@@ -285,13 +287,14 @@
// PtrSet's iterator dereferences by value so auto & can't be used.
{
- PtrSet<int *> int_ptrs;
- for (PtrSet<int *>::iterator I = int_ptrs.begin(),
- E = int_ptrs.end();
+ PtrSet<int *> val_int_ptrs;
+ for (PtrSet<int *>::iterator I = val_int_ptrs.begin(),
+ E = val_int_ptrs.end();
I != E; ++I) {
+ (void) *I;
}
- // CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto int_ptr : int_ptrs) {
+ // CHECK-MESSAGES: :[[@LINE-5]]:5: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto val_int_ptr : val_int_ptrs)
}
// This container uses an iterator where the derefence type is a typedef of
@@ -302,9 +305,10 @@
for (TypedefDerefContainer<int>::iterator I = int_ptrs.begin(),
E = int_ptrs.end();
I != E; ++I) {
+ (void) *I;
}
- // CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & int_ptr : int_ptrs) {
+ // CHECK-MESSAGES: :[[@LINE-5]]:5: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & int_ptr : int_ptrs)
}
{
@@ -314,10 +318,8 @@
for (RValueDerefContainer<int>::iterator I = container.begin(),
E = container.end();
I != E; ++I) {
+ (void) *I;
}
- // CHECK-FIXES: for (RValueDerefContainer<int>::iterator I = container.begin(),
- // CHECK-FIXES-NEXT: E = container.end();
- // CHECK-FIXES-NEXT: I != E; ++I) {
}
}
@@ -350,17 +352,11 @@
it != e; ++it) {
printf("Fibonacci number is %d\n", *it);
}
- // CHECK-FIXES: for (dependent<int>::const_iterator it = v.begin(), e = v.end();
- // CHECK-FIXES-NEXT: it != e; ++it) {
- // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", *it);
for (dependent<int>::const_iterator it(v.begin()), e = v.end();
it != e; ++it) {
printf("Fibonacci number is %d\n", *it);
}
- // CHECK-FIXES: for (dependent<int>::const_iterator it(v.begin()), e = v.end();
- // CHECK-FIXES-NEXT: it != e; ++it) {
- // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", *it);
}
// Tests to ensure that an implicit 'this' is picked up as the container.
@@ -380,42 +376,45 @@
void doSomething() const;
void doLoop() {
- for (iterator I = begin(), E = end(); I != E; ++I) {
- }
+ for (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 (auto & elem : *this) {
+ // CHECK-FIXES: for (auto & elem : *this)
- for (iterator I = C::begin(), E = C::end(); I != E; ++I) {
- }
+ for (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 (auto & elem : *this) {
+ // CHECK-FIXES: for (auto & elem : *this)
for (iterator I = begin(), E = end(); I != E; ++I) {
+ (void) *I;
doSomething();
}
- for (iterator I = begin(); I != end(); ++I) {
- }
+ for (iterator I = begin(); I != end(); ++I)
+ (void) *I;
// CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto & elem : *this) {
+ // CHECK-FIXES: for (auto & elem : *this)
for (iterator I = begin(); I != end(); ++I) {
+ (void) *I;
doSomething();
}
}
void doLoop() const {
- for (const_iterator I = begin(), E = end(); I != E; ++I) {
- }
+ 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 (auto & elem : *this) {
+ // CHECK-FIXES: for (auto & elem : *this)
- for (const_iterator I = C::begin(), E = C::end(); I != E; ++I) {
- }
+ 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 (auto & elem : *this) {
+ // CHECK-FIXES: for (auto & elem : *this)
for (const_iterator I = begin(), E = end(); I != E; ++I) {
+ (void) *I;
doSomething();
}
}
@@ -431,10 +430,10 @@
void doLoop() {
// The implicit 'this' will have an Implicit cast to const C2* wrapped
// around it. Make sure the replacement still happens.
- for (iterator I = begin(), E = end(); I != E; ++I) {
- }
+ for (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 (auto & elem : *this) {
+ // CHECK-FIXES: for (auto & elem : *this)
}
};
@@ -445,6 +444,8 @@
const int N = 6;
dependent<int> v;
dependent<int> *pv;
+const dependent<int> constv;
+const dependent<int> *pconstv;
transparent<dependent<int>> cv;
@@ -500,59 +501,106 @@
// CHECK-FIXES-NEXT: sum += elem + 2;
}
+void constness() {
+ int sum = 0;
+ for (int i = 0, e = constv.size(); i < e; ++i) {
+ printf("Fibonacci number is %d\n", constv[i]);
+ 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-NEXT: printf("Fibonacci number is %d\n", elem);
+ // CHECK-FIXES-NEXT: sum += elem + 2;
+
+ for (int i = 0, e = constv.size(); i < e; ++i) {
+ printf("Fibonacci number is %d\n", constv.at(i));
+ 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-NEXT: printf("Fibonacci number is %d\n", elem);
+ // CHECK-FIXES-NEXT: sum += elem + 2;
+
+ for (int i = 0, e = pconstv->size(); i < e; ++i) {
+ printf("Fibonacci number is %d\n", pconstv->at(i));
+ 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-NEXT: printf("Fibonacci number is %d\n", elem);
+ // CHECK-FIXES-NEXT: sum += elem + 2;
+
+ // This test will fail if size() isn't called repeatedly, since it
+ // returns unsigned int, and 0 is deduced to be signed int.
+ // FIXME: Insert the necessary explicit conversion, or write out the types
+ // explicitly.
+ for (int i = 0; i < pconstv->size(); ++i) {
+ printf("Fibonacci number is %d\n", (*pconstv).at(i));
+ 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-NEXT: printf("Fibonacci number is %d\n", elem);
+ // CHECK-FIXES-NEXT: sum += elem + 2;
+}
+
// Check for loops that don't mention containers.
void noContainer() {
for (auto i = 0; i < v.size(); ++i) {
}
- // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (const auto & elem : v) {
for (auto i = 0; i < v.size(); ++i)
;
- // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (const auto & elem : v)
}
struct NoBeginEnd {
unsigned size() const;
+ unsigned& operator[](int);
+ const unsigned& operator[](int) const;
};
struct NoConstBeginEnd {
NoConstBeginEnd();
unsigned size() const;
unsigned* begin();
unsigned* end();
+ unsigned& operator[](int);
+ const unsigned& operator[](int) const;
};
struct ConstBeginEnd {
ConstBeginEnd();
unsigned size() const;
unsigned* begin() const;
unsigned* end() const;
+ unsigned& operator[](int);
+ const unsigned& operator[](int) const;
};
// Shouldn't transform pseudo-array uses if the container doesn't provide
// begin() and end() of the right const-ness.
void NoBeginEndTest() {
NoBeginEnd NBE;
- for (unsigned i = 0, e = NBE.size(); i < e; ++i) {
- }
+ for (unsigned i = 0, e = NBE.size(); i < e; ++i)
+ printf("%d\n", NBE[i]);
const NoConstBeginEnd const_NCBE;
- for (unsigned i = 0, e = const_NCBE.size(); i < e; ++i) {
- }
+ for (unsigned i = 0, e = const_NCBE.size(); i < e; ++i)
+ printf("%d\n", const_NCBE[i]);
ConstBeginEnd CBE;
- for (unsigned i = 0, e = CBE.size(); i < e; ++i) {
- }
+ for (unsigned i = 0, e = CBE.size(); i < e; ++i)
+ printf("%d\n", CBE[i]);
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (const auto & elem : CBE) {
+ // CHECK-FIXES: for (auto & elem : CBE)
+ // CHECK-FIXES-NEXT: printf("%d\n", elem);
const ConstBeginEnd const_CBE;
- for (unsigned i = 0, e = const_CBE.size(); i < e; ++i) {
- }
+ 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 (const auto & elem : const_CBE)
+ // CHECK-FIXES-NEXT: printf("%d\n", elem);
}
struct DerefByValue {
@@ -570,16 +618,16 @@
printf("%d\n", DBV[i]);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto elem : DBV) {
+ // CHECK-FIXES: for (auto elem : DBV)
// CHECK-FIXES-NEXT: printf("%d\n", elem);
for (unsigned i = 0, e = DBV.size(); i < e; ++i) {
auto f = [DBV, i]() {};
printf("%d\n", DBV[i]);
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
- // CHECK-FIXES: for (auto elem : DBV) {
- // CHECK-FIXES-NEXT: auto f = [DBV, elem]() {};
+ // CHECK-FIXES: for (auto elem : DBV)
+ // CHECK-FIXES-NEXT: auto f = [DBV, &elem]() {};
// CHECK-FIXES-NEXT: printf("%d\n", elem);
}
Index: test/clang-tidy/Inputs/modernize-loop-convert/structures.h
===================================================================
--- test/clang-tidy/Inputs/modernize-loop-convert/structures.h
+++ test/clang-tidy/Inputs/modernize-loop-convert/structures.h
@@ -59,8 +59,9 @@
};
template<typename ElemType>
-class dependent{
+class dependent {
public:
+ dependent<ElemType>();
struct iterator_base {
const ElemType& operator*()const;
iterator_base& operator ++();
Index: clang-tidy/modernize/LoopConvertUtils.h
===================================================================
--- clang-tidy/modernize/LoopConvertUtils.h
+++ clang-tidy/modernize/LoopConvertUtils.h
@@ -197,14 +197,30 @@
/// \brief The information needed to describe a valid convertible usage
/// of an array index or iterator.
struct Usage {
+ enum UsageKind {
+ // Regular usages of the loop index (the ones not specified below).
+ UK_Default,
+ // Indicates whether this is an access to a member through the arrow
+ // operator on pointers or iterators.
+ UK_MemberThroughArrow,
+ // If the variable is being captured by a lambda, indicates whether the
+ // capture was done by value or by reference.
+ UK_CaptureByCopy,
+ UK_CaptureByRef
+ };
+ // The expression that is going to be converted. Null in case of lambda
+ // captures.
const Expr *Expression;
- bool IsArrow;
+
+ UsageKind Kind;
+
+ // Range that covers this usage.
SourceRange Range;
explicit Usage(const Expr *E)
- : Expression(E), IsArrow(false), Range(Expression->getSourceRange()) {}
- Usage(const Expr *E, bool IsArrow, SourceRange Range)
- : Expression(E), IsArrow(IsArrow), Range(std::move(Range)) {}
+ : Expression(E), Kind(UK_Default), Range(Expression->getSourceRange()) {}
+ Usage(const Expr *E, UsageKind Kind, SourceRange Range)
+ : Expression(E), Kind(Kind), Range(std::move(Range)) {}
};
/// \brief A class to encapsulate lowering of the tool's confidence level.
Index: clang-tidy/modernize/LoopConvertUtils.cpp
===================================================================
--- clang-tidy/modernize/LoopConvertUtils.cpp
+++ clang-tidy/modernize/LoopConvertUtils.cpp
@@ -560,7 +560,7 @@
// If something complicated is happening (i.e. the next token isn't an
// arrow), give up on making this work.
if (!ArrowLoc.isInvalid()) {
- addUsage(Usage(ResultExpr, /*IsArrow=*/true,
+ addUsage(Usage(ResultExpr, Usage::UK_MemberThroughArrow,
SourceRange(Base->getExprLoc(), ArrowLoc)));
return true;
}
@@ -762,7 +762,10 @@
// FIXME: if the index is captured, it will count as an usage and the
// alias (if any) won't work, because it is only used in case of having
// exactly one usage.
- addUsage(Usage(nullptr, false, C->getLocation()));
+ addUsage(Usage(nullptr,
+ C->getCaptureKind() == LCK_ByCopy ? Usage::UK_CaptureByCopy
+ : Usage::UK_CaptureByRef,
+ C->getLocation()));
}
}
return VisitorBase::TraverseLambdaCapture(LE, C);
Index: clang-tidy/modernize/LoopConvertCheck.h
===================================================================
--- clang-tidy/modernize/LoopConvertCheck.h
+++ clang-tidy/modernize/LoopConvertCheck.h
@@ -27,9 +27,9 @@
private:
struct RangeDescriptor {
bool ContainerNeedsDereference;
+ bool DerefByConstRef;
bool DerefByValue;
bool IsTriviallyCopyable;
- bool DerefByConstRef;
};
void doConversion(ASTContext *Context, const VarDecl *IndexVar,
Index: clang-tidy/modernize/LoopConvertCheck.cpp
===================================================================
--- clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tidy/modernize/LoopConvertCheck.cpp
@@ -381,6 +381,20 @@
return true;
}
+/// \brief Returns true if the container is const-qualified.
+static bool containerIsConst(const Expr *ContainerExpr, bool Dereference) {
+ if (const auto *VDec = getReferencedVariable(ContainerExpr)) {
+ QualType CType = VDec->getType();
+ if (Dereference) {
+ if (!CType->isPointerType())
+ return false;
+ CType = CType->getPointeeType();
+ }
+ return CType.isConstQualified();
+ }
+ return false;
+}
+
LoopConvertCheck::LoopConvertCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context), TUInfo(new TUTrackingInfo),
MinConfidence(StringSwitch<Confidence::Level>(
@@ -401,6 +415,11 @@
StringRef ContainerString, const UsageResult &Usages,
const DeclStmt *AliasDecl, bool AliasUseRequired, bool AliasFromForInit,
const ForStmt *TheLoop, RangeDescriptor Descriptor) {
+ // If there aren't any usages, converting the loop would generate an unused
+ // variable warning.
+ if (Usages.size() == 0)
+ return;
+
auto Diag = diag(TheLoop->getForLoc(), "use range-based for loop instead");
std::string VarName;
@@ -436,11 +455,23 @@
VarName = Namer.createIndexName();
// First, replace all usages of the array subscript expression with our new
// variable.
- for (const auto &I : Usages) {
- std::string ReplaceText = I.IsArrow ? VarName + "." : VarName;
+ for (const auto &Usage : Usages) {
+ std::string ReplaceText;
+ if (Usage.Expression) {
+ // If this is an access to a member through the arrow operator, after
+ // the replacement it must be accessed through the '.' operator.
+ ReplaceText = Usage.Kind == Usage::UK_MemberThroughArrow ? VarName + "."
+ : VarName;
+ } else {
+ // The Usage expression is only null in case of lambda captures (which
+ // are VarDecl). If the index is captured by value, add '&' to capture
+ // by reference instead.
+ ReplaceText =
+ Usage.Kind == Usage::UK_CaptureByCopy ? "&" + VarName : VarName;
+ }
TUInfo->getReplacedVars().insert(std::make_pair(TheLoop, IndexVar));
Diag << FixItHint::CreateReplacement(
- CharSourceRange::getTokenRange(I.Range), ReplaceText);
+ CharSourceRange::getTokenRange(Usage.Range), ReplaceText);
}
}
@@ -537,16 +568,24 @@
if (FixerKind == LFK_Array) {
// The array being indexed by IndexVar was discovered during traversal.
ContainerExpr = Finder.getContainerIndexed()->IgnoreParenImpCasts();
+
// Very few loops are over expressions that generate arrays rather than
// array variables. Consider loops over arrays that aren't just represented
// by a variable to be risky conversions.
if (!getReferencedVariable(ContainerExpr) &&
!isDirectMemberExpr(ContainerExpr))
ConfidenceLevel.lowerTo(Confidence::CL_Risky);
+
+ // Use 'const' if the array is const.
+ if (containerIsConst(ContainerExpr, Descriptor.ContainerNeedsDereference))
+ Descriptor.DerefByConstRef = true;
+
} else if (FixerKind == LFK_PseudoArray) {
if (!Descriptor.DerefByValue && !Descriptor.DerefByConstRef) {
const UsageResult &Usages = Finder.getUsages();
- 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,
@@ -558,7 +597,7 @@
if (!U.Expression || U.Expression->getType().isNull())
continue;
QualType Type = U.Expression->getType().getCanonicalType();
- if (U.IsArrow) {
+ if (U.Kind == Usage::UK_MemberThroughArrow) {
if (!Type->isPointerType())
continue;
Type = Type->getPointeeType();
@@ -625,6 +664,7 @@
// expression the loop variable is being tested against instead.
const auto *EndCall = Nodes.getStmtAs<CXXMemberCallExpr>(EndCallName);
const auto *BoundExpr = Nodes.getStmtAs<Expr>(ConditionBoundName);
+
// If the loop calls end()/size() after each iteration, lower our confidence
// level.
if (FixerKind != LFK_Array && !EndVar)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
