[clang-tools-extra] r251943 - Handle correctly containers that are data members in modernize-loop-convert.
Angel Garcia Gomez via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 3 08:38:31 PST 2015
Author: angelgarcia
Date: Tue Nov 3 10:38:31 2015
New Revision: 251943
URL: http://llvm.org/viewvc/llvm-project?rev=251943&view=rev
Log:
Handle correctly containers that are data members in modernize-loop-convert.
Summary:
I recently found that the variable naming wasn't working as expected with containers that are data members. The new index always received the name "Elem" (or equivalent) regardless of the container's name.
The check was assuming that the container's declaration was a VarDecl, which cannot be converted to a FieldDecl (a data member), and then it could never retrieve its name.
This also fixes some cases where the check failed to find the container at all (so it didn't do any fix) because of the same reason.
Reviewers: klimek
Subscribers: cfe-commits, alexfh
Differential Revision: http://reviews.llvm.org/D14289
Modified:
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h
clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-const.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-negative.cpp
Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp?rev=251943&r1=251942&r2=251943&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp Tue Nov 3 10:38:31 2015
@@ -353,10 +353,12 @@ static StringRef getStringFromRange(Sour
}
/// \brief If the given expression is actually a DeclRefExpr, find and return
-/// the underlying VarDecl; otherwise, return NULL.
-static const VarDecl *getReferencedVariable(const Expr *E) {
+/// the underlying ValueDecl; otherwise, return NULL.
+static const ValueDecl *getReferencedVariable(const Expr *E) {
if (const DeclRefExpr *DRE = getDeclRef(E))
return dyn_cast<VarDecl>(DRE->getDecl());
+ if (const auto *Mem = dyn_cast<MemberExpr>(E))
+ return dyn_cast<FieldDecl>(Mem->getMemberDecl());
return nullptr;
}
@@ -500,9 +502,10 @@ void LoopConvertCheck::getAliasRange(Sou
/// \brief Computes the changes needed to convert a given for loop, and
/// applies them.
void LoopConvertCheck::doConversion(
- ASTContext *Context, const VarDecl *IndexVar, const VarDecl *MaybeContainer,
- const UsageResult &Usages, const DeclStmt *AliasDecl, bool AliasUseRequired,
- bool AliasFromForInit, const ForStmt *Loop, RangeDescriptor Descriptor) {
+ ASTContext *Context, const VarDecl *IndexVar,
+ const ValueDecl *MaybeContainer, const UsageResult &Usages,
+ const DeclStmt *AliasDecl, bool AliasUseRequired, bool AliasFromForInit,
+ const ForStmt *Loop, RangeDescriptor Descriptor) {
auto Diag = diag(Loop->getForLoc(), "use range-based for loop instead");
std::string VarName;
Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h?rev=251943&r1=251942&r2=251943&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h Tue Nov 3 10:38:31 2015
@@ -37,7 +37,7 @@ private:
void getAliasRange(SourceManager &SM, SourceRange &DeclRange);
void doConversion(ASTContext *Context, const VarDecl *IndexVar,
- const VarDecl *MaybeContainer, const UsageResult &Usages,
+ const ValueDecl *MaybeContainer, const UsageResult &Usages,
const DeclStmt *AliasDecl, bool AliasUseRequired,
bool AliasFromForInit, const ForStmt *Loop,
RangeDescriptor Descriptor);
Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h?rev=251943&r1=251942&r2=251943&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h Tue Nov 3 10:38:31 2015
@@ -425,7 +425,7 @@ public:
VariableNamer(StmtGeneratedVarNameMap *GeneratedDecls,
const StmtParentMap *ReverseAST, const clang::Stmt *SourceStmt,
const clang::VarDecl *OldIndex,
- const clang::VarDecl *TheContainer,
+ const clang::ValueDecl *TheContainer,
const clang::ASTContext *Context, NamingStyle Style)
: GeneratedDecls(GeneratedDecls), ReverseAST(ReverseAST),
SourceStmt(SourceStmt), OldIndex(OldIndex), TheContainer(TheContainer),
@@ -443,7 +443,7 @@ private:
const StmtParentMap *ReverseAST;
const clang::Stmt *SourceStmt;
const clang::VarDecl *OldIndex;
- const clang::VarDecl *TheContainer;
+ const clang::ValueDecl *TheContainer;
const clang::ASTContext *Context;
const NamingStyle Style;
Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp?rev=251943&r1=251942&r2=251943&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp Tue Nov 3 10:38:31 2015
@@ -168,6 +168,41 @@ struct HasArr {
}
};
+struct HasIndirectArr {
+ HasArr HA;
+ void implicitThis() {
+ for (int I = 0; I < N; ++I) {
+ printf("%d", HA.Arr[I]);
+ }
+ // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
+ // CHECK-FIXES: for (int Elem : HA.Arr)
+ // CHECK-FIXES-NEXT: printf("%d", Elem);
+
+ for (int I = 0; I < N; ++I) {
+ printf("%d", HA.ValArr[I].X);
+ }
+ // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & Elem : HA.ValArr)
+ // CHECK-FIXES-NEXT: printf("%d", Elem.X);
+ }
+
+ void explicitThis() {
+ for (int I = 0; I < N; ++I) {
+ printf("%d", this->HA.Arr[I]);
+ }
+ // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
+ // CHECK-FIXES: for (int Elem : this->HA.Arr)
+ // CHECK-FIXES-NEXT: printf("%d", Elem);
+
+ for (int I = 0; I < N; ++I) {
+ printf("%d", this->HA.ValArr[I].X);
+ }
+ // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & Elem : this->HA.ValArr)
+ // CHECK-FIXES-NEXT: printf("%d", Elem.X);
+ }
+};
+
// Loops whose bounds are value-dependent should not be converted.
template <int N>
void dependentExprBound() {
Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-const.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-const.cpp?rev=251943&r1=251942&r2=251943&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-const.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-const.cpp Tue Nov 3 10:38:31 2015
@@ -341,7 +341,7 @@ class TestInsideConstFunction {
copyArg(Ints[I]);
}
// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use range-based for loop
- // CHECK-FIXES: for (int Elem : Ints)
+ // CHECK-FIXES: for (int Int : Ints)
for (int I = 0; I < N; ++I) {
Array[I].constMember(0);
Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp?rev=251943&r1=251942&r2=251943&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp Tue Nov 3 10:38:31 2015
@@ -237,6 +237,26 @@ void refs_and_vals() {
}
}
+struct MemberNaming {
+ const static int N = 10;
+ int Ints[N], Ints_[N];
+ void loops() {
+ for (int I = 0; I < N; ++I) {
+ printf("%d\n", Ints[I]);
+ }
+ // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
+ // CHECK-FIXES: for (int Int : Ints)
+ // CHECK-FIXES-NEXT: printf("%d\n", Int);
+
+ for (int I = 0; I < N; ++I) {
+ printf("%d\n", Ints_[I]);
+ }
+ // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
+ // CHECK-FIXES: for (int Int : Ints_)
+ // CHECK-FIXES-NEXT: printf("%d\n", Int);
+ }
+};
+
} // namespace NamingAlias
namespace NamingConlict {
Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-negative.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-negative.cpp?rev=251943&r1=251942&r2=251943&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-negative.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-negative.cpp Tue Nov 3 10:38:31 2015
@@ -92,33 +92,6 @@ void multipleArrays() {
}
}
-struct HasArr {
- int Arr[N];
- Val ValArr[N];
-};
-
-struct HasIndirectArr {
- HasArr HA;
- void implicitThis() {
- for (int I = 0; I < N; ++I) {
- printf("%d", HA.Arr[I]);
- }
-
- for (int I = 0; I < N; ++I) {
- printf("%d", HA.ValArr[I].X);
- }
- }
-
- void explicitThis() {
- for (int I = 0; I < N; ++I) {
- printf("%d", this->HA.Arr[I]);
- }
-
- for (int I = 0; I < N; ++I) {
- printf("%d", this->HA.ValArr[I].X);
- }
- }
-};
}
namespace NegativeIterator {
More information about the cfe-commits
mailing list