Szelethus created this revision.
Szelethus added reviewers: aaron.ballman, alexfh, steakhal, whisperity.
Szelethus added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, martong, gamesh411, dkrupp, rnkovacs,
xazax.hun.
Szelethus requested review of this revision.
Herald added a subscriber: cfe-commits.
When an iterator is initialized with a conversion operator (e.g. `for
(const_iterator it = non_const_container.begin(); ...)`,
`getContainerFromBeginEndCall()` failed with an assert. It attempts to retrieve
the name of the initializer expression, which in the case of a conversion
operator doesn't exist.
I fixed this by making `digThroughConstructors` dig through conversion
operators as well.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D113201
Files:
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-loop-convert/structures.h
clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp
+++ 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 Qt.begin() converting. Q::converting_iterator
+ // converts with a conversion iterator, which has no name.
+ Q Qt;
+ for (Q::iterator It = Qt.begin(), E = Qt.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 : Qt)
+ // 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);
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-loop-convert/structures.h
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-loop-convert/structures.h
+++ 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 iterator {
+ value_type &operator*();
+ const value_type &operator*() const;
+ iterator &operator++();
+ bool operator!=(const iterator &other);
+ void insert(value_type);
+ value_type X;
+ };
+ struct converting_iterator {
+ operator iterator() const;
+ };
+ converting_iterator begin();
+ converting_iterator end();
+};
+
struct U {
struct iterator {
Val& operator*();
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
===================================================================
--- clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
@@ -275,7 +275,7 @@
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);
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
@@ -7,6 +7,8 @@
//===----------------------------------------------------------------------===//
#include "LoopConvertUtils.h"
+#include "clang/AST/Expr.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/Lambda.h"
@@ -152,7 +154,7 @@
return true;
}
-/// Look through conversion/copy constructors to find the explicit
+/// Look through conversion/copy constructors and operators to find the explicit
/// initialization expression, returning it is found.
///
/// The main idea is that given
@@ -165,7 +167,7 @@
/// 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 +180,13 @@
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 +364,7 @@
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)
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ 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<CXXMemberCallExpr>(digThroughConstructors(Init));
+ const auto *TheCall = dyn_cast_or_null<CXXMemberCallExpr>(
+ digThroughConstructorsConversions(Init));
if (!TheCall || TheCall->getNumArgs() != 0)
return nullptr;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
