[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
This commit is contained in:
parent
2a3878ea16
commit
29a8d45c5a
|
@ -304,8 +304,8 @@ StatementMatcher makePseudoArrayLoopMatcher() {
|
|||
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<CXXMemberCallExpr>(digThroughConstructors(Init));
|
||||
const auto *TheCall = dyn_cast_or_null<CXXMemberCallExpr>(
|
||||
digThroughConstructorsConversions(Init));
|
||||
if (!TheCall || TheCall->getNumArgs() != 0)
|
||||
return nullptr;
|
||||
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -275,7 +275,7 @@ private:
|
|||
typedef llvm::SmallVector<Usage, 8> 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);
|
||||
|
|
|
@ -53,6 +53,23 @@ struct T {
|
|||
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*();
|
||||
|
|
|
@ -273,6 +273,16 @@ void f() {
|
|||
// 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);
|
||||
|
|
Loading…
Reference in New Issue