angelgarcia updated this revision to Diff 39530.
angelgarcia added a comment.
The test revealed a (already existing) bug. If we called getName() on a
CXXConversionDecl, we would get the following assertion:
include/clang/AST/Decl.h:170: llvm::StringRef clang::NamedDecl::getName()
const: Assertion `Name.isIdentifier() && "Name is not a simple identifier"'
failed.
Solved now.
http://reviews.llvm.org/D14442
Files:
clang-tidy/modernize/LoopConvertUtils.cpp
test/clang-tidy/modernize-loop-convert-extra.cpp
Index: test/clang-tidy/modernize-loop-convert-extra.cpp
===================================================================
--- test/clang-tidy/modernize-loop-convert-extra.cpp
+++ test/clang-tidy/modernize-loop-convert-extra.cpp
@@ -159,14 +159,33 @@
// CHECK-FIXES: for (int Alias : IntArr)
// CHECK-FIXES-NEXT: for (unsigned J = 0; Alias; ++J)
- struct IntRef { IntRef(const int& i); };
+ struct IntRef { IntRef(); IntRef(const int& i); operator int*(); };
for (int I = 0; I < N; ++I) {
IntRef Int(IntArr[I]);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (int I : IntArr)
// CHECK-FIXES-NEXT: IntRef Int(I);
+ int *PtrArr[N];
+ for (unsigned I = 0; I < N; ++I) {
+ const int* const P = PtrArr[I];
+ printf("%d\n", *P);
+ }
+ // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto P : PtrArr)
+ // CHECK-FIXES-NEXT: printf("%d\n", *P);
+
+ IntRef Refs[N];
+ for (unsigned I = 0; I < N; ++I) {
+ int *P = Refs[I];
+ printf("%d\n", *P);
+ }
+ // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & Ref : Refs)
+ // CHECK-FIXES-NEXT: int *P = Ref;
+ // CHECK-FIXES-NEXT: printf("%d\n", *P);
+
// Ensure that removing the alias doesn't leave empty lines behind.
for (int I = 0; I < N; ++I) {
auto &X = IntArr[I];
Index: clang-tidy/modernize/LoopConvertUtils.cpp
===================================================================
--- clang-tidy/modernize/LoopConvertUtils.cpp
+++ clang-tidy/modernize/LoopConvertUtils.cpp
@@ -342,21 +342,27 @@
if (!VDecl->hasInit())
return false;
- const Expr *Init =
- digThroughConstructors(VDecl->getInit()->IgnoreParenImpCasts());
+ bool OnlyCasts = true;
+ const Expr *Init = VDecl->getInit()->IgnoreParenImpCasts();
+ if (Init && isa<CXXConstructExpr>(Init)) {
+ Init = digThroughConstructors(Init);
+ OnlyCasts = false;
+ }
if (!Init)
return false;
// Check that the declared type is the same as (or a reference to) the
// container type.
- QualType InitType = Init->getType();
- QualType DeclarationType = VDecl->getType();
- if (!DeclarationType.isNull() && DeclarationType->isReferenceType())
- DeclarationType = DeclarationType.getNonReferenceType();
+ if (!OnlyCasts) {
+ QualType InitType = Init->getType();
+ QualType DeclarationType = VDecl->getType();
+ if (!DeclarationType.isNull() && DeclarationType->isReferenceType())
+ DeclarationType = DeclarationType.getNonReferenceType();
- if (InitType.isNull() || DeclarationType.isNull() ||
- !Context->hasSameUnqualifiedType(DeclarationType, InitType))
- return false;
+ if (InitType.isNull() || DeclarationType.isNull() ||
+ !Context->hasSameUnqualifiedType(DeclarationType, InitType))
+ return false;
+ }
switch (Init->getStmtClass()) {
case Stmt::ArraySubscriptExprClass: {
@@ -384,8 +390,8 @@
const auto *MemCall = cast<CXXMemberCallExpr>(Init);
// This check is needed because getMethodDecl can return nullptr if the
// callee is a member function pointer.
- if (MemCall->getMethodDecl() &&
- MemCall->getMethodDecl()->getName() == "at") {
+ const auto *MDecl = MemCall->getMethodDecl();
+ if (MDecl && !isa<CXXConversionDecl>(MDecl) && MDecl->getName() == "at") {
assert(MemCall->getNumArgs() == 1);
return isIndexInSubscriptExpr(MemCall->getArg(0), IndexVar);
}
Index: test/clang-tidy/modernize-loop-convert-extra.cpp
===================================================================
--- test/clang-tidy/modernize-loop-convert-extra.cpp
+++ test/clang-tidy/modernize-loop-convert-extra.cpp
@@ -159,14 +159,33 @@
// CHECK-FIXES: for (int Alias : IntArr)
// CHECK-FIXES-NEXT: for (unsigned J = 0; Alias; ++J)
- struct IntRef { IntRef(const int& i); };
+ struct IntRef { IntRef(); IntRef(const int& i); operator int*(); };
for (int I = 0; I < N; ++I) {
IntRef Int(IntArr[I]);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (int I : IntArr)
// CHECK-FIXES-NEXT: IntRef Int(I);
+ int *PtrArr[N];
+ for (unsigned I = 0; I < N; ++I) {
+ const int* const P = PtrArr[I];
+ printf("%d\n", *P);
+ }
+ // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto P : PtrArr)
+ // CHECK-FIXES-NEXT: printf("%d\n", *P);
+
+ IntRef Refs[N];
+ for (unsigned I = 0; I < N; ++I) {
+ int *P = Refs[I];
+ printf("%d\n", *P);
+ }
+ // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & Ref : Refs)
+ // CHECK-FIXES-NEXT: int *P = Ref;
+ // CHECK-FIXES-NEXT: printf("%d\n", *P);
+
// Ensure that removing the alias doesn't leave empty lines behind.
for (int I = 0; I < N; ++I) {
auto &X = IntArr[I];
Index: clang-tidy/modernize/LoopConvertUtils.cpp
===================================================================
--- clang-tidy/modernize/LoopConvertUtils.cpp
+++ clang-tidy/modernize/LoopConvertUtils.cpp
@@ -342,21 +342,27 @@
if (!VDecl->hasInit())
return false;
- const Expr *Init =
- digThroughConstructors(VDecl->getInit()->IgnoreParenImpCasts());
+ bool OnlyCasts = true;
+ const Expr *Init = VDecl->getInit()->IgnoreParenImpCasts();
+ if (Init && isa<CXXConstructExpr>(Init)) {
+ Init = digThroughConstructors(Init);
+ OnlyCasts = false;
+ }
if (!Init)
return false;
// Check that the declared type is the same as (or a reference to) the
// container type.
- QualType InitType = Init->getType();
- QualType DeclarationType = VDecl->getType();
- if (!DeclarationType.isNull() && DeclarationType->isReferenceType())
- DeclarationType = DeclarationType.getNonReferenceType();
+ if (!OnlyCasts) {
+ QualType InitType = Init->getType();
+ QualType DeclarationType = VDecl->getType();
+ if (!DeclarationType.isNull() && DeclarationType->isReferenceType())
+ DeclarationType = DeclarationType.getNonReferenceType();
- if (InitType.isNull() || DeclarationType.isNull() ||
- !Context->hasSameUnqualifiedType(DeclarationType, InitType))
- return false;
+ if (InitType.isNull() || DeclarationType.isNull() ||
+ !Context->hasSameUnqualifiedType(DeclarationType, InitType))
+ return false;
+ }
switch (Init->getStmtClass()) {
case Stmt::ArraySubscriptExprClass: {
@@ -384,8 +390,8 @@
const auto *MemCall = cast<CXXMemberCallExpr>(Init);
// This check is needed because getMethodDecl can return nullptr if the
// callee is a member function pointer.
- if (MemCall->getMethodDecl() &&
- MemCall->getMethodDecl()->getName() == "at") {
+ const auto *MDecl = MemCall->getMethodDecl();
+ if (MDecl && !isa<CXXConversionDecl>(MDecl) && MDecl->getName() == "at") {
assert(MemCall->getNumArgs() == 1);
return isIndexInSubscriptExpr(MemCall->getArg(0), IndexVar);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
