Add support for 'cbegin()' and 'cend()' on modernize-loop-convert.

[angelgarcia]

Summary

This fixes https://llvm.org/bugs/show_bug.cgi?id=22196 .

Also add a non-trivially copyable type to fix some tests that were meant to be about using const-refs, but were changed in r248438.

[klimek]

... Which indicates that the tests might need better names and more comments, so I don't mess them up next time :D

[angelgarcia]

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.

clang-tidy/modernize/LoopConvertCheck.cpp

 ///     EndCallName: 'container.end()' (as a CXXMemberCallExpr)
 ///
 /// Client code will need to make sure that:
 ///   - The iterator variables 'it', 'f', and 'h' are the same.
 ///   - The two containers on which 'begin' and 'end' are called are the same.
 ///   - 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 =
       varDecl(hasInitializer(anyOf(ignoringParenImpCasts(BeginCallMatcher),
                                    materializeTemporaryExpr(
                                        ignoringParenImpCasts(BeginCallMatcher)),
                                    hasDescendant(BeginCallMatcher))))
           .bind(InitVarName);
 
   DeclarationMatcher EndDeclMatcher =
       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(
                      declRefExpr(to(varDecl().bind(ConditionEndVarName)))),
                  ignoringParenImpCasts(expr(EndCallMatcher).bind(EndCallName)),
                  materializeTemporaryExpr(ignoringParenImpCasts(
                      expr(EndCallMatcher).bind(EndCallName)))));
 
static const Expr *getContainerFromBeginEndCall(const Expr *Init, bool IsBegin,
   if (!TheCall || TheCall->getNumArgs() != 0)
     return nullptr;
 
   const auto *Member = dyn_cast<MemberExpr>(TheCall->getCallee());
   if (!Member)
     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();
   if (!SourceExpr)
     return nullptr;
 
   *IsArrow = Member->isArrow();
   return SourceExpr;
: ClangTidyCheck(Name, Context), TUInfo(new TUTrackingInfo),
                       .Case("UPPER_CASE", VariableNamer::NS_UpperCase)
                       .Default(VariableNamer::NS_CamelCase)) {}
 
 void LoopConvertCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   SmallVector<std::string, 3> Confs{"risky", "reasonable", "safe"};
   Options.store(Opts, "MinConfidence", Confs[static_cast<int>(MinConfidence)]);
 
   SmallVector<std::string, 4> Styles{"camelBack", "CamelCase", "lower_case",
                                      "UPPER_CASE"};
   Options.store(Opts, "NamingStyle", Styles[static_cast<int>(NamingStyle)]);
 }
 
 void LoopConvertCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matchers for C++. Because this checker is used for
   // modernization, it is reasonable to run it on any C++ standard with the
   // assumption the user is trying to modernize their codebase.
   if (!getLangOpts().CPlusPlus)

test/clang-tidy/Inputs/modernize-loop-convert/structures.h

struct MutableVal {
   void constFun(int) const;
   void nonConstFun(int, int);
   void constFun(MutableVal &) const;
   void constParamFun(const MutableVal &) const;
   void nonConstParamFun(const MutableVal &);
   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();
 };
 
 struct T {
   struct iterator {
     int& operator*();
     const int& operator*()const;

test/clang-tidy/modernize-loop-convert-basic.cpp

 
 void constArray() {
   for (int I = 0; I < N; ++I) {
     printf("2 * %d = %d\n", ConstArr[I], ConstArr[I] + ConstArr[I]);
   }
   // 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 {
   int Arr[N];
   Val ValArr[N];
   void implicitThis() {
     for (int I = 0; I < N; ++I) {
       printf("%d", Arr[I]);
void f() {
   S *Ps;
   for (S::iterator It = Ps->begin(), E = Ps->end(); 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 & 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);
   }
   // 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) {

 
 } // namespace Iterator
 
 namespace PseudoArray {
 
 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;
 
 void f() {
   int Sum = 0;
   for (int I = 0, E = V.size(); I < E; ++I) {
     printf("Fibonacci number is %d\n", V[I]);
     Sum += V[I] + 2;
for (int I = 0; I < Cv->size(); ++I) {
     Sum += Cv->at(I) + 2;
   }
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & Elem : *Cv)
   // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem);
   // 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.
   for (int i = 0; i < ConstVRef.size(); ++i) {
     sum += ConstVRef[i];
   }
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto Elem : ConstVRef)