angelgarcia updated this revision to Diff 36198. angelgarcia added a comment.
Yes, right now it is pretty hard to figure out what some of the tests are for, they are a bit messy. I plan to do something about it, but for now I added a comment on that test. http://reviews.llvm.org/D13292 Files: clang-tidy/modernize/LoopConvertCheck.cpp test/clang-tidy/Inputs/modernize-loop-convert/structures.h test/clang-tidy/modernize-loop-convert-basic.cpp
Index: test/clang-tidy/modernize-loop-convert-basic.cpp =================================================================== --- test/clang-tidy/modernize-loop-convert-basic.cpp +++ test/clang-tidy/modernize-loop-convert-basic.cpp @@ -104,6 +104,14 @@ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto Elem : ConstArr) // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", Elem, Elem + Elem); + + const NonTriviallyCopyable NonCopy[N]{}; + for (int I = 0; I < N; ++I) { + printf("2 * %d = %d\n", NonCopy[I].X, NonCopy[I].X + NonCopy[I].X); + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (const auto & Elem : NonCopy) + // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", Elem.X, Elem.X + Elem.X); } struct HasArr { @@ -209,6 +217,13 @@ // CHECK-FIXES: for (auto & P : *Ps) // CHECK-FIXES-NEXT: printf("s has value %d\n", P.X); + for (S::const_iterator It = Ss.cbegin(), E = Ss.cend(); It != E; ++It) { + printf("s has value %d\n", (*It).X); + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (auto Elem : Ss) + // CHECK-FIXES-NEXT: printf("s has value %d\n", Elem.X); + for (S::iterator It = Ss.begin(), E = Ss.end(); It != E; ++It) { printf("s has value %d\n", It->X); } @@ -459,8 +474,8 @@ const int N = 6; dependent<int> V; dependent<int> *Pv; -const dependent<int> Constv; -const dependent<int> *Pconstv; +const dependent<NonTriviallyCopyable> Constv; +const dependent<NonTriviallyCopyable> *Pconstv; transparent<dependent<int>> Cv; @@ -516,50 +531,51 @@ // CHECK-FIXES-NEXT: Sum += Elem + 2; } +// Ensure that 'const auto &' is used with containers of non-trivial types. void constness() { int Sum = 0; for (int I = 0, E = Constv.size(); I < E; ++I) { - printf("Fibonacci number is %d\n", Constv[I]); - Sum += Constv[I] + 2; + printf("Fibonacci number is %d\n", Constv[I].X); + Sum += Constv[I].X + 2; } // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead - // CHECK-FIXES: for (auto Elem : Constv) - // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem); - // CHECK-FIXES-NEXT: Sum += Elem + 2; + // CHECK-FIXES: for (const auto & Elem : Constv) + // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem.X); + // CHECK-FIXES-NEXT: Sum += Elem.X + 2; for (int I = 0, E = Constv.size(); I < E; ++I) { - printf("Fibonacci number is %d\n", Constv.at(I)); - Sum += Constv.at(I) + 2; + printf("Fibonacci number is %d\n", Constv.at(I).X); + Sum += Constv.at(I).X + 2; } // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead - // CHECK-FIXES: for (auto Elem : Constv) - // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem); - // CHECK-FIXES-NEXT: Sum += Elem + 2; + // CHECK-FIXES: for (const auto & Elem : Constv) + // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem.X); + // CHECK-FIXES-NEXT: Sum += Elem.X + 2; for (int I = 0, E = Pconstv->size(); I < E; ++I) { - printf("Fibonacci number is %d\n", Pconstv->at(I)); - Sum += Pconstv->at(I) + 2; + printf("Fibonacci number is %d\n", Pconstv->at(I).X); + Sum += Pconstv->at(I).X + 2; } // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead - // CHECK-FIXES: for (auto Elem : *Pconstv) - // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem); - // CHECK-FIXES-NEXT: Sum += Elem + 2; + // CHECK-FIXES: for (const auto & Elem : *Pconstv) + // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem.X); + // CHECK-FIXES-NEXT: Sum += Elem.X + 2; // This test will fail if size() isn't called repeatedly, since it // returns unsigned int, and 0 is deduced to be signed int. // FIXME: Insert the necessary explicit conversion, or write out the types // explicitly. for (int I = 0; I < Pconstv->size(); ++I) { - printf("Fibonacci number is %d\n", (*Pconstv).at(I)); - Sum += (*Pconstv)[I] + 2; + printf("Fibonacci number is %d\n", (*Pconstv).at(I).X); + Sum += (*Pconstv)[I].X + 2; } // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead - // CHECK-FIXES: for (auto Elem : *Pconstv) - // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem); - // CHECK-FIXES-NEXT: Sum += Elem + 2; + // CHECK-FIXES: for (const auto & Elem : *Pconstv) + // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem.X); + // CHECK-FIXES-NEXT: Sum += Elem.X + 2; } -void ConstRef(const dependent<int>& ConstVRef) { +void constRef(const dependent<int>& ConstVRef) { int sum = 0; // FIXME: This does not work with size_t (probably due to the implementation // of dependent); make dependent work exactly like a std container type. Index: test/clang-tidy/Inputs/modernize-loop-convert/structures.h =================================================================== --- test/clang-tidy/Inputs/modernize-loop-convert/structures.h +++ test/clang-tidy/Inputs/modernize-loop-convert/structures.h @@ -16,11 +16,20 @@ int X; }; +struct NonTriviallyCopyable { + NonTriviallyCopyable() = default; + // Define this constructor to make this class non-trivially copyable. + NonTriviallyCopyable(const NonTriviallyCopyable& Ntc); + int X; +}; + struct S { typedef MutableVal *iterator; typedef const MutableVal *const_iterator; const_iterator begin() const; const_iterator end() const; + const_iterator cbegin() const; + const_iterator cend() const; iterator begin(); iterator end(); }; Index: clang-tidy/modernize/LoopConvertCheck.cpp =================================================================== --- clang-tidy/modernize/LoopConvertCheck.cpp +++ clang-tidy/modernize/LoopConvertCheck.cpp @@ -112,8 +112,9 @@ /// - If the end iterator variable 'g' is defined, it is the same as 'f'. StatementMatcher makeIteratorLoopMatcher() { StatementMatcher BeginCallMatcher = - cxxMemberCallExpr(argumentCountIs(0), - callee(cxxMethodDecl(hasName("begin")))) + cxxMemberCallExpr( + argumentCountIs(0), + callee(cxxMethodDecl(anyOf(hasName("begin"), hasName("cbegin"))))) .bind(BeginCallName); DeclarationMatcher InitDeclMatcher = @@ -127,7 +128,8 @@ varDecl(hasInitializer(anything())).bind(EndVarName); StatementMatcher EndCallMatcher = cxxMemberCallExpr( - argumentCountIs(0), callee(cxxMethodDecl(hasName("end")))); + argumentCountIs(0), + callee(cxxMethodDecl(anyOf(hasName("end"), hasName("cend"))))); StatementMatcher IteratorBoundMatcher = expr(anyOf(ignoringParenImpCasts( @@ -296,7 +298,8 @@ return nullptr; StringRef Name = Member->getMemberDecl()->getName(); StringRef TargetName = IsBegin ? "begin" : "end"; - if (Name != TargetName) + StringRef ConstTargetName = IsBegin ? "cbegin" : "cend"; + if (Name != TargetName && Name != ConstTargetName) return nullptr; const Expr *SourceExpr = Member->getBase(); @@ -423,7 +426,7 @@ Options.store(Opts, "MinConfidence", Confs[static_cast<int>(MinConfidence)]); SmallVector<std::string, 4> Styles{"camelBack", "CamelCase", "lower_case", - "UPPER_CASE"}; + "UPPER_CASE"}; Options.store(Opts, "NamingStyle", Styles[static_cast<int>(NamingStyle)]); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits