[clang-tools-extra] r360785 - [clang-tidy] modernize-loop-convert: impl const cast iter
Don Hinton via cfe-commits
cfe-commits at lists.llvm.org
Wed May 15 10:08:18 PDT 2019
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 at 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 at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190515/555d7ea2/attachment.html>
More information about the cfe-commits
mailing list