[PATCH] D56661: [clang-tidy] Fix incorrect array name generation in cppcoreguidelines-pro-bounds-constant-array-index

2019-09-12 Thread Jonas Toth via Phabricator via cfe-commits
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

2019-07-12 Thread Dmitry Venikov via Phabricator via cfe-commits
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

2019-01-15 Thread Jonas Toth via Phabricator via cfe-commits
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

2019-01-15 Thread Dmitry Venikov via Phabricator via cfe-commits
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

2019-01-14 Thread Dmitry Venikov via Phabricator via cfe-commits
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

2019-01-14 Thread Dmitry Venikov via Phabricator via cfe-commits
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,