[PATCH] D56661: [clang-tidy] Fix incorrect array name generation in cppcoreguidelines-pro-bounds-constant-array-index
JonasToth added a comment. Sorry for the long delay! I have one question about a slighty different code constellation. std::optional> my_array; for(int i = 0; i < 42; ++i) (*my_array)[i] = 42; Will this be fixed properly? for(int i = 0; i < 42; ++i) gsl::at(*my_array, i) = 42; If the braces in `(*my_array)` would be moved into the function that is ok as well. This is just of general interest and should not block this patch in my opinion, as its a different bug. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56661/new/ https://reviews.llvm.org/D56661 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56661: [clang-tidy] Fix incorrect array name generation in cppcoreguidelines-pro-bounds-constant-array-index
Quolyk updated this revision to Diff 209485. Quolyk added a comment. Herald added a subscriber: wuzish. Herald added a project: clang. Update. Solution is not elegant, because DeclRef->BaseRange somehow has zero length. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56661/new/ https://reviews.llvm.org/D56661 Files: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp Index: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp === --- test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp +++ test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp @@ -23,46 +23,46 @@ return base + 3; } -void f(std::array a, int pos) { - a [ pos / 2 /*comment*/] = 1; +void f(std::array list, int pos) { + list [ pos / 2 /*comment*/] = 1; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index] - int j = a[pos - 1]; + int j = list[pos - 1]; // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead - a.at(pos-1) = 2; // OK, at() instead of [] - gsl::at(a, pos-1) = 2; // OK, gsl::at() instead of [] + list.at(pos-1) = 2; // OK, at() instead of [] + gsl::at(list, pos-1) = 2; // OK, gsl::at() instead of [] - a[-1] = 3; + list[-1] = 3; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index -1 is negative [cppcoreguidelines-pro-bounds-constant-array-index] - a[10] = 4; + list[10] = 4; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past the end of the array (which contains 10 elements) [cppcoreguidelines-pro-bounds-constant-array-index] - a[const_index(7)] = 3; + list[const_index(7)] = 3; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past the end of the array (which contains 10 elements) - a[0] = 3; // OK, constant index and inside bounds - a[1] = 3; // OK, constant index and inside bounds - a[9] = 3; // OK, constant index and inside bounds - a[const_index(6)] = 3; // OK, constant index and inside bounds + list[0] = 3; // OK, constant index and inside bounds + list[1] = 3; // OK, constant index and inside bounds + list[9] = 3; // OK, constant index and inside bounds + list[const_index(6)] = 3; // OK, constant index and inside bounds } void g() { - int a[10]; + int list[10]; for (int i = 0; i < 10; ++i) { -a[i] = i; +list[i] = i; // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead -// CHECK-FIXES: gsl::at(a, i) = i; -gsl::at(a, i) = i; // OK, gsl::at() instead of [] +// CHECK-FIXES: gsl::at(list, i) = i; +gsl::at(list, i) = i; // OK, gsl::at() instead of [] } - a[-1] = 3; // flagged by clang-diagnostic-array-bounds - a[10] = 4; // flagged by clang-diagnostic-array-bounds - a[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds - - a[0] = 3; // OK, constant index and inside bounds - a[1] = 3; // OK, constant index and inside bounds - a[9] = 3; // OK, constant index and inside bounds - a[const_index(6)] = 3; // OK, constant index and inside bounds + list[-1] = 3; // flagged by clang-diagnostic-array-bounds + list[10] = 4; // flagged by clang-diagnostic-array-bounds + list[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds + + list[0] = 3; // OK, constant index and inside bounds + list[1] = 3; // OK, constant index and inside bounds + list[9] = 3; // OK, constant index and inside bounds + list[const_index(6)] = 3; // OK, constant index and inside bounds } struct S { Index: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp === --- test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp +++ test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp @@ -25,48 +25,48 @@ return base + 3; } -void f(std::array a, int pos) { - a [ pos / 2 /*comment*/] = 1; +void f(std::array list, int pos) { + list [ pos / 2 /*comment*/] = 1; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index] - // CHECK-FIXES: gsl::at(a, pos / 2 /*comment*/) = 1; - int j = a[pos - 1]; + // CHECK-FIXES: gsl::at(list, pos / 2 /*comment*/) = 1; + int j = list[pos - 1]; // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when
[PATCH] D56661: [clang-tidy] Fix incorrect array name generation in cppcoreguidelines-pro-bounds-constant-array-index
JonasToth added a comment. Thank you for working on this! Could you please run the check over LLVM or any other significant codebase once you have the fix implemented and report if the transformation still breaks code? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56661/new/ https://reviews.llvm.org/D56661 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56661: [clang-tidy] Fix incorrect array name generation in cppcoreguidelines-pro-bounds-constant-array-index
Quolyk updated this revision to Diff 181763. Quolyk added a comment. Patch is not yet fixed. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56661/new/ https://reviews.llvm.org/D56661 Files: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp Index: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp === --- test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp +++ test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp @@ -23,46 +23,46 @@ return base + 3; } -void f(std::array a, int pos) { - a [ pos / 2 /*comment*/] = 1; +void f(std::array list, int pos) { + list [ pos / 2 /*comment*/] = 1; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index] - int j = a[pos - 1]; + int j = list[pos - 1]; // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead - a.at(pos-1) = 2; // OK, at() instead of [] - gsl::at(a, pos-1) = 2; // OK, gsl::at() instead of [] + list.at(pos-1) = 2; // OK, at() instead of [] + gsl::at(list, pos-1) = 2; // OK, gsl::at() instead of [] - a[-1] = 3; + list[-1] = 3; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index -1 is negative [cppcoreguidelines-pro-bounds-constant-array-index] - a[10] = 4; + list[10] = 4; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past the end of the array (which contains 10 elements) [cppcoreguidelines-pro-bounds-constant-array-index] - a[const_index(7)] = 3; + list[const_index(7)] = 3; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past the end of the array (which contains 10 elements) - a[0] = 3; // OK, constant index and inside bounds - a[1] = 3; // OK, constant index and inside bounds - a[9] = 3; // OK, constant index and inside bounds - a[const_index(6)] = 3; // OK, constant index and inside bounds + list[0] = 3; // OK, constant index and inside bounds + list[1] = 3; // OK, constant index and inside bounds + list[9] = 3; // OK, constant index and inside bounds + list[const_index(6)] = 3; // OK, constant index and inside bounds } void g() { - int a[10]; + int list[10]; for (int i = 0; i < 10; ++i) { -a[i] = i; +list[i] = i; // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead -// CHECK-FIXES: gsl::at(a, i) = i; -gsl::at(a, i) = i; // OK, gsl::at() instead of [] +// CHECK-FIXES: gsl::at(list, i) = i; +gsl::at(list, i) = i; // OK, gsl::at() instead of [] } - a[-1] = 3; // flagged by clang-diagnostic-array-bounds - a[10] = 4; // flagged by clang-diagnostic-array-bounds - a[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds - - a[0] = 3; // OK, constant index and inside bounds - a[1] = 3; // OK, constant index and inside bounds - a[9] = 3; // OK, constant index and inside bounds - a[const_index(6)] = 3; // OK, constant index and inside bounds + list[-1] = 3; // flagged by clang-diagnostic-array-bounds + list[10] = 4; // flagged by clang-diagnostic-array-bounds + list[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds + + list[0] = 3; // OK, constant index and inside bounds + list[1] = 3; // OK, constant index and inside bounds + list[9] = 3; // OK, constant index and inside bounds + list[const_index(6)] = 3; // OK, constant index and inside bounds } struct S { Index: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp === --- test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp +++ test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp @@ -24,48 +24,48 @@ return base + 3; } -void f(std::array a, int pos) { - a [ pos / 2 /*comment*/] = 1; +void f(std::array list, int pos) { + list [ pos / 2 /*comment*/] = 1; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index] - // CHECK-FIXES: gsl::at(a, pos / 2 /*comment*/) = 1; - int j = a[pos - 1]; + // CHECK-FIXES: gsl::at(list, pos / 2 /*comment*/) = 1; + int j = list[pos - 1]; // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead - // CHECK-FIXES: int j = gsl::at(a, pos - 1); + //
[PATCH] D56661: [clang-tidy] Fix incorrect array name generation in cppcoreguidelines-pro-bounds-constant-array-index
Quolyk added a comment. For now I just updated tests. The problem is in `BaseRange` definition, as it holds `EndLoc` and `BeginLoc` pointing to the beginning of ArrayExpression base https://github.com/llvm-mirror/clang-tools-extra/blob/e0441f6939da38f26bea9c1d75bb33024daa4e40/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp#L78. I'm investigating this. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56661/new/ https://reviews.llvm.org/D56661 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56661: [clang-tidy] Fix incorrect array name generation in cppcoreguidelines-pro-bounds-constant-array-index
Quolyk created this revision. Quolyk added reviewers: aaron.ballman, alexfh, JonasToth, omtcyfz. Herald added subscribers: cfe-commits, arphaman, kbarton, xazax.hun, nemanjai. This patch fixes incorrect array name generation for a cppcoreguidelines-pro-bounds-constant-array-index warning. Motivation: https://bugs.llvm.org/show_bug.cgi?id=38510. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D56661 Files: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp Index: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp === --- test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp +++ test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp @@ -47,12 +47,12 @@ } void g() { - int a[10]; + int list[10]; for (int i = 0; i < 10; ++i) { -a[i] = i; +list[i] = i; // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead -// CHECK-FIXES: gsl::at(a, i) = i; -gsl::at(a, i) = i; // OK, gsl::at() instead of [] +// CHECK-FIXES: gsl::at(list, i) = i; +gsl::at(list, i) = i; // OK, gsl::at() instead of [] } a[-1] = 3; // flagged by clang-diagnostic-array-bounds Index: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp === --- test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp +++ test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp @@ -24,48 +24,48 @@ return base + 3; } -void f(std::array a, int pos) { - a [ pos / 2 /*comment*/] = 1; +void f(std::array list, int pos) { + list [ pos / 2 /*comment*/] = 1; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index] - // CHECK-FIXES: gsl::at(a, pos / 2 /*comment*/) = 1; - int j = a[pos - 1]; + // CHECK-FIXES: gsl::at(list, pos / 2 /*comment*/) = 1; + int j = list[pos - 1]; // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead - // CHECK-FIXES: int j = gsl::at(a, pos - 1); + // CHECK-FIXES: int j = gsl::at(list, pos - 1); - a.at(pos-1) = 2; // OK, at() instead of [] - gsl::at(a, pos-1) = 2; // OK, gsl::at() instead of [] + list.at(pos-1) = 2; // OK, at() instead of [] + gsl::at(list, pos-1) = 2; // OK, gsl::at() instead of [] - a[-1] = 3; + list[-1] = 3; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index -1 is negative [cppcoreguidelines-pro-bounds-constant-array-index] - a[10] = 4; + list[10] = 4; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past the end of the array (which contains 10 elements) [cppcoreguidelines-pro-bounds-constant-array-index] - a[const_index(7)] = 3; + list[const_index(7)] = 3; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past the end of the array (which contains 10 elements) - a[0] = 3; // OK, constant index and inside bounds - a[1] = 3; // OK, constant index and inside bounds - a[9] = 3; // OK, constant index and inside bounds - a[const_index(6)] = 3; // OK, constant index and inside bounds + list[0] = 3; // OK, constant index and inside bounds + list[1] = 3; // OK, constant index and inside bounds + list[9] = 3; // OK, constant index and inside bounds + list[const_index(6)] = 3; // OK, constant index and inside bounds } void g() { int a[10]; for (int i = 0; i < 10; ++i) { -a[i] = i; +list[i] = i; // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead -// CHECK-FIXES: gsl::at(a, i) = i; -gsl::at(a, i) = i; // OK, gsl::at() instead of [] +// CHECK-FIXES: gsl::at(list, i) = i; +gsl::at(list, i) = i; // OK, gsl::at() instead of [] } - a[-1] = 3; // flagged by clang-diagnostic-array-bounds - a[10] = 4; // flagged by clang-diagnostic-array-bounds - a[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds - - a[0] = 3; // OK, constant index and inside bounds - a[1] = 3; // OK, constant index and inside bounds - a[9] = 3; // OK, constant index and inside bounds - a[const_index(6)] = 3; // OK, constant index and inside bounds + list[-1] = 3; // flagged by clang-diagnostic-array-bounds + list[10] = 4; // flagged by clang-diagnostic-array-bounds + list[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds + + list[0] = 3; // OK, constant index and inside bounds + list[1] = 3; // OK, constant index and inside bounds + list[9] = 3; // OK,