carlosgalvezp created this revision.
Herald added subscribers: shchenz, arphaman, kbarton, kristof.beyls, xazax.hun,
nemanjai.
carlosgalvezp requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Currently the fix hint is hardcoded to gsl::at(). This poses
a problem for people who, for a number of reasons, don't want
or cannot use the GSL library.
In these situations, the fix hint does more harm than good
as it creates confusion as to what the fix should be. People
can even misinterpret the fix "gsl::at" as e.g. "std::array::at",
which can lead to even more trouble (e.g. when having guidelines
that disallow exceptions).
Furthermore, this is not a requirement from the CppCoreGuidelines,
simply that array indexing needs to be safe. Each project should
be able to decide upon a strategy for safe indexing.
The fix-it is kept for people who want to use the GSL library.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D117857
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index.cpp
Index:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index.cpp
===================================================================
---
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index.cpp
+++
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index.cpp
@@ -25,9 +25,9 @@
void f(std::array<int, 10> a, int pos) {
a [ 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-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when
the index is not an integer constant expression
[cppcoreguidelines-pro-bounds-constant-array-index]
int j = a[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-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when
the index is not an integer constant expression
a.at(pos-1) = 2; // OK, at() instead of []
gsl::at(a, pos-1) = 2; // OK, gsl::at() instead of []
@@ -50,7 +50,7 @@
int a[10];
for (int i = 0; i < 10; ++i) {
a[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-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript
when the index is not an integer constant expression
// CHECK-FIXES: gsl::at(a, i) = i;
gsl::at(a, i) = i; // OK, gsl::at() instead of []
}
Index:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
===================================================================
---
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
+++
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
@@ -27,10 +27,10 @@
void f(std::array<int, 10> a, int pos) {
a [ 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-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when
the index is not an integer constant expression
[cppcoreguidelines-pro-bounds-constant-array-index]
// CHECK-FIXES: gsl::at(a, pos / 2 /*comment*/) = 1;
int j = a[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-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when
the index is not an integer constant expression
// CHECK-FIXES: int j = gsl::at(a, pos - 1);
a.at(pos-1) = 2; // OK, at() instead of []
@@ -54,7 +54,7 @@
int a[10];
for (int i = 0; i < 10; ++i) {
a[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-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript
when the index is not an integer constant expression
// CHECK-FIXES: gsl::at(a, i) = i;
gsl::at(a, i) = i; // OK, gsl::at() instead of []
}
Index:
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
===================================================================
---
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
+++
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
@@ -11,6 +11,8 @@
This rule is part of the "Bounds safety" profile of the C++ Core Guidelines,
see
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Pro-bounds-arrayindex.
+Optionally, this check can generate fixes using ``gsl::at`` for indexing.
+
Options
-------
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -156,6 +156,12 @@
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Removed suggestion ``use gsl::at`` from warning message in the
+ ``cppcoreguidelines-pro-bounds-constant-array-index`` check, since that's not
+ a requirement from the CppCoreGuidelines. This allows people to choose
+ their own safe indexing strategy. The fix-it is kept for those who want to
+ use the GSL library.
+
- Removed default setting
``cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"``,
to match the current state of the C++ Core Guidelines.
Index:
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
===================================================================
---
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -76,8 +76,7 @@
auto Diag = diag(Matched->getExprLoc(),
"do not use array subscript when the index is "
- "not an integer constant expression; use gsl::at() "
- "instead");
+ "not an integer constant expression");
if (!GslHeader.empty()) {
Diag << FixItHint::CreateInsertion(BaseRange.getBegin(), "gsl::at(")
<< FixItHint::CreateReplacement(
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index.cpp
@@ -25,9 +25,9 @@
void f(std::array<int, 10> a, int pos) {
a [ 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-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not an integer constant expression [cppcoreguidelines-pro-bounds-constant-array-index]
int j = a[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-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not an integer constant expression
a.at(pos-1) = 2; // OK, at() instead of []
gsl::at(a, pos-1) = 2; // OK, gsl::at() instead of []
@@ -50,7 +50,7 @@
int a[10];
for (int i = 0; i < 10; ++i) {
a[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-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript when the index is not an integer constant expression
// CHECK-FIXES: gsl::at(a, i) = i;
gsl::at(a, i) = i; // OK, gsl::at() instead of []
}
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
@@ -27,10 +27,10 @@
void f(std::array<int, 10> a, int pos) {
a [ 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-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not an integer constant expression [cppcoreguidelines-pro-bounds-constant-array-index]
// CHECK-FIXES: gsl::at(a, pos / 2 /*comment*/) = 1;
int j = a[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-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not an integer constant expression
// CHECK-FIXES: int j = gsl::at(a, pos - 1);
a.at(pos-1) = 2; // OK, at() instead of []
@@ -54,7 +54,7 @@
int a[10];
for (int i = 0; i < 10; ++i) {
a[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-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript when the index is not an integer constant expression
// CHECK-FIXES: gsl::at(a, i) = i;
gsl::at(a, i) = i; // OK, gsl::at() instead of []
}
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
@@ -11,6 +11,8 @@
This rule is part of the "Bounds safety" profile of the C++ Core Guidelines, see
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Pro-bounds-arrayindex.
+Optionally, this check can generate fixes using ``gsl::at`` for indexing.
+
Options
-------
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -156,6 +156,12 @@
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Removed suggestion ``use gsl::at`` from warning message in the
+ ``cppcoreguidelines-pro-bounds-constant-array-index`` check, since that's not
+ a requirement from the CppCoreGuidelines. This allows people to choose
+ their own safe indexing strategy. The fix-it is kept for those who want to
+ use the GSL library.
+
- Removed default setting ``cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"``,
to match the current state of the C++ Core Guidelines.
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -76,8 +76,7 @@
auto Diag = diag(Matched->getExprLoc(),
"do not use array subscript when the index is "
- "not an integer constant expression; use gsl::at() "
- "instead");
+ "not an integer constant expression");
if (!GslHeader.empty()) {
Diag << FixItHint::CreateInsertion(BaseRange.getBegin(), "gsl::at(")
<< FixItHint::CreateReplacement(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
