From 4ecb581188ff34c049eb2e9f36b32ae6df532221 Mon Sep 17 00:00:00 2001 From: Don Hinton Date: Wed, 15 May 2019 17:36:54 +0000 Subject: [PATCH] Revert [clang-tidy] modernize-loop-convert: impl const cast iter This reverts r360785 (git commit 42d28be802fe5beab18bc1a27f89894c0a290d44) llvm-svn: 360787 --- .../clang-tidy/modernize/LoopConvertCheck.cpp | 5 +++++ .../docs/clang-tidy/checks/modernize-loop-convert.rst | 12 ------------ clang-tools-extra/docs/clang-tidy/index.rst | 2 -- .../test/clang-tidy/modernize-loop-convert-basic.cpp | 11 ++++------- .../test/clang-tidy/modernize-loop-convert-extra.cpp | 19 ++++--------------- 5 files changed, 13 insertions(+), 36 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp index 0e59a88..f9e941c 100644 --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp @@ -791,6 +791,11 @@ bool LoopConvertCheck::isConvertible(ASTContext *Context, 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. diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst index 82b27bb..bad574f 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst @@ -253,15 +253,3 @@ below ends up being performed at the `safe` level. 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. diff --git a/clang-tools-extra/docs/clang-tidy/index.rst b/clang-tools-extra/docs/clang-tidy/index.rst index dc186b6..1b5af60 100644 --- a/clang-tools-extra/docs/clang-tidy/index.rst +++ b/clang-tools-extra/docs/clang-tidy/index.rst @@ -258,8 +258,6 @@ An overview of all the command-line options: value: 'some value' ... -.. _clang-tidy-nolint: - Suppressing Undesired Diagnostics ================================= diff --git a/clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp b/clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp index 0cd1087..def7c4b 100644 --- a/clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp +++ b/clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp @@ -369,7 +369,7 @@ void f() { // CHECK-FIXES: for (auto Val_int_ptr : Val_int_ptrs) } - // This container uses an iterator where the dereference type is a typedef of + // This container uses an iterator where the derefence 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,22 +431,19 @@ 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 V; for (dependent::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::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. diff --git a/clang-tools-extra/test/clang-tidy/modernize-loop-convert-extra.cpp b/clang-tools-extra/test/clang-tidy/modernize-loop-convert-extra.cpp index d7011f7..b46ff25 100644 --- a/clang-tools-extra/test/clang-tidy/modernize-loop-convert-extra.cpp +++ b/clang-tools-extra/test/clang-tidy/modernize-loop-convert-extra.cpp @@ -776,20 +776,17 @@ 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 V; for (dependent::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::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 @@ -994,26 +991,18 @@ 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::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::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::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() { -- 2.7.4