Just wanted to note that this patch was contributed by Torbjörn Klatt, and I failed to add the following line to the commit message:
Patch by Torbjörn Klatt! Sorry about that Torbjörn. On Wed, May 15, 2019 at 9:56 AM Don Hinton via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: dhinton > Date: Wed May 15 09:58:58 2019 > New Revision: 360785 > > URL: http://llvm.org/viewvc/llvm-project?rev=360785&view=rev > Log: > [clang-tidy] modernize-loop-convert: impl const cast iter > > Summary: > modernize-loop-convert was not detecting implicit casts to > const_iterator as convertible to range-based loops: > > std::vector<int> vec{1,2,3,4} > for(std::vector<int>::const_iterator i = vec.begin(); > i != vec.end(); > ++i) { } > > Thanks to Don Hinton for advice. > > As well, this change adds a note for this check's applicability to code > targeting OpenMP prior to version 5 as this check will continue breaking > compilation with `-fopenmp`. Thanks to Roman Lebedev for pointing this > out. > > Fixes PR#35082 > > Reviewed By: hintonda > > Tags: #clang-tools-extra, #clang > > Differential Revision: https://reviews.llvm.org/D61827 > > Modified: > clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp > > clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-loop-convert.rst > clang-tools-extra/trunk/docs/clang-tidy/index.rst > > 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=360785&r1=360784&r2=360785&view=diff > > ============================================================================== > --- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp > (original) > +++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp Wed > May 15 09:58:58 2019 > @@ -791,11 +791,6 @@ bool LoopConvertCheck::isConvertible(AST > 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. > > Modified: > clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-loop-convert.rst > URL: > 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 > > ============================================================================== > --- > clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-loop-convert.rst > (original) > +++ > clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-loop-convert.rst > Wed May 15 09:58:58 2019 > @@ -253,3 +253,15 @@ below ends up being performed at the `sa > 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. It is **intentional** that this check does not make any > attempts to > +exclude incorrect diagnostics on OpenMP for loops prior to OpenMP 5. > + > +To prevent this check to be applied (and to break) OpenMP for loops but > still be > +applied to non-OpenMP for loops the usage of ``NOLINT`` (see > +:ref:`clang-tidy-nolint`) on the specific for loops is recommended. > > Modified: clang-tools-extra/trunk/docs/clang-tidy/index.rst > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/index.rst?rev=360785&r1=360784&r2=360785&view=diff > > ============================================================================== > --- clang-tools-extra/trunk/docs/clang-tidy/index.rst (original) > +++ clang-tools-extra/trunk/docs/clang-tidy/index.rst Wed May 15 09:58:58 > 2019 > @@ -258,6 +258,8 @@ An overview of all the command-line opti > value: 'some value' > ... > > +.. _clang-tidy-nolint: > + > Suppressing Undesired Diagnostics > ================================= > > > 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=360785&r1=360784&r2=360785&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 May 15 09:58:58 2019 > @@ -369,7 +369,7 @@ void f() { > // 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 @@ void different_type() { > // 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. > > 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=360785&r1=360784&r2=360785&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 May 15 09:58:58 2019 > @@ -776,17 +776,20 @@ void different_type() { > // 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 @@ void iterators() { > // 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() { > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
