diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp @@ -304,8 +304,8 @@ static const Expr *getContainerFromBeginEndCall(const Expr *Init, bool IsBegin, bool *IsArrow, bool IsReverse) { // FIXME: Maybe allow declaration/initialization outside of the for loop. - const auto *TheCall = - dyn_cast_or_null(digThroughConstructors(Init)); + const auto *TheCall = dyn_cast_or_null( + digThroughConstructorsConversions(Init)); if (!TheCall || TheCall->getNumArgs() != 0) return nullptr; diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h @@ -275,7 +275,7 @@ typedef llvm::SmallVector UsageResult; // General functions used by ForLoopIndexUseVisitor and LoopConvertCheck. -const Expr *digThroughConstructors(const Expr *E); +const Expr *digThroughConstructorsConversions(const Expr *E); bool areSameExpr(ASTContext *Context, const Expr *First, const Expr *Second); const DeclRefExpr *getDeclRef(const Expr *E); bool areSameVariable(const ValueDecl *First, const ValueDecl *Second); diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp @@ -152,20 +152,21 @@ 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 v; /// we consider either of these initializations /// vector::iterator it = v.begin(); /// vector::iterator it(v.begin()); +/// vector::const_iterator it(v.begin()); /// and retrieve `v.begin()` as the expression used to initialize `it` but do /// not include /// vector::iterator it; /// vector::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 @@ E = ConstructExpr->getArg(0); if (const auto *Temp = dyn_cast(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(E)) + if (const auto *D = dyn_cast(ME->getMethodDecl())) + return digThroughConstructorsConversions(ME->getImplicitObjectArgument()); return E; } @@ -357,7 +363,7 @@ bool OnlyCasts = true; const Expr *Init = VDecl->getInit()->IgnoreParenImpCasts(); if (isa_and_nonnull(Init)) { - Init = digThroughConstructors(Init); + Init = digThroughConstructorsConversions(Init); OnlyCasts = false; } if (!Init) diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-loop-convert/structures.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-loop-convert/structures.h --- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-loop-convert/structures.h +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-loop-convert/structures.h @@ -53,6 +53,23 @@ iterator end(); }; +struct Q { + typedef int value_type; + struct const_iterator { + value_type &operator*(); + const value_type &operator*() const; + const_iterator &operator++(); + bool operator!=(const const_iterator &other); + void insert(value_type); + value_type X; + }; + struct iterator { + operator const_iterator() const; + }; + iterator begin(); + iterator end(); +}; + struct U { struct iterator { Val& operator*(); diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp @@ -273,6 +273,16 @@ // CHECK-FIXES: for (int & It : Tt) // CHECK-FIXES-NEXT: printf("I found %d\n", It); + // Do not crash because of Qq.begin() converting. Q::iterator converts with a + // conversion operator, which has no name, to Q::const_iterator. + Q Qq; + for (Q::const_iterator It = Qq.begin(), E = Qq.end(); It != E; ++It) { + printf("I found %d\n", *It); + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (int & It : Qq) + // CHECK-FIXES-NEXT: printf("I found %d\n", It); + T *Pt; for (T::iterator It = Pt->begin(), E = Pt->end(); It != E; ++It) { printf("I found %d\n", *It);