[clang-tidy] Fix a crash in modernize-loop-convert around conversion operators

modernize-loop-convert checks and fixes when a loop that iterates over the
elements of a container can be rewritten from a for(...; ...; ...) style into
the "new" C++11 for-range format. For that, it needs to parse the elements of
that loop, like its init-statement, such as ItType it = cont.begin().
modernize-loop-convert checks whether the loop variable is initialized by a
begin() member function.

When an iterator is initialized with a conversion operator (e.g. for
(const_iterator it = non_const_container.begin(); ...), attempts to retrieve the
name of the initializer expression resulted in an assert, as conversion
operators don't have a valid IdentifierInfo.

I fixed this by making digThroughConstructors dig through conversion operators
as well.

Differential Revision: https://reviews.llvm.org/D113201

1 files changed, 11 insertions, 5 deletions

diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
index 74dd4a3..35fe51f 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
@@ -152,20 +152,21 @@ bool DeclFinderASTVisitor::VisitTypeLoc(TypeLoc TL) {
   return true;
 }
 
-/// Look through conversion/copy constructors to find the explicit
-/// initialization expression, returning it is found.
+/// Look through conversion/copy constructors and member functions to find the
+/// explicit initialization expression, returning it is found.
 ///
 /// The main idea is that given
 ///   vector<int> v;
 /// we consider either of these initializations
 ///   vector<int>::iterator it = v.begin();
 ///   vector<int>::iterator it(v.begin());
+///   vector<int>::const_iterator it(v.begin());
 /// and retrieve `v.begin()` as the expression used to initialize `it` but do
 /// not include
 ///   vector<int>::iterator it;
 ///   vector<int>::iterator it(v.begin(), 0); // if this constructor existed
 /// as being initialized from `v.begin()`
-const Expr *digThroughConstructors(const Expr *E) {
+const Expr *digThroughConstructorsConversions(const Expr *E) {
   if (!E)
     return nullptr;
   E = E->IgnoreImplicit();
@@ -178,8 +179,13 @@ const Expr *digThroughConstructors(const Expr *E) {
     E = ConstructExpr->getArg(0);
     if (const auto *Temp = dyn_cast<MaterializeTemporaryExpr>(E))
       E = Temp->getSubExpr();
-    return digThroughConstructors(E);
+    return digThroughConstructorsConversions(E);
   }
+  // If this is a conversion (as iterators commonly convert into their const
+  // iterator counterparts), dig through that as well.
+  if (const auto *ME = dyn_cast<CXXMemberCallExpr>(E))
+    if (const auto *D = dyn_cast<CXXConversionDecl>(ME->getMethodDecl()))
+      return digThroughConstructorsConversions(ME->getImplicitObjectArgument());
   return E;
 }
 
@@ -357,7 +363,7 @@ static bool isAliasDecl(ASTContext *Context, const Decl *TheDecl,
   bool OnlyCasts = true;
   const Expr *Init = VDecl->getInit()->IgnoreParenImpCasts();
   if (isa_and_nonnull<CXXConstructExpr>(Init)) {
-    Init = digThroughConstructors(Init);
+    Init = digThroughConstructorsConversions(Init);
     OnlyCasts = false;
   }
   if (!Init)