squiid/llvm
1
0
Fork 0
Code Issues Pull requests Activity

[clang-tidy] fix modernize-loop-convert to retain needed array-like operator[]

Browse source 
`modernize-loop-convert` handles //array-like// objects like vectors fairly well, but strips slightly too much information from the iteration expression by converting:
```
  Vector<Vector<int>> X;
  for (int J = 0; J < X[5].size(); ++J)
    copyArg(X[5][J]);
```
to
```
  Vector<Vector<int>> X;
  for (int J : X) // should be for (int J : X[5])
    copyArg(J);
```
The `[5]` is a call to `operator[]` and gets stripped by `LoopConvertCheck::getContainerString`. This patch fixes that and adds several test cases.

Reviewed By: njames93

Differential Revision: https://reviews.llvm.org/D95771
This commit is contained in:
poelmanc 2021-02-07 16:34:05 +00:00 committed by Nathan James
parent ddca007a29
commit 5229edd667
No known key found for this signature in database
GPG key ID: CC007AFCDA90AA5F
2 changed files with 83 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
View file

@ -712,10 +712,11 @@ StringRef LoopConvertCheck::getContainerString(ASTContext *Context,
if (isa<CXXThisExpr>(ContainerExpr)) {
ContainerString = "this";
} else {
// For CXXOperatorCallExpr (e.g. vector_ptr->size()), its first argument is
// the class object (vector_ptr) we are targeting.
// For CXXOperatorCallExpr such as vector_ptr->size() we want the class
// object vector_ptr, but for vector[2] we need the whole expression.
if (const auto* E = dyn_cast<CXXOperatorCallExpr>(ContainerExpr))
ContainerExpr = E->getArg(0);
if (E->getOperator() != OO_Subscript)
ContainerExpr = E->getArg(0);
ContainerString =
getStringFromRange(Context->getSourceManager(), Context->getLangOpts(),
ContainerExpr->getSourceRange());

79
clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-multidimensional.cpp Normal file
View file

@ -0,0 +1,79 @@
// RUN: %check_clang_tidy %s modernize-loop-convert %t
template <class T>
struct Vector {
using iterator = T*;
unsigned size() const;
const T &operator[](int) const;
T &operator[](int);
T *begin();
T *end();
const T *begin() const;
const T *end() const;
};
template <typename T>
void copyArg(T);
class TestArrayOfVector {
Vector<int> W[2];
void foo() const {
for (int I = 0; I < W[0].size(); ++I) {
if (W[0][I])
copyArg(W[0][I]);
}
// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use range-based for loop
// CHECK-FIXES: for (int I : W[0]) {
// CHECK-FIXES-NEXT: if (I)
// CHECK-FIXES-NEXT: copyArg(I);
}
};
class TestVectorOfVector {
Vector<Vector<int>> X;
void foo() const {
for (int J = 0; J < X[0].size(); ++J) {
if (X[0][J])
copyArg(X[0][J]);
}
// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use range-based for loop
// CHECK-FIXES: for (int J : X[0]) {
// CHECK-FIXES-NEXT: if (J)
// CHECK-FIXES-NEXT: copyArg(J);
}
};
void testVectorOfVectorOfVector() {
Vector<Vector<Vector<int>>> Y;
for (int J = 0; J < Y[3].size(); ++J) {
if (Y[3][J][7])
copyArg(Y[3][J][8]);
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop
// CHECK-FIXES: for (auto & J : Y[3]) {
// CHECK-FIXES-NEXT: if (J[7])
// CHECK-FIXES-NEXT: copyArg(J[8]);
for (int J = 0; J < Y[3][4].size(); ++J) {
if (Y[3][4][J])
copyArg(Y[3][4][J]);
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop
// CHECK-FIXES: for (int J : Y[3][4]) {
// CHECK-FIXES-NEXT: if (J)
// CHECK-FIXES-NEXT: copyArg(J);
}
void testVectorOfVectorIterator() {
Vector<Vector<int>> Z;
for (Vector<int>::iterator it = Z[4].begin(); it != Z[4].end(); ++it) {
if (*it)
copyArg(*it);
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop
// CHECK-FIXES: for (int & it : Z[4]) {
// CHECK-FIXES-NEXT: if (it)
// CHECK-FIXES-NEXT: copyArg(it);
}