[clang-tools-extra] r246762 - Two more fixes to loop convert.
Angel Garcia Gomez via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 3 05:28:11 PDT 2015
Author: angelgarcia
Date: Thu Sep 3 07:28:11 2015
New Revision: 246762
URL: http://llvm.org/viewvc/llvm-project?rev=246762&view=rev
Log:
Two more fixes to loop convert.
Summary: Ensure that the alias has the same type than the loop variable. Now it works with lambda captures.
Reviewers: klimek
Subscribers: alexfh, cfe-commits
Differential Revision: http://reviews.llvm.org/D12597
Modified:
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h
clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp
Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp?rev=246762&r1=246761&r2=246762&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp Thu Sep 3 07:28:11 2015
@@ -334,7 +334,8 @@ static bool isDereferenceOfUop(const Una
/// // use t, do not use i
/// }
/// \endcode
-static bool isAliasDecl(const Decl *TheDecl, const VarDecl *IndexVar) {
+static bool isAliasDecl(ASTContext *Context, const Decl *TheDecl,
+ const VarDecl *IndexVar) {
const auto *VDecl = dyn_cast<VarDecl>(TheDecl);
if (!VDecl)
return false;
@@ -346,6 +347,15 @@ static bool isAliasDecl(const Decl *TheD
if (!Init)
return false;
+ // Check that the declared type is the same as (or a reference to) the
+ // container type.
+ QualType DeclarationType = VDecl->getType();
+ if (DeclarationType->isReferenceType())
+ DeclarationType = DeclarationType.getNonReferenceType();
+ QualType InitType = Init->getType();
+ if (!Context->hasSameUnqualifiedType(DeclarationType, InitType))
+ return false;
+
switch (Init->getStmtClass()) {
case Stmt::ArraySubscriptExprClass: {
const auto *E = cast<ArraySubscriptExpr>(Init);
@@ -711,13 +721,49 @@ bool ForLoopIndexUseVisitor::VisitDeclRe
return true;
}
+/// \brief If the loop index is captured by a lambda, replace this capture
+/// by the range-for loop variable.
+///
+/// For example:
+/// \code
+/// for (int i = 0; i < N; ++i) {
+/// auto f = [v, i](int k) {
+/// printf("%d\n", v[i] + k);
+/// };
+/// f(v[i]);
+/// }
+/// \endcode
+///
+/// Will be replaced by:
+/// \code
+/// for (auto & elem : v) {
+/// auto f = [v, elem](int k) {
+/// printf("%d\n", elem + k);
+/// };
+/// f(elem);
+/// }
+/// \endcode
+bool ForLoopIndexUseVisitor::TraverseLambdaCapture(LambdaExpr *LE,
+ const LambdaCapture *C) {
+ if (C->capturesVariable()) {
+ const VarDecl* VDecl = C->getCapturedVar();
+ if (areSameVariable(IndexVar, cast<ValueDecl>(VDecl))) {
+ // FIXME: if the index is captured, it will count as an usage and the
+ // alias (if any) won't work, because it is only used in case of having
+ // exactly one usage.
+ Usages.push_back(Usage(nullptr, false, C->getLocation()));
+ }
+ }
+ return VisitorBase::TraverseLambdaCapture(LE, C);
+}
+
/// \brief If we find that another variable is created just to refer to the loop
/// element, note it for reuse as the loop variable.
///
/// See the comments for isAliasDecl.
bool ForLoopIndexUseVisitor::VisitDeclStmt(DeclStmt *S) {
if (!AliasDecl && S->isSingleDecl() &&
- isAliasDecl(S->getSingleDecl(), IndexVar)) {
+ isAliasDecl(Context, S->getSingleDecl(), IndexVar)) {
AliasDecl = S;
if (CurrStmtParent) {
if (isa<IfStmt>(CurrStmtParent) || isa<WhileStmt>(CurrStmtParent) ||
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=246762&r1=246761&r2=246762&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h Thu Sep 3 07:28:11 2015
@@ -309,6 +309,7 @@ private:
bool TraverseArraySubscriptExpr(ArraySubscriptExpr *E);
bool TraverseCXXMemberCallExpr(CXXMemberCallExpr *MemberCall);
bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *OpCall);
+ bool TraverseLambdaCapture(LambdaExpr *LE, const LambdaCapture *C);
bool TraverseMemberExpr(MemberExpr *Member);
bool TraverseUnaryDeref(UnaryOperator *Uop);
bool VisitDeclRefExpr(DeclRefExpr *E);
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=246762&r1=246761&r2=246762&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 Thu Sep 3 07:28:11 2015
@@ -159,8 +159,17 @@ void aliasing() {
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto alias : IntArr)
// CHECK-FIXES-NEXT: for (unsigned j = 0; alias; ++j) {
+
+ struct IntRef { IntRef(const int& i); };
+ 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 (auto & elem : IntArr) {
+ // CHECK-FIXES-NEXT: IntRef Int(elem);
}
+
void refs_and_vals() {
// The following tests check that the transform correctly preserves the
// reference or value qualifiers of the aliased variable. That is, if the
@@ -713,3 +722,165 @@ void template_instantiation() {
}
} // namespace Templates
+
+namespace Lambdas {
+
+void capturesIndex() {
+ const int N = 10;
+ int Arr[N];
+ // FIXME: the next four loops could be convertible, if the capture list is
+ // also changed.
+
+ for (int I = 0; I < N; ++I)
+ auto F1 = [Arr, I]() { int R1 = Arr[I] + 1; };
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & elem : Arr)
+ // CHECK-FIXES-NEXT: auto F1 = [Arr, elem]() { int R1 = elem + 1; };
+
+ for (int I = 0; I < N; ++I)
+ auto F2 = [Arr, &I]() { int R2 = Arr[I] + 3; };
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & elem : Arr)
+ // CHECK-FIXES-NEXT: auto F2 = [Arr, &elem]() { int R2 = elem + 3; };
+
+ // FIXME: alias don't work if the index is captured.
+ // Alias declared inside lambda (by value).
+ for (int I = 0; I < N; ++I)
+ auto F3 = [&Arr, I]() { int R3 = Arr[I]; };
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & elem : Arr)
+ // CHECK-FIXES-NEXT: auto F3 = [&Arr, elem]() { int R3 = elem; };
+ // FIXME: this does two copies instead of one. Capture elem by ref?
+
+
+ for (int I = 0; I < N; ++I)
+ auto F4 = [&Arr, &I]() { int R4 = Arr[I]; };
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & elem : Arr)
+ // CHECK-FIXES-NEXT: auto F4 = [&Arr, &elem]() { int R4 = elem; };
+
+ // Alias declared inside lambda (by reference).
+ for (int I = 0; I < N; ++I)
+ auto F5 = [&Arr, I]() { int &R5 = Arr[I]; };
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & elem : Arr)
+ // CHECK-FIXES-NEXT: auto F5 = [&Arr, elem]() { int &R5 = elem; };
+ // FIXME: this does one copy instead of none. Capture elem by ref?
+
+
+ for (int I = 0; I < N; ++I)
+ auto F6 = [&Arr, &I]() { int &R6 = Arr[I]; };
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & elem : Arr)
+ // CHECK-FIXES-NEXT: auto F6 = [&Arr, &elem]() { int &R6 = elem; };
+
+ for (int I = 0; I < N; ++I) {
+ auto F = [Arr, I](int k) {
+ printf("%d\n", Arr[I] + k);
+ };
+ F(Arr[I]);
+ }
+ // CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & elem : Arr)
+ // CHECK-FIXES-NEXT: auto F = [Arr, elem](int k) {
+ // CHECK-FIXES-NEXT: printf("%d\n", elem + k);
+ // CHECK-FIXES-NEXT: };
+ // CHECK-FIXES-NEXT: F(elem);
+}
+
+void implicitCapture() {
+ const int N = 10;
+ int Arr[N];
+ // Index is used, not convertible.
+ for (int I = 0; I < N; ++I) {
+ auto G1 = [&]() {
+ int R = Arr[I];
+ int J = I;
+ };
+ }
+
+ for (int I = 0; I < N; ++I) {
+ auto G2 = [=]() {
+ int R = Arr[I];
+ int J = I;
+ };
+ }
+
+ // Convertible.
+ for (int I = 0; I < N; ++I) {
+ auto G3 = [&]() {
+ int R3 = Arr[I];
+ int J3 = Arr[I] + R3;
+ };
+ }
+ // CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & elem : Arr)
+ // CHECK-FIXES-NEXT: auto G3 = [&]() {
+ // CHECK-FIXES-NEXT: int R3 = elem;
+ // CHECK-FIXES-NEXT: int J3 = elem + R3;
+
+ for (int I = 0; I < N; ++I) {
+ auto G4 = [=]() {
+ int R4 = Arr[I] + 5;
+ };
+ }
+ // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & elem : Arr)
+ // CHECK-FIXES-NEXT: auto G4 = [=]() {
+ // CHECK-FIXES-NEXT: int R4 = elem + 5;
+
+ // Alias by value.
+ for (int I = 0; I < N; ++I) {
+ auto G5 = [&]() {
+ int R5 = Arr[I];
+ int J5 = 8 + R5;
+ };
+ }
+ // CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto R5 : Arr)
+ // CHECK-FIXES-NEXT: auto G5 = [&]() {
+ // CHECK-FIXES: int J5 = 8 + R5;
+
+ // Alias by reference.
+ for (int I = 0; I < N; ++I) {
+ auto G6 = [&]() {
+ int &R6 = Arr[I];
+ int J6 = -1 + R6;
+ };
+ }
+ // CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & R6 : Arr)
+ // CHECK-FIXES-NEXT: auto G6 = [&]() {
+ // CHECK-FIXES: int J6 = -1 + R6;
+}
+
+void iterators() {
+ dependent<int> dep;
+
+ for (dependent<int>::iterator I = dep.begin(), E = dep.end(); I != E; ++I)
+ auto H1 = [&I]() { int R = *I; };
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & elem : dep)
+ // CHECK-FIXES-NEXT: auto H1 = [&elem]() { int R = elem; };
+
+ for (dependent<int>::iterator I = dep.begin(), E = dep.end(); I != E; ++I)
+ auto H2 = [&]() { int R = *I + 2; };
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES: for (auto & elem : dep)
+ // CHECK-FIXES-NEXT: auto H2 = [&]() { int R = elem + 2; };
+
+ // FIXME: It doesn't work with const iterators.
+ for (dependent<int>::const_iterator I = dep.begin(), E = dep.end();
+ I != E; ++I)
+ auto H3 = [I]() { int R = *I; };
+
+ for (dependent<int>::const_iterator I = dep.begin(), E = dep.end();
+ I != E; ++I)
+ auto H4 = [&]() { int R = *I + 1; };
+
+ for (dependent<int>::const_iterator I = dep.begin(), E = dep.end();
+ I != E; ++I)
+ auto H5 = [=]() { int R = *I; };
+}
+
+} // namespace Lambdas
More information about the cfe-commits
mailing list