Author: angelgarcia Date: Tue Nov 3 10:31:36 2015 New Revision: 251940 URL: http://llvm.org/viewvc/llvm-project?rev=251940&view=rev Log: Improve more the const-detection in modernize-loop-convert.
Summary: The previous change was focused in detecting when a non-const object was used in a constant way. Looks like I forgot the most important and trivial case: when the object is already constant. Failing to detect this cases results in compile errors, due to trying to bind a constant object to a non-const reference in the range-for statement. This change should fix that. Reviewers: klimek Subscribers: alexfh, cfe-commits Differential Revision: http://reviews.llvm.org/D14282 Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-const.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=251940&r1=251939&r2=251940&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp Tue Nov 3 10:31:36 2015 @@ -372,6 +372,8 @@ static bool isDirectMemberExpr(const Exp /// containter that we are iterating over, returns false when it can be /// guaranteed this element cannot be modified as a result of this usage. static bool canBeModified(ASTContext *Context, const Expr *E) { + if (E->getType().isConstQualified()) + return false; auto Parents = Context->getParents(*E); if (Parents.size() != 1) return true; Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-const.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-const.cpp?rev=251940&r1=251939&r2=251940&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-const.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-const.cpp Tue Nov 3 10:31:36 2015 @@ -271,28 +271,91 @@ void takingReferences() { // Aliases. for (int I = 0; I < N; ++I) { const Str &J = Array[I]; - (void) J; + (void)J; } // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop // CHECK-FIXES: for (auto J : Array) for (int I = 0; I < N; ++I) { Str &J = Array[I]; - (void) J; + (void)J; } // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop // CHECK-FIXES: for (auto & J : Array) for (int I = 0; I < N; ++I) { const int &J = Ints[I]; - (void) J; + (void)J; } // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop // CHECK-FIXES: for (int J : Ints) for (int I = 0; I < N; ++I) { int &J = Ints[I]; - (void) J; + (void)J; } // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop // CHECK-FIXES: for (int & J : Ints) } + +template <class T> +struct vector { + unsigned size() const; + const T &operator[](int) const; + T &operator[](int); + T *begin(); + T *end(); + const T *begin() const; + const T *end() const; +}; + +// If the elements are already constant, we won't do any ImplicitCast to const. +void testContainerOfConstElements() { + const int Ints[N]{}; + for (int I = 0; I < N; ++I) { + OtherInt -= Ints[I]; + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop + // CHECK-FIXES: for (int Int : Ints) + + vector<const Str> Strs; + for (int I = 0; I < Strs.size(); ++I) { + Strs[I].constMember(0); + constRefArg(Strs[I]); + } + // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop + // CHECK-FIXES: for (auto Str : Strs) +} + +// When we are inside a const-qualified member functions, all the data members +// are implicitly set as const. As before, there won't be any ImplicitCast to +// const in their usages. +class TestInsideConstFunction { + const static int N = 10; + int Ints[N]; + Str Array[N]; + vector<int> V; + + void foo() const { + for (int I = 0; I < N; ++I) { + if (Ints[I]) + copyArg(Ints[I]); + } + // CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use range-based for loop + // CHECK-FIXES: for (int Elem : Ints) + + for (int I = 0; I < N; ++I) { + Array[I].constMember(0); + constRefArg(Array[I]); + } + // CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use range-based for loop + // CHECK-FIXES: for (auto Elem : Array) + + for (int I = 0; I < V.size(); ++I) { + if (V[I]) + copyArg(V[I]); + } + // CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use range-based for loop + // CHECK-FIXES: for (int Elem : V) + + } +}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
