torbjoernk updated this revision to Diff 199326.
torbjoernk edited the summary of this revision.
torbjoernk added a comment.
Herald added subscribers: jdoerfert, jfb.
Fixed the issues pointed out by Don Hinton and added note on OpenMP to the
check's docs as suggested by Roman Lebedev.
Also fixed a small typo in a comment within the tests.
Side note: Somehow, the various failing tests I had previously where due to the
fact that `make check-clang-tools` was picking my system-installed clang-tidy-8
and not the one I was building... o.O
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61827/new/
https://reviews.llvm.org/D61827
Files:
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp
clang-tools-extra/test/clang-tidy/modernize-loop-convert-extra.cpp
Index: clang-tools-extra/test/clang-tidy/modernize-loop-convert-extra.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/modernize-loop-convert-extra.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-loop-convert-extra.cpp
@@ -776,17 +776,20 @@
// 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
- // different from const_iterator, disqualifies these loops from
- // transformation.
dependent<int> V;
for (dependent<int>::const_iterator It = V.begin(); It != V.end(); ++It) {
printf("Fibonacci number is %d\n", *It);
}
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (int It : V)
+ // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", It);
for (dependent<int>::const_iterator It(V.begin()); It != V.end(); ++It) {
printf("Fibonacci number is %d\n", *It);
}
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (int It : V)
+ // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", It);
}
} // namespace SingleIterator
@@ -991,18 +994,26 @@
// CHECK-FIXES: for (int & I : Dep)
// CHECK-FIXES-NEXT: auto H2 = [&]() { int R = I + 2; };
- // FIXME: It doesn't work with const iterators.
for (dependent<int>::const_iterator I = Dep.begin(), E = Dep.end();
I != E; ++I)
auto H3 = [I]() { int R = *I; };
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (int I : Dep)
+ // CHECK-FIXES-NEXT: auto H3 = [&I]() { int R = I; };
for (dependent<int>::const_iterator I = Dep.begin(), E = Dep.end();
I != E; ++I)
auto H4 = [&]() { int R = *I + 1; };
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (int I : Dep)
+ // CHECK-FIXES-NEXT: auto H4 = [&]() { int R = I + 1; };
for (dependent<int>::const_iterator I = Dep.begin(), E = Dep.end();
I != E; ++I)
auto H5 = [=]() { int R = *I; };
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (int R : Dep)
+ // CHECK-FIXES-NEXT: auto H5 = [=]() { };
}
void captureByValue() {
Index: clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp
@@ -369,7 +369,7 @@
// CHECK-FIXES: for (auto Val_int_ptr : Val_int_ptrs)
}
- // This container uses an iterator where the derefence type is a typedef of
+ // This container uses an iterator where the dereference type is a typedef of
// a reference type. Make sure non-const auto & is still used. A failure here
// means canonical types aren't being tested.
{
@@ -431,19 +431,22 @@
// 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
- // different from const_iterator, disqualifies these loops from
- // transformation.
dependent<int> V;
for (dependent<int>::const_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 (int It : V)
+ // 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-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (int It : V)
+ // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", It);
}
// Tests to ensure that an implicit 'this' is picked up as the container.
Index: clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
@@ -253,3 +253,10 @@
flag = true;
}
}
+
+OpenMP
+^^^^^^
+
+As range-based for loops are only available since OpenMP 5, this check should
+not been used on code with a compatibility requirements of OpenMP prior to
+version 5.
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -791,11 +791,6 @@
CanonicalBeginType->getPointeeType(),
CanonicalInitVarType->getPointeeType()))
return false;
- } else if (!Context->hasSameType(CanonicalInitVarType,
- CanonicalBeginType)) {
- // Check for qualified types to avoid conversions from non-const to const
- // iterator types.
- return false;
}
} else if (FixerKind == LFK_PseudoArray) {
// This call is required to obtain the container.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
