<div dir="ltr">Just wanted to note that this patch was contribu<span style="background-color:rgb(255,255,255)">ted by <span style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif">Torbjörn Klatt, and I failed to add the following line to the commit message:</span></span><div><span style="background-color:rgb(255,255,255)"><span style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif"><br></span></span></div><div><span style="background-color:rgb(255,255,255)"><span style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif">Patch by </span></span><span style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif">Torbjörn Klatt!</span></div><div><span style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif"><br></span></div><div><span style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif">Sorry about that </span><span style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif">Torbjörn.</span></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, May 15, 2019 at 9:56 AM Don Hinton via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: dhinton<br>
Date: Wed May 15 09:58:58 2019<br>
New Revision: 360785<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=360785&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=360785&view=rev</a><br>
Log:<br>
[clang-tidy] modernize-loop-convert: impl const cast iter<br>
<br>
Summary:<br>
modernize-loop-convert was not detecting implicit casts to<br>
const_iterator as convertible to range-based loops:<br>
<br>
std::vector<int> vec{1,2,3,4}<br>
for(std::vector<int>::const_iterator i = vec.begin();<br>
i != vec.end();<br>
++i) { }<br>
<br>
Thanks to Don Hinton for advice.<br>
<br>
As well, this change adds a note for this check's applicability to code<br>
targeting OpenMP prior to version 5 as this check will continue breaking<br>
compilation with `-fopenmp`. Thanks to Roman Lebedev for pointing this<br>
out.<br>
<br>
Fixes PR#35082<br>
<br>
Reviewed By: hintonda<br>
<br>
Tags: #clang-tools-extra, #clang<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D61827" rel="noreferrer" target="_blank">https://reviews.llvm.org/D61827</a><br>
<br>
Modified:<br>
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp<br>
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-loop-convert.rst<br>
clang-tools-extra/trunk/docs/clang-tidy/index.rst<br>
clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp<br>
clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp<br>
<br>
Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp?rev=360785&r1=360784&r2=360785&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp?rev=360785&r1=360784&r2=360785&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp (original)<br>
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp Wed May 15 09:58:58 2019<br>
@@ -791,11 +791,6 @@ bool LoopConvertCheck::isConvertible(AST<br>
CanonicalBeginType->getPointeeType(),<br>
CanonicalInitVarType->getPointeeType()))<br>
return false;<br>
- } else if (!Context->hasSameType(CanonicalInitVarType,<br>
- CanonicalBeginType)) {<br>
- // Check for qualified types to avoid conversions from non-const to const<br>
- // iterator types.<br>
- return false;<br>
}<br>
} else if (FixerKind == LFK_PseudoArray) {<br>
// This call is required to obtain the container.<br>
<br>
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-loop-convert.rst<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-loop-convert.rst?rev=360785&r1=360784&r2=360785&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-loop-convert.rst?rev=360785&r1=360784&r2=360785&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-loop-convert.rst (original)<br>
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-loop-convert.rst Wed May 15 09:58:58 2019<br>
@@ -253,3 +253,15 @@ below ends up being performed at the `sa<br>
flag = true;<br>
}<br>
}<br>
+<br>
+OpenMP<br>
+^^^^^^<br>
+<br>
+As range-based for loops are only available since OpenMP 5, this check should<br>
+not been used on code with a compatibility requirements of OpenMP prior to<br>
+version 5. It is **intentional** that this check does not make any attempts to<br>
+exclude incorrect diagnostics on OpenMP for loops prior to OpenMP 5.<br>
+<br>
+To prevent this check to be applied (and to break) OpenMP for loops but still be<br>
+applied to non-OpenMP for loops the usage of ``NOLINT`` (see<br>
+:ref:`clang-tidy-nolint`) on the specific for loops is recommended.<br>
<br>
Modified: clang-tools-extra/trunk/docs/clang-tidy/index.rst<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/index.rst?rev=360785&r1=360784&r2=360785&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/index.rst?rev=360785&r1=360784&r2=360785&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/docs/clang-tidy/index.rst (original)<br>
+++ clang-tools-extra/trunk/docs/clang-tidy/index.rst Wed May 15 09:58:58 2019<br>
@@ -258,6 +258,8 @@ An overview of all the command-line opti<br>
value: 'some value'<br>
...<br>
<br>
+.. _clang-tidy-nolint:<br>
+<br>
Suppressing Undesired Diagnostics<br>
=================================<br>
<br>
<br>
Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp?rev=360785&r1=360784&r2=360785&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp?rev=360785&r1=360784&r2=360785&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp (original)<br>
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp Wed May 15 09:58:58 2019<br>
@@ -369,7 +369,7 @@ void f() {<br>
// CHECK-FIXES: for (auto Val_int_ptr : Val_int_ptrs)<br>
}<br>
<br>
- // This container uses an iterator where the derefence type is a typedef of<br>
+ // This container uses an iterator where the dereference type is a typedef of<br>
// a reference type. Make sure non-const auto & is still used. A failure here<br>
// means canonical types aren't being tested.<br>
{<br>
@@ -431,19 +431,22 @@ void different_type() {<br>
// CHECK-FIXES: for (auto P : *Ps)<br>
// CHECK-FIXES-NEXT: printf("s has value %d\n", P.X);<br>
<br>
- // V.begin() returns a user-defined type 'iterator' which, since it's<br>
- // different from const_iterator, disqualifies these loops from<br>
- // transformation.<br>
dependent<int> V;<br>
for (dependent<int>::const_iterator It = V.begin(), E = V.end();<br>
It != E; ++It) {<br>
printf("Fibonacci number is %d\n", *It);<br>
}<br>
+ // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead<br>
+ // CHECK-FIXES: for (int It : V)<br>
+ // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", It);<br>
<br>
for (dependent<int>::const_iterator It(V.begin()), E = V.end();<br>
It != E; ++It) {<br>
printf("Fibonacci number is %d\n", *It);<br>
}<br>
+ // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead<br>
+ // CHECK-FIXES: for (int It : V)<br>
+ // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", It);<br>
}<br>
<br>
// Tests to ensure that an implicit 'this' is picked up as the container.<br>
<br>
Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp?rev=360785&r1=360784&r2=360785&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp?rev=360785&r1=360784&r2=360785&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp (original)<br>
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp Wed May 15 09:58:58 2019<br>
@@ -776,17 +776,20 @@ void different_type() {<br>
// CHECK-FIXES: for (auto P : *Ps)<br>
// CHECK-FIXES-NEXT: printf("s has value %d\n", P.X);<br>
<br>
- // V.begin() returns a user-defined type 'iterator' which, since it's<br>
- // different from const_iterator, disqualifies these loops from<br>
- // transformation.<br>
dependent<int> V;<br>
for (dependent<int>::const_iterator It = V.begin(); It != V.end(); ++It) {<br>
printf("Fibonacci number is %d\n", *It);<br>
}<br>
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead<br>
+ // CHECK-FIXES: for (int It : V)<br>
+ // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", It);<br>
<br>
for (dependent<int>::const_iterator It(V.begin()); It != V.end(); ++It) {<br>
printf("Fibonacci number is %d\n", *It);<br>
}<br>
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead<br>
+ // CHECK-FIXES: for (int It : V)<br>
+ // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", It);<br>
}<br>
<br>
} // namespace SingleIterator<br>
@@ -991,18 +994,26 @@ void iterators() {<br>
// CHECK-FIXES: for (int & I : Dep)<br>
// CHECK-FIXES-NEXT: auto H2 = [&]() { int R = I + 2; };<br>
<br>
- // FIXME: It doesn't work with const iterators.<br>
for (dependent<int>::const_iterator I = Dep.begin(), E = Dep.end();<br>
I != E; ++I)<br>
auto H3 = [I]() { int R = *I; };<br>
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead<br>
+ // CHECK-FIXES: for (int I : Dep)<br>
+ // CHECK-FIXES-NEXT: auto H3 = [&I]() { int R = I; };<br>
<br>
for (dependent<int>::const_iterator I = Dep.begin(), E = Dep.end();<br>
I != E; ++I)<br>
auto H4 = [&]() { int R = *I + 1; };<br>
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead<br>
+ // CHECK-FIXES: for (int I : Dep)<br>
+ // CHECK-FIXES-NEXT: auto H4 = [&]() { int R = I + 1; };<br>
<br>
for (dependent<int>::const_iterator I = Dep.begin(), E = Dep.end();<br>
I != E; ++I)<br>
auto H5 = [=]() { int R = *I; };<br>
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead<br>
+ // CHECK-FIXES: for (int R : Dep)<br>
+ // CHECK-FIXES-NEXT: auto H5 = [=]() { };<br>
}<br>
<br>
void captureByValue() {<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>